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

Disable Preview button when post type doesn't support it #6120

Closed
wants to merge 6 commits into from

Conversation

danielbachhuber
Copy link
Member

@danielbachhuber danielbachhuber commented Apr 11, 2018

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:

function create_product_type() {
    register_post_type( 'acme_product',
        array(
			'label'  => 'Product',
            'labels' => array(
                'name' => __( 'Products' ),
                'singular_name' => __( 'Product' )
            ),
            'public' => true,
			'show_in_rest' => true,
			'publicly_queryable' => false,
        )
    );
}
add_action( 'init', 'create_product_type' );

Verify preview button is not available for posts of this Post Type.

image

image

@danielbachhuber
Copy link
Member Author

danielbachhuber commented Apr 11, 2018

I've done what I think should work, but I'm still running into what I think is a simple error:

The above error occurred in the <WithSelect(IfCondition(Connect(t)))> component:
    in WithSelect(IfCondition(Connect(t)))
    in div
    in div
    in Unknown (created by WithDispatch(Component))
    in WithDispatch(Component) (created by WithSelect(WithDispatch(Component)))
    in WithSelect(WithDispatch(Component))
    in div
    in Unknown (created by WithViewportMatch(Component))
    in WithViewportMatch(Component) (created by n)
    in div (created by n)
    in n (created by WithDispatch(n))
    in WithDispatch(n) (created by WithSelect(WithDispatch(n)))
    in WithSelect(WithDispatch(n))
    in t
    in r (created by t)
    in t (created by t)
    in t (created by t)
    in t (created by t)
    in t (created by t)
    in t

React will try to recreate this component tree from scratch using the error boundary you provided, t.
react-dom.b69cca96.js:10003:5
TypeError: e(...).getPostType(...) is undefined[Learn More]
index.js:17:66194
TypeError: e(...).getPostType(...) is undefined[Learn More]
index.js:17:66194

Is there some simple mistake I'm making?

@danielbachhuber danielbachhuber requested a review from a team April 11, 2018 20:50
@danielbachhuber danielbachhuber added this to the 2.7 milestone Apr 11, 2018
} ),
{ autosave }
ifCondition( ( { isPreviewable } ) => isPreviewable ),
connect(
Copy link
Member

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.

Copy link
Member Author

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?

Copy link
Member

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,
Copy link
Member

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.

Copy link
Member

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.

Copy link
Member Author

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',
Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Member

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.

@danielbachhuber
Copy link
Member Author

@pento What opinions or suggestions do you have on how this should be tested?

@danielbachhuber
Copy link
Member Author

This is going to conflict with #6167

@pento
Copy link
Member

pento commented Apr 16, 2018

I'd split the testing into two parts:

  • Test that the API returns the correct viewable value for different post types.
  • Test that the button is disabled when the isPreviewable prop is false. Check the tests for this component, there are some examples of props being manually overridden in the tests.

@pento pento added the Core REST API Task Task for Core REST API efforts label Apr 16, 2018
@danielbachhuber danielbachhuber removed this from the 2.7 milestone Apr 18, 2018
@danielbachhuber
Copy link
Member Author

Closing in favor of #6232

@danielbachhuber danielbachhuber deleted the 5749-disable-preview-button branch April 18, 2018 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core REST API Task Task for Core REST API efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants