Delay private publish #12023
Delay private publish #12023
Conversation
There was a problem hiding this comment.
Hi @earnjam,
I'm noticing a bug in this branch:
Create a new post, write a time, random content.
Set a publish date for the future.
Set visibility to private.
Press schedule and finish the process.
Reload the post and now the post visibility is set to public.
|
Thanks for catching that @jorgefilipecosta In the classic editor, if you set post visibility to private, then the post date didn't matter. It was live for any logged in user, and hidden (returns a 404) for any non-logged in user. This makes sense because all scheduled posts are visible for logged in users. So scheduling doesn't matter if it's private, but we should just make sure to account for the date when transitioning from private to public. I'll update the branch so that |
|
@earnjam - do you have any plans to update this patch to address feedback from @jorgefilipecosta and ensure that code is refreshed to use all the latest changes from |
gziolo
added
the
Backwards Compatibility
|
Yes, I should have some time tomorrow to work on it. When are we planning to release Gutenberg 5.1? |
|
In two weeks. |
earnjam
force-pushed
the
try/delay-private-publish
branch
from
b02f385 to
28db7b7
Compare
earnjam
requested review from
gziolo,
noisysocks,
talldan and
youknowriad
as code owners
|
I rebased |
|
Thanks for the update @earnjam Added a "Needs design feedback" label. This PR changes the flows a little bit and to be honest, whenever we touch these buttons and anything related to "post status", I freak out as it's very delicate, it affects (scheduling, autosaves, metaboxes,dirty checking...). Let's get some design eyes and if it's a good direction, let's make sure we test this PR properly. |
|
Hey @earnjam I'm sorry no one has provided a design review. I'd like to provide one, but can't get this PR running locally. Can you rebase it? I prefer the pre-publish panel to stay inline with the current publish flow. Thanks! |
There was a problem hiding this comment.
If I recall correctly the challenges around setting private visibility without publishing, I would have expected to see some more changes here to the underlying data mechanics of considering a post as "saveable" (e.g. isEditedPostSaveable). Otherwise, I suspect the autosave mechanic could still save the post in an unintended state? Likewise for Cmd+S ? It seems this should be forbidden at the data flow (the savePost action), not just hidden from the interface.
|
Hi @earnjam! I'm trying to test this branch locally but I'm running into some errors: Other than that (not sure it's related, I seem to be finding all the strange errors of late) this feels like a better solution than the current setup where selecting private visibility immediately publishes the post. 😱 One thing I noticed whilst testing is that there's some inconsistency in the way the three different visibility options behave:
Public sets the publish button text to "publish", and the status to "public". To increase consistency here, I'd recommend either: a) Keeping the text of the publish button consistent, regardless of visibility settings. My instinct is to opt for a) here, especially since the visibility is then reiterated in the pre-publish panel, which is going to be redesigned a bit to aid in user certain when publishing (see #7602 for more), but I'd love additional feedback from a user experience perspective. |
|
@sarahmonster I last updated this PR ~2 months ago, so I wouldn't be surprised if has new issues by now. I'll take a look at it. |
|
@aduth I'll look more closely at it to confirm, but since the autosave mechanic should only be saving revisions, it seems like it would be unaffected by this. I suppose for a new post that has yet to be published, autosave updates the original post, which has a status of |
earnjam
force-pushed
the
try/delay-private-publish
branch
from
affadf0 to
a81316b
Compare
|
Size Change: +13 B (0%) Total Size: 856 kB
ℹ️ View Unchanged
|
|
I've rebased master onto this branch and updated. It's got a couple of failed e2e tests that I think just need to be updated to reflect some UI changes. I'll fix those this morning, but it would be good to get design/UX feedback on this, because that's still the major holdup.
@mapk The flow in this PR is essentially the flow from the classic editor. Quick highlights:
I think the remaining question is one I posed earlier in the thread, about how to handle the switch back to draft after "publishing" the private post. It will kick the visibility back to public because you can't have a private draft. (Both are stored in Should we add a snackbar notice or something similar to make that more obvious that it has occurred? It's possible a user might switch a published private post to a draft, then publish again not realizing that it had changed to public. |
|
Size Change: +597 B (0%) Total Size: 856 kB
ℹ️ View Unchanged
|
earnjam
force-pushed
the
try/delay-private-publish
branch
from
37528a0 to
a81316b
Compare
|
Size Change: +13 B (0%) Total Size: 856 kB
ℹ️ View Unchanged
|
|
Size Change: +115 B (0%) Total Size: 857 kB
ℹ️ View Unchanged
|
|
Ok, tests are passing now. This primarily needs UX feedback, since there isn't much programmatically changed here. |
Yes! 😆 This whole system of draft vs. private vs. published leaves me in confusion every time.
Yes again. If we're switching a setting on the post that isn't obvious to the user (ie. switching a draft post back to public) we should alert the user. That's kinda a big deal. A Snackbar notification sounds like the right way to go. Can you include that in this PR, @earnjam? |
|
@mapk We already show a snackbar notice when reverting to a draft. What do we think about modifying the message in the notice to something like this? I worry it isn't visible enough. Current notice: Modified message: |
|
I should also clarify that this wouldn't be something new to this PR. Currently if you revert a |
|
Love it. Let's add that additional text to the snackbar notice! Thanks, @earnjam. |
|
I just tried testing the PR and got this:
No msg about the visibility. Is that included here? |
|
@mapk Right, that's the current version that we already have. I haven't updated it on this branch yet! 🙂 |
|
Missed the prior conversation here. The relationships between statuses is very convoluted and leads to an awkward experience. I agree with the comments about trying to improve it and work around the data and status design. That's the point of the "switch to draft" action as well, as it encapsulates something that makes sense for the user. That said, I do worry a bit about the implications for a user switching the status and then doing a lot of changes. In my testing, after saving a draft, switching it to "private", making some updates, letting the browser sit for a bit, and then reloading the page actually ended up publishing the post as private. I don't think this experience would be super great even if we address the auto-save issue that seems to be happening. Would a user have clarity on whether things are saved or published? Regarding the snack-bar notice, I'd suggest tweaking the wording to be something like "Post reverted to draft and reseted its visibility". Otherwise it could be misleading as a sentence — I reverted to draft to make it private why would you say it's set to public again?! "Public draft" reads a bit as an oxymoron. |
|
Good point @mtias. How about... "Post reverted to draft and caused the visibility to be reset." |
|
Digging into the autosave issue mentioned by @mtias, I realized it is in fact autosaving the status change. This occurs for any status that isn't already I tested putting in some logic to prevent autosaves, and that worked ok because the post is considered dirty when the status is changed. So the user can't navigate away without knowing they are going to lose something. The keyboard saves are a harder problem to solve. We currently block them at the UI level to prevent posts from being saved if they aren't considered dirty. We could add on to that, even though it really should be handled at a lower level. Regardless of where it's done though, I don't think that randomly disabling keyboard saves is optimal when the post is otherwise considered dirty, even if we were to show a notice saying it didn't save and why it didn't save. Not passing the status in the autosave would allow content work to be saved, but then it doesn't know about the visibility change if you were to restore it. I'm running out of ideas for how to handle it. Short of data storage changes, which are frankly not likely to ever occur, I'm not sure how to tackle this in a graceful way. We'd be trading one inconsistency (a sidebar control publishing the post immediately) for a new one (blocking saves under certain conditions that wouldn't make sense to users). If anyone has any other ideas I'm happy to run with them, but otherwise I think I'll bow out of this one and focus on something else. The snackbar notice verbiage change is an independent issue from this PR. That is something that occurs right now and could be solved separately. So someone can open a PR just for that change if they want. It would require a new label added to the list of post type labels, so it would need a trac ticket and patch as well. |
|
Thanks for attempting this and recapping @earnjam. That's my takeaway as well. There is a fundamental problem in the way the data is modelled and the UI will have to be trading faithfulness to the true state (clarity) with an inconvenient experience. In that sense, being upfront about what would happen (forcing to publish privately) is likely a better tradeoff since it reduces the potential for the user making a mistake. I guess we could look into storing a "fake-status" of pending-private that only stores the intention of switching the status to "private" when the user goes through the publish button flow. That would allow us to present the UI as if the status was both unpublished and private (the real status would be just "draft" and wouldn't conflict with auto-saves). This has its fair share of problems, though, since it's an ad hoc status that only the editor (so far) would understand and not the wider ecosystem — for any other external editor, app, plugin, etc, the post would be just set to "draft", which means if you publish through any mechanism that is not the editor WordPress won't necessarily know to set it as "private" at the right time, which could lead to disclosing information and a much bigger problem than the UI inconvenience we were trying to address. We could move a layer deeper and try to have this "fake status" at a lower level, and ensure all publish actions and hooks are able to account for it if it's set, but I'd imagine it will still possibly lead to issues if a client or flow somehow unawarely forces the "publish" status in a way that WP cannot intercept. If we were to explore anything like it, it'd probably have to be thoroughly thought out and presented more widely as a proposal since the current state has been the status quo for so long. |
|
I thought about possible ways to save it externally to status temporarily, but nothing seemed quite right and had the shortcomings you mentioned about other editors or external code not being aware of it. The cleanest method is just a break of backwards compatibility on |
Update: This PR no longer changes the verbiage on the Publish button per feedback. See updates below for most recent details.
Description
This is an approach to setting private visibility that doesn't immediately publish the post. Instead it changes the verbiage on the publish button and disables the save draft button, allowing a user to continue working on the post before going live.
Fixes #9396
Screenshots
With the pre-publish panel
With immediate publish