Skip to content

Disable post format input if the theme does not support post formats.#5203

Merged
jorgefilipecosta merged 1 commit intomasterfrom
fix/disable-post-fomats-input-if-theme-does-not-supports-it
Feb 28, 2018
Merged

Disable post format input if the theme does not support post formats.#5203
jorgefilipecosta merged 1 commit intomasterfrom
fix/disable-post-fomats-input-if-theme-does-not-supports-it

Conversation

@jorgefilipecosta
Copy link
Copy Markdown
Member

This is the same behaviour we have currently on the classic. Showing the post format input also has another non-desirable side effect, when the theme does not support post formats, posts are created with standard post format and not the default post format setting. As the setting is not respected, leaving the input there confuses the user.

How Has This Been Tested?

Verify that in themes that support post formats e.g:Twenty Seventeen, we have the post format input with the value equal to the default post format setting.
Verify that in themes that do not support post formats e.g: Gutenberg theme the post format input does not appear.

@jorgefilipecosta jorgefilipecosta added [Type] Bug An existing feature does not function as intended [Feature] Document Settings Document settings experience labels Feb 22, 2018
@jorgefilipecosta jorgefilipecosta self-assigned this Feb 22, 2018
@jorgefilipecosta jorgefilipecosta added this to the 2.3 milestone Feb 23, 2018
export default PostFormatCheck;
export default withContext( 'editor' )(
( settings ) => ( {
themeSupportsPostFormats: settings.themeSupportsPostFormats,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe destructure settings on L17?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thank you for your feedback @ockham, the improvement was applied.

Copy link
Copy Markdown
Member

@aduth aduth Feb 28, 2018

Choose a reason for hiding this comment

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

Alternatively:

( settings ) => pick( settings, 'themeSupportsPostFormats' ),

https://lodash.com/docs/4.17.5#pick

Personally, I'm on the fence about whether destructuring arguments in-place harms readability, as it obscures what it is that is being destructured (i.e. we've lost all mention of "settings").

@jorgefilipecosta jorgefilipecosta force-pushed the fix/disable-post-fomats-input-if-theme-does-not-supports-it branch from 27ffe0f to 266ed31 Compare February 27, 2018 21:20
@jorgefilipecosta jorgefilipecosta requested a review from a team February 28, 2018 15:49
'availableTemplates' => wp_get_theme()->get_page_templates( get_post( $post_to_edit['id'] ) ),
'blockTypes' => $allowed_block_types,
'disableCustomColors' => get_theme_support( 'disable-custom-colors' ),
'themeSupportsPostFormats' => current_theme_supports( 'post-formats' ),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not sure the editor should be aware of the concept of "themes", should just rename the property something like disablePostFormats to match disabledCustomColors's convention

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Code was updated 👍

This is the same behaviour we have currently on the classic. Showing the post format input also has another non-desirable side effect, when the theme does not support post formats, posts are created with standard post format and not the default post format setting. As the setting is not respected, leaving the input there confuses the user.
@jorgefilipecosta jorgefilipecosta force-pushed the fix/disable-post-fomats-input-if-theme-does-not-supports-it branch from 266ed31 to ee29111 Compare February 28, 2018 16:57
Copy link
Copy Markdown
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@gziolo
Copy link
Copy Markdown
Member

gziolo commented Feb 28, 2018

Docs need to be updated :)

@jorgefilipecosta jorgefilipecosta merged commit 5a0767e into master Feb 28, 2018
@jorgefilipecosta jorgefilipecosta deleted the fix/disable-post-fomats-input-if-theme-does-not-supports-it branch February 28, 2018 19:39
@jorgefilipecosta
Copy link
Copy Markdown
Member Author

jorgefilipecosta commented Feb 28, 2018

Hi @gziolo, I'm not certain where we should document these behaviours as the flag we use from the theme ! current_theme_supports( 'post-formats' ), is something already used in WordPress and the classic editor already had this behavior. Do you have a place in mind where this and similar behaviors should be documented?

@gziolo
Copy link
Copy Markdown
Member

gziolo commented Feb 28, 2018

Right, skip it then. Probably nothing to worry about for theme developers :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Document Settings Document settings experience [Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants