Replace share settings popup with a page on a StackView

[claucambra]

Screen.Recording.2022-11-23.at.15.23.55.mov

Closes #4945

[claucambra]

Note that big diffs are mainly due to reparenting contents of share settings popup to new file

[codecov]

Codecov Report

Merging #5194 (b682794) into master (6db9c95) will increase coverage by 0.07%.
The diff coverage is n/a.

❗ Current head b682794 differs from pull request most recent head 2261615. Consider uploading reports for the commit 2261615 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5194      +/-   ##
==========================================
+ Coverage   57.63%   57.71%   +0.07%     
==========================================
  Files         138      138              
  Lines       17490    17441      -49     
==========================================
- Hits        10081    10066      -15     
+ Misses       7409     7375      -34     

Impacted Files	Coverage Δ
src/libsync/account.h	33.33% <0.00%> (ø)
src/libsync/discovery.h	59.25% <0.00%> (ø)
src/libsync/syncengine.h	50.00% <0.00%> (ø)
src/libsync/discovery.cpp	88.24% <0.00%> (+0.46%)	⬆️
src/libsync/account.cpp	37.15% <0.00%> (+0.84%)	⬆️
src/libsync/syncengine.cpp	83.33% <0.00%> (+1.68%)	⬆️

[jancborchardt]

Nice! :) Could you adjust the buttons on the bottom to resemble the mockup more:
nextcloud/server#26691

• "Delete link" as part of settings list, text-only button
• "Add another link" same thing (forgot in the mockup)
• Bottom buttons should be "Cancel" on left and "Share & copy link" on right.
• Clicking on "Share & copy link" should also move back to the main overview

[claucambra]

Nice! :) Could you adjust the buttons on the bottom to resemble the mockup more: nextcloud/server#26691

• "Delete link" as part of settings list, text-only button
• "Add another link" same thing (forgot in the mockup)
• Bottom buttons should be "Cancel" on left and "Share & copy link" on right.
• Clicking on "Share & copy link" should also move back to the main overview

Sure, will do. However, my 2 cents: text-only buttons are a bad idea, especially in the way we use them in the client. They look like links, and links are supposed to take you places. Buttons perform actions. Deleting a share is an action and should therefore be a proper button

Edit: corollary -- if we add a "cancel" button, should we be getting rid of the close button at top? Or should this close the whole drawer when it is displayed within the tray window? At the moment these two buttons would make each other redundant

Edit 2: I am confused by the text "Share & copy link" -- what exactly is this supposed to mean? Shares will always already be created when accessing this new page. What about non-link shares (i.e. user shares)? Should this button not be included?

Edit 3: Do we want to move the "Add another link" button to the bottom of the list? This ruins the risk of it being hidden by other share fields above it, forcing users to scroll when they might not be interested in editing a share at all. Seems like bad UX?

Edit 4: Related to Edit 3, we already display a button on the main share list to copy the link for a link share, seems redundant to add another one to the main share details page maybe?

[nimishavijay]

Sure, will do. However, my 2 cents: text-only buttons are a bad idea, especially in the way we use them in the client. They look like links, and links are supposed to take you places. Buttons perform actions. Deleting a share is an action and should therefore be a proper button

I would have to agree with you here, regular text and underline looks like a link. Is it possible to have custom styling for text-only buttons (bold text, no underline, center aligned with the associated icon, for this particular button (Unshare) the color would be red)? Or we could use a button with a white background (or dark in dark mode) and red text and icon. Are any of those possible?

The issue with the existing design it that it is very present and the placement makes it look like a secondary action. if we could move it to be the last item on the list and change the styling to be not as present, that would be an improvement.

Edit: corollary -- if we add a "cancel" button, should we be getting rid of the close button at top? Or should this close the whole drawer when it is displayed within the tray window? At the moment these two buttons would make each other redundant

I think we don't need a cancel button since the close button is there on the top right :)

Edit 2: I am confused by the text "Share & copy link" -- what exactly is this supposed to mean? Shares will always already be created when accessing this new page. What about non-link shares (i.e. user shares)? Should this button not be included?

Maybe better wording would be "Copy share link" for link shares. If it's not a link share it doesn't have to be included like you mentioned, there could just be a button that says "Save" or "Back" or something of that sort.

Edit 3: Do we want to move the "Add another link" button to the bottom of the list? This ruins the risk of it being hidden by other share fields above it, forcing users to scroll when they might not be interested in editing a share at all. Seems like bad UX?

I would say so, yes. I would think it's unlikely that there are so many share links that the "Add another link" button is not visible, so in most cases it is okay. The action of adding another link is not very related to editing a share, so it makes sense to move it outside to the list of shares :)

Edit 4: Related to Edit 3, we already display a button on the main share list to copy the link for a link share, seems redundant to add another one to the main share details page maybe?

For a share link copying it and sharing it is the most frequently used button, so I think duplication is okay. It would be nice to edit a link share and immediately copy the link without navigating to another page :)

[claucambra]

@nimishavijay what do you think of this? 🙂

Screen.Recording.2022-11-23.at.15.23.55.mov

[jancborchardt]

Looks great @claucambra! :) Just one clarification:

I am confused by the text "Share & copy link" -- what exactly is this supposed to mean? Shares will always already be created when accessing this new page. What about non-link shares (i.e. user shares)? Should this button not be included?

So actually this is one thing that the spec at nextcloud/server#26691 was specifically intended to fix. Right now we immediately create the share:

It’s not clear to people when the share will be set & sent, leading to possible access control issues, and also 2 separate mails when you set a note

So this is why this new flow specifically has the "Share & copy link" button. Only on clicking that should the share actually be created. However we should probably do that at the same time in server and mobile, and looking at the issue I don’t think it’s done in either cc @PVince81 @tobiasKaminsky.

[jancborchardt]

Looks nice! :)

[nextcloud-desktop-bot]

AppImage file: nextcloud-PR-5194-2261615f6919564cec31990db9a5f2decee90856-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[wkhj]

Our users reported missing features by this change,
they are unable to grant upload permission from the desktop client as the options changed, so the users are forced to use the web interface for sharing.

Previous features (for example 3.3.2) were

• Read-Only
• Upload and Edit
• Only Upload

Current features are only

• Read-Only (Standard)
• Edit existing files (no upload)

[jancborchardt]

Right! @claucambra regarding @wkhj's comment, we should add the top 3 radio buttons as per mockup to stay feature complete :)
nextcloud/server#26691

[jancborchardt]

@wkhj could I ask you to open a new issue about this, so we can schedule it? Thank you! :)