Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable Full Sharing (user/group) from Desktop #3737

Closed
MTRichards opened this issue Sep 2, 2015 · 27 comments · Fixed by #4553
Closed

Enable Full Sharing (user/group) from Desktop #3737

MTRichards opened this issue Sep 2, 2015 · 27 comments · Fixed by #4553
Assignees
Labels
Design & UX p2-high Escalation, on top of current planning, release blocker ReadyToTest QA, please validate the fix/enhancement
Milestone

Comments

@MTRichards
Copy link

Desktop Full Sharing
As an ownCloud user, I want to be able to use the ownCloud internal sharing mechanism from Windows explorer so that I can quickly and easily share files with other ownCloud users without using share links.

Acceptance Criteria

  • User should right click on the file and see the current share with menu, and then be able to select a new option “share within ownCloud”
  • Share within ownCloud opens up a new window on the desktop client
  • In this case, the user can enter a group name or a user name (or part of either) and click a button to search for the user or group. Note: this is different than the current web auto search interface. We want to be certain we are not flooding the server API with false starts, so the search now button is important
  • While the API is searching, the desktop window shows a little “searching” icon that shows the system is searching and in progress (@janborchart for specific look and feel)
  • There is a cancel button for this search. However, while one search is running a second cannot be initiated to avoid flooding the server with too many requests
  • When the search returns group or usernames (in the same order as on the server, no change in behavior on the search results) and a user can select from the drop down list
  • At most 40 results are returned. If more results are returned, the dropdown shows the 40 but notifies the user that the user and group search returned too many results, and that the search needs to be narrowed.
  • When a user or group is selected, the standard CRUDS capabilities are lined up.
  • This assumes that the complete desktop share with link option is already implemented, including setting a password, setting a password that meets policy,setting expiration.
  • This also assumes that the “notify user via email” option currently on the server is also implemented on the desktop
  • Should link to the server user and group search issue for 8.2 - dependency
  • Should link to mobile sync design for consistency

@dragotin @danimo

@MTRichards MTRichards added this to the 2.1-next milestone Sep 2, 2015
@MTRichards
Copy link
Author

@rullzer
Copy link
Contributor

rullzer commented Sep 3, 2015

Now that the Sharee API is in core I can revive some old code. Cool to finally implement this on the desktop.

Just to be clear. We want to have this in a different window?
Not 1 window wit 2 tabs or whatever?

@karlitschek
Copy link

this will be a huge step forward! 👍 :-)

@phil-davis
Copy link
Contributor

+1 also
And I will just mention that the group/user sharing dialogs in the client will need to respect the admin server-side settings of sharing restrictions, like:

  • Allow apps to use the Share API
  • Allow resharing
  • Restrict users to only share with users in their groups
  • Exclude groups from sharing

That will avoid users on the client trying to do something and then the server spitting back a "not allowed" message.

@rullzer
Copy link
Contributor

rullzer commented Sep 3, 2015

@phil-davis yes good points!

  • Allow apps to use the Share API

Well this basically boils down to if sharing is enabled or not. Since we will move to OCS Share API for the webfrontend as well.

  • Allow resharing

Already available in the capabilities

  • Restrict users to only share with users in their groups

Availble in the sharee api. Basically it only returns valid suggestions

  • Exclude groups from sharing

Probably makes sense to get this into the capabilities as well. So if a user can't share the query of capabilities from the server for him/her shows up as sharing is disabled. @DeepDiver1975 does a separate capability make more sense?

@MTRichards
Copy link
Author

Cross link to Mobile issue so the UI looks similar: owncloud/ios-legacy#425

@MTRichards
Copy link
Author

@jancborchardt need some design thoughts here.

@MTRichards
Copy link
Author

And, @phil-davis :

Allow apps to use the Share API
Allow resharing
Restrict users to only share with users in their groups
Exclude groups from sharing

Spot on.

@petervcook
Copy link

+1 on Mac as well.

@jancborchardt
Copy link
Member

Design-wise as said before, please use the server sharing as blueprint. That’s basically the master living mockup of sharing. There can be contextual enhancements, as for example on mobile. But on desktop I see this being pretty much the same as on web.

@ckamm ckamm added the p2-high Escalation, on top of current planning, release blocker label Sep 30, 2015
@dragotin dragotin added the ReadyToTest QA, please validate the fix/enhancement label Nov 11, 2015
@dragotin
Copy link
Contributor

@rullzer could you please add some test description for @Dianafg76 in https://github.com/owncloud/client/wiki/Testing-Scenarios-2.1 ? thanks

@rullzer
Copy link
Contributor

rullzer commented Nov 11, 2015

Yes that is on my todo.

@rullzer
Copy link
Contributor

rullzer commented Nov 11, 2015

The list is updated. @Dianafg76 let me know if you need info/help on how to test. And feel free to mention me in the issues you find directly.

@Dianafg76
Copy link

@rullzer Thanks

@jancborchardt
Copy link
Member

Some design feedback which I also talked to @rullzer about already when he showed me the dialog:

  • the »Shares« text is superfluous and confusing when there are users in the list. We don’t need it.
  • the permissions should be simplified just like in the web interface. The important ones are »can share« and »can edit«. With that wording, the label »Permissions« is also not needed.
  • the permissions should be directly next to the username, floated to the right (left of the delete button), just like in the web interface. Otherwise it’s not clear where they belong to.
  • actually the search button is not needed when we add a small timeout before the request is sent to the server, like 500ms or a second. Currently it’s really cumbersome to share due to needing to press the search button. cc @MTRichards this is quite important for UX
  • the delete icon is missing on the delete button – just use the one from core (SVG in same folder) https://github.com/owncloud/core/blob/master/core/img/actions/delete.png
  • there should be a loading spinner for feedback when adding a new person, in place of where the entry will be (at the bottom of the list)
  • when unsharing from a user, currently it almost glitches out of the list. If possible there should be a nice animation which compresses the height and then removes it.
  • the dividing line between user/group sharing and link sharing is too dark. 50% grey would be good.

