-
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
Use wp:action-publish
to determine whether to display publish UI
#6724
Conversation
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.
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 ), |
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.
Should we introduce a new selector which would hide all those implementation details?
hasPublishAction: hasCurrentPostAction( 'publish' )
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.
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 /> |
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.
hasPublishAction
- should it default to true
when not provided?
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.
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 } ); |
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.
The same question applies, should hasPublishAction
default to true
?
* 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
Description
Updates
post-publish-button
andpost-publish-panel
to usewp: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: