-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Show confirmation dialog when moving a post to the trash #50219
Conversation
Size Change: +16.6 kB (+1%) Total Size: 1.64 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, it's actually a little weird we don't have one, since we do have one for unpublishing:
Per your comment here, I think we have an opportunity to move away from the red color given there's a confirmation, and given it's actually moving the item to trash, not deleting it forever and ever.
onCancel={ () => setShowConfirmDialog( false ) } | ||
> | ||
{ __( | ||
'Are you sure you want to move this post to the trash?' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably avoid the term "post" or contextualize with whatever post type is the current context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. Current context would be best here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we consider this as a blocker? cc @jasmussen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree current context would be best, but since we are currently shipping in trunk without the warning, it seems an improvement to add one as a first step, but we can just rephrase to this:
Are you sure you want to move this to the trash?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just checked, and "switch to draft" also uses the "post" term in the confirmation message. I think we can merge this as it is and contextualize both in follow-up PR.
f9fc21e
to
5f99550
Compare
Followed up with a padding fix in #50283. |
I can't speak for Matías, but I know he's endlessly busy and travelling right now, and with a green check I think we can just rebase this one and help land it. |
@jasmussen, sounds good. I'll rebase and merge this in a bit. |
5f99550
to
d271ec2
Compare
Flaky tests detected in 8360ac7. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6409042477
|
Working on #50217 it became apparent that we show a confirmation step for switching a post back to a draft state but we don't show one for moving a post to the trash bin.
Fixes #19125.
Fixes #34436.