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

Add notices for when post is locked from publishing #11

Closed
wants to merge 1 commit into from

Conversation

hanifn
Copy link
Contributor

@hanifn hanifn commented Jul 15, 2024

Description

This PR adds a notice to the top of the editor when publish guard is enabled and the post is not in the last status.

Steps to Test

  1. Check out PR.
  2. Enable publish guard in VIP Workflow -> Notifications
  3. Create or edit a post that's not in Pending review
  4. Verify that there is a notice that states the post is blocked from publishing

Screenshot 2024-07-15 at 4 29 41 PM

@hanifn hanifn added the enhancement New feature or request label Jul 15, 2024
@hanifn hanifn self-assigned this Jul 15, 2024
Copy link
Contributor

@ingeniumed ingeniumed left a comment

Choose a reason for hiding this comment

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

Just a comment around the wording of the message in the notice, otherwise it looks good to me.

@@ -52,12 +52,16 @@ subscribe( function () {
if ( ! postLocked ) {
postLocked = true;
dispatch( 'core/editor' ).lockPostSaving( 'vip-workflow' );
dispatch( 'core/notices' ).createNotice(
'warning',
__( 'This post is locked from publishing due to its status.', 'vip-workflow' ),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the wording of this message could be improved. Something like

This post is locked from publishing, until it reaches pending review status.

might be better?

@alecgeatches or @smithjw1 - wordsmithing help

Copy link
Contributor

Choose a reason for hiding this comment

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

A few thoughts:

Can we hide the publish button? If you can't publish, I don't think we want it there. Maybe "Save" instead. I don't think we need any warning in this case.

If we can do 1, then I would have the warning say: "This post can not be published."

Copy link
Contributor

Choose a reason for hiding this comment

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

There is some experimentation that Hanif did with css/jQuery, but the best solution we found was taking advantage of lockPostSaving so that the second publish button was disabled via a built-in block editor way

@alecgeatches
Copy link
Contributor

alecgeatches commented Jul 16, 2024

I do like that we have a persistent way to notify the user that the post is locked, but I don't like this visual style. It takes up valuable vertical editing space and I think would be irritating for me if I was using the plugin.

I have a different proposal - what if we use a PluginSidebar as an indicator instead? This would be the same setup that wp-parsely uses:

Screenshot 2024-07-16 at 11 45 30 AM

and also Jetpack:

Screenshot 2024-07-16 at 11 46 03 AM

My thought is that the icon could dynamically show a lock and text when the post is not publishable "Publish Locked", and if you click it, shows the "This post is locked from publishing due to its status." status in the sidebar. If the post is publishable, we should show a different icon or just hide the button entirely. This could be the way to:

  • Provide a visual locked indicator near the publish button
  • Show information about the locked post
  • Avoid taking up editor space if unneeded

@alecgeatches
Copy link
Contributor

To expand on the comment above, it might even be a better UX to rely on the backend publishing guard over lockPostSaving, because it gives a clear indicator that the post isn't publishable right at the point where the user is clicking the button. lockPostSaving feels unclear about what's happening from the user's perspective because there's no feedback, just a disabled button.

@smithjw1
Copy link
Contributor

I like that UX change, if we can't hide (or change the appearance of the publish button).

@ingeniumed
Copy link
Contributor

For context, I had originally suggested this idea for a notice because there wasn't currently a way to add a tooltip to the disabled publish button or tweak the appearance in any way. This would allow some information to be provided at least, that we could always remove when Gutenberg adds something else that useable in it's place or make it configurable.

This bug that I pointed out in #9 will be a blocker in using that backend functionality over this change. From the quick search I did, it seems like this method is recommended for such situations and people do add notices as a way to let users know why it's there. Perhaps making it dismissible might be better?

I'm hesitant on adding another plugin sidebar, given we were talking about how we could unify all those sidebars down the line. That said, I do like that UX change. It'll keep the notice small, and tucked out of the way. It also gives us the added benefit of having a sidebar to use for when we want to add editorial metadata back or any other features for the block editor.

@hanifn
Copy link
Contributor Author

hanifn commented Jul 18, 2024

Closing as the changes in #15 would make this unnecessary

@hanifn hanifn closed this Jul 18, 2024
@hanifn hanifn deleted the add/publish-guard-notice branch July 18, 2024 04:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants