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

feat(files_sharing): confirm share deletion #47063

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

printminion-co
Copy link
Contributor

@printminion-co printminion-co commented Aug 6, 2024

Summary

Currently, it is possible to remove a share without confirmation.

This is problematic if the share link has already been shared with users. The share link is deleted permanently, and it cannot be restored.

To prevent accidental share deletion, this feature adds a confirmation dialog (see screenshot).

New confirmation dialog on share delete

Selection_20240806-001

Checklist

@printminion-co printminion-co force-pushed the mk/feat/delete_share_confirmation branch 3 times, most recently from 4e22b5b to ecdbc77 Compare August 7, 2024 14:24
@printminion-co printminion-co marked this pull request as ready for review August 7, 2024 14:24
@AndyScherzinger AndyScherzinger added enhancement design Design, UI, UX, etc. 3. to review Waiting for reviews feature: sharing labels Aug 7, 2024
@AndyScherzinger
Copy link
Member

looping in @jancborchardt since we usually (or never?) have confirmation steps on actions also not no destructive ones.

@printminion-co
Copy link
Contributor Author

looping in @jancborchardt since we usually (or never?) have confirmation steps on actions also not no destructive ones.

One can see confirmation dialog on deletion of 5+ files/folders.

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice improvement! Just the wording should be adjusted:

  • The title needs to be "Delete share" like the action that wqs just clicked, and like the primary confirmation button.
  • The content needs to include the name of the share and who it is unshared from, like: "Pictures will be unshared from Andy Scherzinger."
    • The name of the person should use the avatarbubble component.

@skjnldsv skjnldsv mentioned this pull request Aug 20, 2024
5 tasks
@skjnldsv skjnldsv added this to the Nextcloud 31 milestone Aug 20, 2024
@skjnldsv skjnldsv self-assigned this Aug 20, 2024
Copy link
Contributor

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

@jancborchardt
Copy link
Member

@printminion-co do you need any help besides the feedback @skjnldsv and I gave? :) No pressure from our side, just asking if there is any blocker.

@skjnldsv
Copy link
Member

@printminion-co if you are lacking the time, I can take over next week

@printminion-co printminion-co force-pushed the mk/feat/delete_share_confirmation branch 2 times, most recently from db47597 to 87adcec Compare August 22, 2024 09:44
@printminion-co
Copy link
Contributor Author

@printminion-co do you need any help besides the feedback @skjnldsv and I gave? :) No pressure from our side, just asking if there is any blocker.

I was in vacation ;)
...I've applied proposed code changes

@printminion-co printminion-co force-pushed the mk/feat/delete_share_confirmation branch from 87adcec to 70e27dd Compare August 22, 2024 15:02
@printminion-co
Copy link
Contributor Author

  • The content needs to include the name of the share and who it is unshared from, like: "Pictures will be unshared from Andy Scherzinger."

Added File/folder name (without user reference) with similar text in deletion confirmation
Selection_20240822-001
Selection_20240822-002

CC @AndyScherzinger

@printminion-co printminion-co force-pushed the mk/feat/delete_share_confirmation branch from 70e27dd to 725d4d3 Compare August 22, 2024 15:24
@AndyScherzinger
Copy link
Member

Added File/folder name (without user reference) with similar text in deletion confirmation

Looks good to me, any confirmation dialog is somewhat better than none so if technically fine I vote to merge it. Cc @sorbaugh

in order to prevent accidental share deletion

Signed-off-by: Misha M.-Kupriyanov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants