-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Post Editor: Update publish flow #60456
Conversation
Size Change: -3.64 kB (0%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
You've went a bit too far than I expected in this PR :) but it's fine. My thinking was that we could start by just using the "post status" panel from the site editor in the post editor and adapt the publish flow: (only allow publishing when draft and have a generic "save" button for all the other cases). I think having this updated "post status" panel is fine but I don't think it works without also adapting the save buttons. |
How can I help you here? Should we start the review, wait? |
You mean because of the mentioned tasks? Because the UI was mostly copied from site editor and adjusted..
Thank you and I might need some help with the update of the publish button. Let me make the first changes and I'll ping you if needed for help 😄 |
Agree with Riad about the buttons.
Probably fine to do that separately. There is an edge case quirk with draft/publish date though. If you edit a draft with a publish date in the past, when you switch the status to published the chip says "Published x ago". I suppose technically that's correct, but it's a bit unexpected: date.mp4If we're going to leave publish date as a separate field here, then the status chip should probably omit the publish time for now. One other adjustment to make here: since it's possible to set the status to pending in the status popover, we can probably remove the 'Pending review' checkbox from the summary panel. Related: if it's not out of scope, I'd be tempted to move the 'Sticky' option for posts to the status popover too, this could add an icon to the chip like password protecting. I'll push some style tweaks. |
@jameskoster What do you mean? |
The part about adapting the save buttons.
One other observation; since visibility is managed in the status popover, we can remove the duplicative "Visibility" UI. |
Using the We could add the radio item descriptions into the RadioControl API, but ultimately we want to replace RadioControl with a new composable RadioControlV2 component based on Ariakit so it will also support use cases like your extra input field. |
I've pushed some updates and maybe it's time to start reviewing and testing. Testing should be thorough including any possible flow. Examples:
I'm having a hard time to follow through all the complexity we have in this flow and we should definitely try to simplify, but let's do in steps. |
@jameskoster when we can |
The first thing I noticed is that I don't see the "save draft" button when I start a new post and type some content. I think it should be there right? |
This is more of a design/accessibility feedback. I think it's better to move the "pill" to the right and add a "status" label (like the other rows in the summary panel) cc @jameskoster |
Do we really need the date there since the date is right under (where you can actually edit it) |
I won't argue more, I personally still prefer the clarity of separate things but I don't mind this direction too much. |
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
bc43c0e
to
52536a6
Compare
I think this is ready for final testing and review. Thanks!
Since I removed the effect to update the status, the publish button label is affected only on some cases('schedule/publish'), but the chip with the current status remains the same. |
So there's a question about whether the "chip" text should update (without actually updating the status) to say "scheduled" if you set the date in the future (for a published post) and whether it should say "published" (for a scheduled post) when you set a date in the past. That said, it doesn't really feel like a bug, it's more of a choice to make and the choice of showing the status that is actually "set" doesn't bother me either. |
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 like this a lot, it simplifies the publish flow IMO. That said, it does require more scrutiny since it's a critical UI. So I'd appreciate more testing as well.
A slight quirk in the flow; if you edit a draft, switch the status to pending, the primary button becomes "Save". Shouldn't that button remain "Publish", and the "Save as pending" button become active? The same is true for the inverse (edit a pending page and switch to draft). |
Pushed some chip styling adjustments. |
Flaky tests detected in 2ecb498. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8707463653
|
@jameskoster I addressed the following:
I haven't addressed:
In general is not great that we update attributes in different controls and probably is better to explore this in the follow ups with the data control inside post status dialog. The PR is already big enough the same issue is on trunk with the date. You can test by having a scheduled post(future date) and from |
I missed that.. I had updated based on this: #60456 (comment) and the above comment. So when we update the status from the dialog after we save, then the save draft/save as pending buttons render again. Is this something you have strong opinion about? Additionally now that we can manually update the status, if we do update it I find the flows of having two buttons too confusing to be honest. |
Thanks, it's getting close, and may indeed be merge-able.
It's not a super strong opinion, and tbh it's such a niche flow that it may not really matter. But it's basically a question of consistency. Since draft and pending statuses are both unpublished, I'd expect the button configuration to be the same. pending.mp4In the video above, it's a little strange to me that I have to save after switching to pending before I can publish. If it'd add a lot to the PR to adjust this, I'd be fine with exploring it separately. Like I said, it's fairly niche and what we have isn't a big problem. |
Agree with @jameskoster it's a better to be consistent between "drafts" and "pending" but I also agree that it could be done separately. |
So I'm going to land this and explore separately the draft/pending flow and other simplifications we can make. Thanks everyone! |
What?
Part of: #59878
This PR aims to update the publishing flow in post editor, so we can later unify it with site editor.
Tasks
From the above designs I'm not sure we can add another control(publish date) in between the radio control options. --cc @jameskoster @mirka
publish
buttonsave
label and save panel when multiple entities have changedvisibility
panelTesting Instructions
Testing should be thorough including any possible flow. Examples:
published to draft
, etc..draft or pending
post and usesave as draft
orsave as pending
buttons in combination with other changes and changes in other entities.save/publish
button