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

Use wp:action-publish to determine whether to display publish UI #6724

Merged
merged 1 commit into from
May 14, 2018

Conversation

danielbachhuber
Copy link
Member

Description

Updates post-publish-button and post-publish-panel to use wp:action-publish attribute when determining what to display.

See #6361
Previously #6529 #6630 #6670

How has this been tested?

I published a post as an author without JavaScript errors, and submitted a post for review as a contributor without JavaScript errors.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@danielbachhuber danielbachhuber added this to the 2.9 milestone May 12, 2018
@danielbachhuber danielbachhuber requested a review from a team May 12, 2018 00:27
@nfmohit nfmohit mentioned this pull request May 13, 2018
7 tasks
@gziolo gziolo added [Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... REST API Interaction Related to REST API labels May 14, 2018
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

This looks good, I had some minor notes, where we could consider some improvements to save some keystrokes in the future or hide implementation details.

@@ -82,6 +82,7 @@ export default compose( [
visibility: getEditedPostVisibility(),
isSaveable: isEditedPostSaveable(),
isPublishable: forceIsDirty || isEditedPostPublishable(),
hasPublishAction: get( getCurrentPost(), [ '_links', 'wp:action-publish' ], false ),
Copy link
Member

Choose a reason for hiding this comment

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

Should we introduce a new selector which would hide all those implementation details?

hasPublishAction: hasCurrentPostAction( 'publish' )

Copy link
Member Author

Choose a reason for hiding this comment

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

Should we introduce a new selector which would hide all those implementation details?

See #6655

it( 'should be disabled if post is currently saving', () => {
const wrapper = shallow(
<PostPublishButton user={ user } isSaving />
<PostPublishButton hasPublishAction={ true } isSaving />
Copy link
Member

Choose a reason for hiding this comment

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

hasPublishAction - should it default to true when not provided?

Copy link
Member Author

Choose a reason for hiding this comment

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

should it default to true when not provided?

I'm not sure. My concern would be the possibility of showing incorrect UI to the end user. Could you explain your thinking?

it( 'should show publishing if publishing in progress', () => {
const label = PublishButtonLabel( { user, isPublishing: true } );
const label = PublishButtonLabel( { hasPublishAction: true, isPublishing: true } );
Copy link
Member

Choose a reason for hiding this comment

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

The same question applies, should hasPublishAction default to true?

@gziolo gziolo added the [Type] Enhancement A suggestion for improvement. label May 14, 2018
@danielbachhuber danielbachhuber merged commit ed5a779 into master May 14, 2018
@danielbachhuber danielbachhuber deleted the 6361-post-publish branch May 14, 2018 11:28
danielbachhuber pushed a commit that referenced this pull request Jun 19, 2018
* Fix publishing UX for contributors

* Merge #6724 into PR

* Removed compose from prepublish.js, improved the conditions checking publish action

* Avoid ternaries from toggle label

* Introduce unit tests to post publish panel toggle

* Add explicit tests for button text

* Restore publish toggle label, update publish panel copy for contributors

* Update unit tests according to changed toggle label

* Fix a unit test description
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... REST API Interaction Related to REST API [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants