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

Delete old versions: Add confirm prompt to avoid accidental delete #773

Conversation

BigRoy
Copy link
Collaborator

@BigRoy BigRoy commented Jul 12, 2024

Changelog Description

Add informative confirm prompt for delete versions to avoid accidental clicks deleting versions and files.

Additional info

image

With "Show details..." clicked:

image

Testing notes:

  1. Open Loader from Tray
  2. Use Delete Old Versions action
  3. Deletion should be confirmed (and do nothing when cancelled!)

@ynbot ynbot added size/XS type: enhancement Improvement of existing functionality or minor addition labels Jul 12, 2024
Copy link
Member

@moonyuet moonyuet left a comment

Choose a reason for hiding this comment

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

When I click on delete old version:
it shows the popup dialog as expected
image
if I hit Cancel, it won't error out
If I hit Ok, it would keep the latest 45 versions as expected.
image

I also tested with Calculate Old Version, it also works smoothly without error.

@BigRoy
Copy link
Collaborator Author

BigRoy commented Jul 15, 2024

@iLLiCiTiT could you provide this a quick glance and merge if you're ok with it?

@iLLiCiTiT
Copy link
Member

iLLiCiTiT commented Jul 15, 2024

I would rename Yes to Delete (if possible you can set object name of delete button to #DeleteButton), and moved Show details to most left.

EDITED:
Oh, it's built-in dialog...

@BigRoy
Copy link
Collaborator Author

BigRoy commented Jul 15, 2024

I would rename Yes to Delete (if possible you can set object name of delete button to #DeleteButton), and moved Show details to most left.

This would 'adhere' to the rules of the current OS - and hence it may look different on Mac OS or Linux if the "Show details" behavior is different there because it's default Qt message box behavior. I'd rather not touch it.

@iLLiCiTiT
Copy link
Member

This would 'adhere' to the rules of the current OS - and hence it may look different on Mac OS or Linux if the "Show details" behavior is different there because it's default Qt message box behavior. I'd rather not touch it.

I'm not good at UX, but this doesn't look nice. I had to read the text 2 times to understand what would the buttons do, and was confused why is the biggest button showing details between Yes and Cancel, and still don't understand why it's there (not as a developer but as user).

@BigRoy
Copy link
Collaborator Author

BigRoy commented Jul 15, 2024

I'm not good at UX, but this doesn't look nice. I had to read the text 2 times to understand what would the buttons do, and was confused why is the biggest button showing details between Yes and Cancel, and still don't understand why it's there (not as a developer but as user).

I see what you mean - yet at the same time - it does allow us to not having to write dedicated widgets and rely on what apparently are more OS native designs for this.

I could also just remove the details information - but I felt since it was now simple using this feature that it's nice 'extra check' someone can do to decide whether it's deleting what they wanted?

@m-u-r-p-h-y from artist perspective? Would you like me to redesign the pop-up dialog? See screenshots in PR description.

Turning this around into a dedicated widget - with maybe a list or items may take me maybe 30m-60m.

Copy link
Member

@m-u-r-p-h-y m-u-r-p-h-y left a comment

Choose a reason for hiding this comment

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

Delete dialogue is working fine and prevents from accidental deletion of files.

The UI itself needs a lot of love though.

@m-u-r-p-h-y m-u-r-p-h-y added the sponsored This is directly sponsored by a client or community member label Jul 15, 2024
@BigRoy
Copy link
Collaborator Author

BigRoy commented Jul 15, 2024

Details page is not taking the Remove Publish Folder into account

@m-u-r-p-h-y Would you like the details tab to just report that it's also removing the publish folder - or what kind of output did you expect additionally there?

By the way - I'm fine with merging (so that it's in the client's hands asap) and then polish the UI cosmetics in a follow-up PR.

@iLLiCiTiT iLLiCiTiT merged commit 8be1027 into ynput:develop Jul 15, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XS sponsored This is directly sponsored by a client or community member type: enhancement Improvement of existing functionality or minor addition
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants