-
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
Improve "switch to draft" placement #50217
Conversation
Move button next to the "move to trash" action in post settings and out of the prominent header placement.
44a2c30
to
483df1a
Compare
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.
Makes sense to me. A published post still auto-saves when published, and grouping two destructive actions next to each other makes sense:
Does "trash" actually need to be marked red? Trash unpublishes and puts it in trash. It doesn't empty the trash. Switch to draft also unpublishes. To an extent, they are equally destructive, which is not a lot.
I approved it since the buttons are the same. But should we move these buttons to using the new default 40px footprint? And while you're there, should the Author dropdown also move to the default 40px footprint, like so? CC: @ciampo in case you have a better path forward so we can make progress on #46734. There are a lot of 36px buttons that should be 40px, like these: The padding in that modal is also a bit off on the top-side. And there's a mix of sizes here: That disabled state, is that the default one? If so we can probably do better. Let me know if you want me to open issues for any of these separately. |
Size Change: +58 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
Yes, a lot of cleanup to do, but let's do them all separately, if possible. There are also some tests that likely need to be updated with this move. |
I think we can switch away from the red button, but it stands out now that we display a confirmation step for switch to draft but not for move to trash. We should have a confirmation step for both. The move to trash could maybe even include an actual red button in the dialog that says "delete permanently", which we don't expose now |
|
||
export default function PostTrash() { | ||
return ( | ||
<PostTrashCheck> | ||
<PanelRow> | ||
<FlexItem isBlock> |
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.
It feels like these "FlexItem" elements should move into the PostStatus
component instead of keeping them within the reusable PostTrash
and PostSwitchToDraftButton
components?
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.
These are not really reusable, I think — it already had a "PanelRow". Moving the flex item to PostStatus means lifting up the logic for display or not displaying each of these, otherwise you end up with an empty flex element that still takes space
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.
Ok 👍
0de5e03
to
b595fea
Compare
Flaky tests detected in b595fea. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4852140066
|
@@ -106,10 +103,6 @@ export default function PostSavedState( { | |||
return null; | |||
} | |||
|
|||
if ( isPublished || isScheduled ) { | |||
return <PostSwitchToDraftButton />; | |||
} |
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.
Looks like this change is causing a small bug. Now for "published posts", the button is there and it says "Save draft", should we return null
here instead?
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.
Opened this #51193 to fix this issue.
isSaveable, | ||
isSaving, | ||
isScheduled, |
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.
Also we removed these two return values but forgot to remove the keys within the useSelect callback.
@jasmussen With #51012 merged, folks can use the |
Fantastic. Let's get on that especially with the top toolbar at first. |
Move the "switch to draft" button next to the "move to trash" action in post settings and out of the prominent header placement.