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

Disabled conditions for publish buttons #11584

Merged
merged 20 commits into from
Nov 9, 2018
Merged

Disabled conditions for publish buttons #11584

merged 20 commits into from
Nov 9, 2018

Conversation

oandregal
Copy link
Member

@oandregal oandregal commented Nov 7, 2018

We have two components that can be used to publish a post: PostPublishToggle or PostPublishPanelButton. The first will open the PostPublishPanel sidebar, the second is used in the PostPublishPanel sidebar, and substitutes the toggle when pre-publish checks are disabled or when the post is published.

The conditions under which they are disabled are different at the moment, tests pass due to wrong assumptions and/or conditions aren't met. In bold what this PR fixes:

Button Toggle
isSavingPost=true disabled disabled
forceIsSaving=true disabled disabled
isEditedPostSaveable=false disabled disabled
isEditedPostPublishable=false && forceIsDirty=false disabled disabled
isCurrentPostPublished=true - disabled
isPostSavingLocked=true disabled -
isEditedPostSaveable=true && isEditedPostPublishable=false && forceIsDirty=true enabled enabled
isEditedPostSaveable=true && isEditedPostPublishable=true && forceIsDirty=false enabled enabled

Testing

Button and Toggle are disabled while a draft is being saved

  • Create a new post.
  • Click "Save draft" button.
  • Check that publish toggle is disabled.
  • Repeat for the publish button (disable pre-publish checks in "More options > Options" modal first).

They should look like this:

screenshot from 2018-11-08 10-42-45

Note that when a post is being published, the button should show a busy status instead:

screenshot from 2018-11-08 10-45-16

Button and Toggle are disabled when forceIsSaving is true

  • Create a new post.
  • Open the console and execute wp.data.dispatch( 'core/edit-post' ).requestMetaBoxUpdates();
  • Check that the publish toggle is disabled for a quick moment.
  • Repeat for the publish button (disable pre-publish checks in "More options > Options" modal first).

They should look like this:

screenshot from 2018-11-08 10-42-45

Toggle is disabled if post is published

No manual testing for this, we have a unit test that covers it. In Gutenberg case, the toggle is never shown if the post is published (we show the button instead). TLDR: at this point in the release cycle I don't want to introduce API changes.

Toggle is enabled if post is saveable (has contents) and forceIsDirty

No manual test for this either. Same reasons than above. Note that forceIsDirty has been added here as a new condition just for completeness. It doesn't modifies API.

@oandregal oandregal self-assigned this Nov 7, 2018
@oandregal oandregal added the [Type] Bug An existing feature does not function as intended label Nov 7, 2018
@oandregal oandregal changed the title Fix disable publish Disabled conditions for publish buttons Nov 7, 2018
@oandregal oandregal force-pushed the fix/disable-publish branch from 53f0b4a to 5de5bc2 Compare November 8, 2018 09:13
@oandregal oandregal added this to the 4.4 milestone Nov 8, 2018
@oandregal oandregal requested a review from a team November 8, 2018 10:56
@oandregal
Copy link
Member Author

@tofumatt this is part of figuring out the accessibility stuff. I'm trying to merge this two buttons, which implies I need to understand (and fix!) their enable/disable states.

@oandregal oandregal force-pushed the fix/disable-publish branch from 0c91e4b to 4075b57 Compare November 8, 2018 13:35
Copy link
Member

@tofumatt tofumatt left a comment

Choose a reason for hiding this comment

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

Worked for me! The manual testing instructions here read like they'd make good E2E tests 😏

@oandregal oandregal merged commit ca67e13 into master Nov 9, 2018
@youknowriad youknowriad modified the milestones: 4.4, 4.3 Nov 9, 2018
@mtias mtias deleted the fix/disable-publish branch November 12, 2018 16:15
@mtias mtias added the [Feature] Document Settings Document settings experience label Nov 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Document Settings Document settings experience [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants