-
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
Disable Preview button when post type doesn't support it #6120
Conversation
I've done what I think should work, but I'm still running into what I think is a simple error:
Is there some simple mistake I'm making? |
ef79471
to
aa9acfe
Compare
} ), | ||
{ autosave } | ||
ifCondition( ( { isPreviewable } ) => isPreviewable ), | ||
connect( |
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.
Everything in the connect()
should be moved to the withSelect()
. This also avoids having to import getEditedPostAttribute()
twice.
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.
I'm not sure I follow. Can you clarify what you mean by this, or point me to an example?
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.
Sure, check out the withSelect()
usage in the post title component.
Anything that's coming from the editor selectors should be returned from the withSelect()
function. For example, the current modified: getStateEditedPostAttribute( state, 'modified' )
in connect()
would be moved to the object returned from the withSelect()
function, so it becomes:
return {
isPreviewable: get( postType, 'viewable', false ),
modified: getEditedPostAttribute( 'modified' ),
};
The same thing can be repeated for all of the items in the connect()
.
const { getPostType } = select( 'core' ); | ||
const postType = getPostType( getEditedPostAttribute( 'type' ) ); | ||
return { | ||
isPreviewable: postType && postType.previewable, |
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.
You can use get( postType, 'previewable', false )
here, rather than having the inline check.
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.
Actually, should probably just access it as postType.previewable
, without the postType &&
, as the posttype should always be retrieved in edit context.
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.
Updated.
lib/compat.php
Outdated
*/ | ||
function gutenberg_register_rest_api_post_type_previewable() { | ||
register_rest_field( 'type', | ||
'previewable', |
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.
Is there are particular reason for calling it "previewable", rather than "viewable"? I added a similar flag in this PR: https://github.com/WordPress/gutenberg/pull/5756/files#diff-6ff32417da0658502e7caa1a1abbeae6R403
But "previewable" is not quite the flag name I'd be using in that case.
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.
No particular reason. Changed it to viewable
.
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.
Noice. I shamelessly stole this and merged it in 4c90150, so it doesn't need to be in this PR anymore.
aa9acfe
to
683c595
Compare
@pento What opinions or suggestions do you have on how this should be tested? |
This is going to conflict with #6167 |
I'd split the testing into two parts:
|
Closing in favor of #6232 |
Description
In the classic editor, the preview button is only available if is_post_type_viewable returns true. We are updating the logic to follow the same behavior.
Fixes #5749
Previously #5770
How Has This Been Tested?
Register a post type with publicly_queryable set to false.
E.g:
Verify preview button is not available for posts of this Post Type.