Sharing overview page #8417
Sharing overview page #8417
Conversation
| * | ||
| */ | ||
|
|
||
| // Check if we are a user |
There was a problem hiding this comment.
Please get rid of this. - I don't know who distributed this over the whole code but we should stop doing it :-)
|
Very cool. The switch between "Shared with me" and "Shared by me" is a bit clunky. I goes we should have two separate entries for that in the future sidebar. We have plenty of space for that then. |
|
I do not understand fully, can you please explain more? I thought, the files app should get a app-navigation ( #1936 ) - then this should not be a seperate app but a list entry in the app-navigation. Or is this just the first step and will be part of the app-navigation in a next step?
|
|
Yes, this is just the first step. The focus is the share overview itself, not the navigation. |
|
So if this will get merged into the future app-navigation of the files app, there should no top-bar exist. In the app-navigation should be two entries "shared with me" and "shared with others" |
Yes, that's the plan. But as long as we don't have the sidebar for the files app we thought that the two buttons at the top are better than two entries at the apps menu at the left. |
|
Even though it’s just a stop-gap solution regarding layout (before the sidebar), we should put the »Shared« not in the navigation but rather as a button in the Files app like »Deleted files« (which we could shorten to »Trash« or »Deleted«.) Otherwise you might think it would also contain other shares, like Calendars or Contacts. But it is a filter for Files, so it should be in Files. |
|
Okay, will do. Should I put two buttons there, one for "shared with me" and one "shared with others" ?
|
|
@PVince81 yeah, that would be fine for now. But to be honest I’d say the sidebar would be pretty nice to have – especially because it’s going to be very bad like this on mobile. Even if there’s just these 4 entries: »All files«, »Shared with you« (do not say »me«), »Shared with others« and »Deleted files«. Everything for the sidebar is in place – the #6260 (files app navigation sidebar) and #8159 (mobile sidebar swipe). |
|
I don't think this branch should be merged before the sidebar anyway, so no worries about it being bad on mobile. |
|
@jancborchardt mentionned in the sidebar PR that all file actions should be available for the overview page. I'm not sure yet whether it will work, but I'll have a try. With a bit of luck it will work directly as we're providing file name and path to the API. @jancborchardt could you please also have a look at the other design/UX questions at the top ? |
|
Also actions like opening the text editor from this page might not work correctly as it will expect the breadcrumb to exist. Not sure though, need to test first. |
|
Regarding question 2.:
I think we should group it. If we show all file actions then the share-action will contain all the different shares like you are used to it from the normal files view. |
|
|
One remaining question: If we group them, should we still indicate somehow how the file was shared directly in the list so that you can get a easy overview and later maybe even sort them? Then the question would be how to display it? Extend type to a list which can contain any combination of "user", "group", "link"? |
|
I wouldn't want to add an expandable box in the first version as it makes everything more complicated to implement. I think we should display the "Share" action in the same way as the regular file list. There wouldn't be any sorting any more because we'd remove the "share type" column when grouping. |
|
@PVince81 the breadcrumbs should be there – cause you can navigate down into folders. The only thing which is not needed in the shared view is the »New« and »Upload« buttons. To answer your questions:
Ideally, as two separate entries in the Files sidebar. For now, the 2 buttons are ok. Ideally make it look like a toggle (like the database chooser on installation)
Of course. In »Files« it’s a single file as well. The row should be exactly the same as in Files – not reduced in functionality or different. Otherwise that’s highly confusing.
All of them. Otherwise if I want to delete a file or rename it, I have to switch the view and then find the file again. That would make it highly annoying to work with the shared view. If we introduce the view we should make it right, not half-useful.
Clicking on a file should open it. If that’s not possible … yeah, going to the files app is ok.
Yes.
There should just be the same share dropdown.
Delete, yes. For a file which is the share root: If you are the owner, it will delete the file. If you are not the owner, it will unshare from you.
No, because the type (people, public) should be indicated by the icon next to the »Shared« text. Again, just like in files.
Flat list for now. Showing the full path on hover is ok, yes. (not needed on mobile really so hover is ok) TL;DR: Make it a filter of the files app list and have the exact same functionality as there! If we have different actions it will be highly confusing. We made that mistake for the mobile (iOS) app and it’s just not nice to use when you have to constantly switch views to get to what you want. |
In the "Shared with me" view you will never be the owner by definition. 😉 |
Right, I agree. |
|
@jancborchardt I don't like the idea of the breadcrumbs, navigating into a folder from that view. It will make the whole logic even more complicated as the "root" is not the real root.
In which case I assume that you agree to delay this feature to the next release. I usually try to think in phases instead of providing everything at once. But with a bit of luck everything might work okay already. |
|
I'll have a look at the other comments later, was busy with the sidebar. |
|
@PVince81 well, redirecting to the folder in the Files app is ok for now. I guess we can always iterate. I was just under the impression that the concept was not really thought through: The end result should be that the actions available in the Shared views are the same as in Files. |
|
True, sorry. It was late yesterday when I read your comment. As soon as I'm done with the sidebar I'll rebase this branch on top of the sidebar so you can see it in action. |
A context hash is now passed to file action handlers which makes it possible to have file list specific file actions.
FileActions can now be clone to be use for separate file list views without having the side-effect of affecting the main file list view. Added "Open" action in sharing overview file lists to redirect to the regular file list when clicking on a folder.
Added "dir" in file actions handler context so that handlers can know
what the path of the file was without having to look it up from the file
list.
Fixed versions app to use the context.dir instead of the old $('#dir')
element. This makes the versions popup work in the sharing overview.
- Removed file size from file summary in sharing overview - Fixed document title - Fixed empty content text for shared overview
Legacy file actions are registered by legacy apps through
window.FileActions.register(). These actions can only be used by the
main file list ("all files") because legacy apps can only deal with a
single list / container.
New file actions of compatible apps must be registered through
OCA.Files.fileActions. These will be used for other lists like the
sharing overview.
Fixed versions and sharing actions to use OCA.Files.fileActions, which
makes them available in the sharing overview list.
Sometimes no icon file is passed to replaceSVGIcon(), it showed an error in IE8 and broke the code flow. This fix adds a check whether the file name is set.
Table headers should be 999 even when using links (introduced by the sorting feature) When selecting with checkboxes, they must appear black.
- Fixed renaming and fileActionsReady event - Added unit tests for shares list - Fixed public page with defer - Fixed file actions in sharing overview - Fixed sharing counterpart list (10 entries max) - Fixed file path attribute to be used in download action - Fix sharing list headers - OC.Share icons now operate on fileList instance - Fix OC.Share.updateIcon when more than one list in DOM
OC.Share can be used in non-files apps, so the fileList callback needs to support that as well.
|
The test failure is related to changes in the sharing API calls, I'll have a look. |
In some cases (like in the unit tests) "file_target" is not set yet whenever the target file system hasn't been mounted yet.
|
A new inspection was created. |
|
Rebased and fixed the unit test warning. |
|
The inspection completed: 23 new issues, 3 updated code elements |
|
Code seems good, tested 👍 |
|
🚀 Test Passed. 🚀 |
|
Awesome 👯 |
@owncloud/designers please try it out and give some input about how the list/table should be displayed.
Questions:
CC @DeepDiver1975 @karlitschek @craigpg @schiesbn