Future stuff:

  • add avatars to the left of the username, just like in the web interface. Same for the suggestion dropdown.
  • add a dropdown next to the permissions to also expose the detailed settings like »Delete/Create/Update«.

@rullzer
Copy link
Contributor

rullzer commented Nov 13, 2015

I started work on the UI fixes in #4133

@guruz
Copy link
Contributor

guruz commented Nov 17, 2015

@ogoffart ^^ I think you did some of those checkbox items yesterday and on friday..

@guruz guruz changed the title Enable Full Sharing from Desktop Enable Full Sharing (user/group) from Desktop Nov 18, 2015
@rullzer rullzer added the ReadyToTest QA, please validate the fix/enhancement label Nov 25, 2015
@guruz
Copy link
Contributor

guruz commented Nov 25, 2015

@Dianafg76 If it works nicely you can move to 2.1.1 so we can do the further checkbox items that @jancborchardt mentioned above :)

@jospoortvliet
Copy link

Just tested with testpilot client beta 1. Three things:

  • Minor: It doesn't seem to pick up on changes on the server very quick and there is no refresh option. I actually just quit and restarted the client to see things show up. Also, avatars would be really really nice.
  • what in the name of $DEITY is the 'Sync Protocol' tab??!? It seems generally empty for me and I have no idea what to expect to show up there. It is for Sync Status, perhaps? Use that, the word 'protocol' ensures nobody has any idea what to expect there...
  • can't figure out where that desktop integration should be. Running the latest of everything...

@dragotin
Copy link
Contributor

@jospoortvliet I am not sure what you intend with your post here. I am not seeing what that has to do with "desktop sharing", or I misunderstand completely.

@rullzer
Copy link
Contributor

rullzer commented Nov 25, 2015

@jospoortvliet

  • Minor: It doesn't seem to pick up on changes on the server very quick and there is no refresh option. I actually just quit and restarted the client to see things show up.

Well we do not have feedback from the server. So the only way would be to poll. But even then you look at an outdated version most of the time. And adding polling really is a burden once the number of users get larger.

  • Also, avatars would be really really nice.

Yes, but we do not have avatars available via an external endpoint yet. So patenince my friend. 😄

  • what in the name of $DEITY is the 'Sync Protocol' tab??!? It seems generally empty for me and I have no idea what to expect to show up there. It is for Sync Status, perhaps? Use that, the word 'protocol' ensures nobody has any idea what to expect there...

????

  • can't figure out where that desktop integration should be. Running the latest of everything...

You need the dekstop intergration stuff running.

@jospoortvliet
Copy link

@rullzer ok, cool - I guess then that once the activity API is improved this can be done quite easily.

About the Sync Protocol tab, it's in the activity page, there are three tabs. One server side activity and one about sync issues. One is named 'Sync Protocol' and I'm not sure what it is for or what it even means. Well, I know what it means but don't know what to expect on that tab.

And yeah, I figured out why I didn't see the desktop integration, my fault.

@dragotin sorry for polluting this issue with client feedback.

@jancborchardt
Copy link
Member

About the Sync Protocol tab, it's in the activity page, there are three tabs. One server side activity and one about sync issues. One is named 'Sync Protocol' and I'm not sure what it is for or what it even means. Well, I know what it means but don't know what to expect on that tab.

This is exactly why I commented on #4083 (comment) that this new 3-tab-view is too complicated. It’s technical wording and too detailed. We should reopen #1443 and work on a proper solution. cc @MTRichards @dragotin

@Dianafg76 Dianafg76 modified the milestones: 2.1.1-nextpatch, 2.1-current Nov 27, 2015
@Dianafg76 Dianafg76 removed the ReadyToTest QA, please validate the fix/enhancement label Nov 27, 2015
@guruz guruz modified the milestones: 2.2-nextminor, 2.1.1-current Jan 7, 2016
@MTRichards
Copy link
Author

In addition to the incomplete checks above, we also need to implement the sharing logic for when a display name of a user is the same. owncloud/core#20291 has the server side response, we need to reflect that in the desktop.

Also not sure we do this. If we do, great, otherwise we need parity:

  • Don't show the switch to disable/enable password on a public link if password is mandatory (prevents error message if user tries to disable password)
  • Don't show the switch to disable/enlable expiration date on a public link if mandatory
  • Don't allow the user select dates later than the maximum expiration date set in the server, if mandatory
  • Support federated sharing by including (remote) next to the username as it is done on the web interface. The remote users won't be shown if this is not enable via the capabilities api

And that closes out this feature set.

@rullzer
Copy link
Contributor

rullzer commented Feb 23, 2016

there should be a loading spinner for feedback when adding a new person, in place of where the entry will be (at the bottom of the list)

I'll add that

add avatars to the left of the username, just like in the web interface. Same for the suggestion dropdown.

We can't do this yet since the server did not support it.

@rullzer
Copy link
Contributor

rullzer commented Feb 23, 2016

@MTRichards #3737 (comment) that is all in already :)

I'll fix the final part we can add (the spinner) soonish.

@rullzer
Copy link
Contributor

rullzer commented Mar 8, 2016

Avatar issue in #4554
PR for spinner in #4553

@rullzer rullzer added the ReadyToTest QA, please validate the fix/enhancement label Mar 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design & UX p2-high Escalation, on top of current planning, release blocker ReadyToTest QA, please validate the fix/enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.