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

Only display featured image UI when theme supports it too #6541

Merged
merged 2 commits into from
May 3, 2018

Conversation

danielbachhuber
Copy link
Member

@danielbachhuber danielbachhuber commented May 2, 2018

Description

Ensures the "Featured Image" UI only displays when the theme supports post-thumbnails too.

Fixes #2523
Previously #6510

Types of changes

My code introduces a new ThemeSupportCheck component to inspect the theme's post-thumbnails setting. If the theme doesn't support post thumbnails, then the featured image UI doesn't render.

The code also introduces post-thumbnails=true on the site index for exposure through the REST API.

Checklist:

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

How has this been tested?

Using Twenty Seventeen, here's the UI that normally displays:

image

Then, add the following code snippet to a wp-content/mu-plugins/local.php file:

add_action( 'after_setup_theme', function(){
	remove_theme_support( 'post-thumbnails' );
}, 100 );

The "Featured Image" will no longer display:

image

@danielbachhuber danielbachhuber added this to the 2.8 milestone May 2, 2018
@danielbachhuber danielbachhuber requested a review from a team May 2, 2018 14:55
@danielbachhuber
Copy link
Member Author

Worth noting: I wanted to put theme-support-check in @wordpress/components, but doing so apparently causes a dependency issue with withSelect().

@gziolo
Copy link
Member

gziolo commented May 2, 2018

That’s neat idea 👍

Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Works well 👍


function PostFeaturedImageCheck( props ) {
return <PostTypeSupportCheck { ...props } supportKeys="thumbnail" />;
return <ThemeSupportCheck supportKeys="post-thumbnails">
Copy link
Member

@aduth aduth May 3, 2018

Choose a reason for hiding this comment

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

Nit (Style): My preference (and predominantly throughout the codebase) is for JSX to be placed on its own lines with parentheses if multi-lined, e.g.:

return (
	<ThemeSupportCheck supportKeys="post-thumbnails">
		<PostTypeSupportCheck { ...props } supportKeys="thumbnail" />
	</ThemeSupportCheck>
);

If it's a convention we want to adopt, we could enforce with:

https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-wrap-multilines.md

Copy link
Member Author

Choose a reason for hiding this comment

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

My preference (and predominantly throughout the codebase) is for JSX to be placed on its own lines with parentheses if multi-lined, e.g.:

Seems reasonable to me 1916026

@danielbachhuber danielbachhuber merged commit dc7ee12 into master May 3, 2018
@danielbachhuber danielbachhuber deleted the 2523-theme-supports-2 branch May 3, 2018 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants