Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles #42544
Conversation
andrewserong
added
Global Styles
|
Size Change: +2.51 kB (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
|
Thank you @andrewserong for working on the pull request. I don't fully understand why should not the following in the Also is there a defined "rule" when theme support is managed by |
Thanks for taking a look @grappler! Before this PR is ready for review, I'll try adding a bit of documentation to help clarify the feature. The implementation here is different to With this PR, folks who currently completely opt-out of all generated styles via disabling the Layout block support, can do so via The necessity for the feature is mostly due to the fact that #40875 moved where some of the Layout styles are generated, so the undocumented approach that some folks currently use by dequeueing or removing the filter |
andrewserong
force-pushed
the
try/add-theme-support-to-disable-layout-styles
branch
from
819e9c3 to
e825920
Compare
andrewserong
requested review from
TimothyBJacobs,
spacedmonkey,
geriux,
ellatrix,
ajitbohra,
ryanwelcher,
juanmaguitar and
fabiankaegy
as code owners
| * | ||
| * @return {boolean} Whether or not the theme opts-in to disable all layout styles. | ||
| */ | ||
| function useThemeHasDisabledLayoutStyles() { |
There was a problem hiding this comment.
Could the site editor package access the hook from the block editor package instead of duplicating it?
There was a problem hiding this comment.
That's a good question — I wasn't sure if we wanted to expose it for use anywhere else so left it as a duplication for now. But if it's an official theme support, then yes, it probably makes better sense to have it defined once in the block editor package, and then export it 👍
I'll have a play.
There was a problem hiding this comment.
After taking another look, I think it makes the code a little clearer if we don't actually use the hook abstraction. It didn't add much, and meant that if you wanted to see what it was actually checking you'd have to look at the implementation of the hook. In 6b6dadb I've switched to inlining the settings checks, so that it's closer to the feature name, which I think on balance, is a little easier to read.
Happy to change it, though, if you think there's a better way to handle it!
|
We should make sure to register this with |
lib/block-supports/layout.php
Outdated
| $class_names[] = $container_class; | ||
| wp_enqueue_block_support_styles( $style ); | ||
| // Only generate Layout styles if the theme has not opted-out. | ||
| if ( ! current_theme_supports( 'disable-layout-styles' ) ) { |
There was a problem hiding this comment.
There's also a bunch of layout-related logic towards the top of this function. Would it make sense to return early instead of adding this condition here?
There was a problem hiding this comment.
Thanks for testing! In this case, I don't think we can return early, because we still want the Layout support's semantic classnames to be added to the output which happens at line 260. Part of the feedback that inspired this PR was that there are Classic themes that would like the semantic classnames like layout type and content justification to be available, but not the generated Layout styles themselves.
It does make it a bit of an awkward place for the check to live, though! I suppose we could move it up to line 228 so that this if block includes the $gap_value declaration and everything below that?
There was a problem hiding this comment.
Oooh, of course, that makes sense 🤦 Yeah, I guess we could move it up a little bit, so there are no unnecessary calculations performed.
There was a problem hiding this comment.
I've moved it up a little in d77120e, which feels a bit more logical to me now, thanks for the suggestion! Also added a comment about the classnames so it's clearer why we're not returning early.
Separately to this PR, now that we've got a fair few classes being generated it might be worth us re-exploring splitting out the classnames logic to a separate function to make it a bit easier to read. We had it in a separate function for a little while but reverted in #41885 so that we could give it some more thought before committing to a particular approach that we then need to continue to support. But 🤞 we can settle on an approach before 6.1.
…o it appears in API responses
Thanks for the suggestion @TimothyBJacobs! I've added that in d77120e, and the default value is now being returned in API responses from the Themes API endpoint 👍
I think the only outstanding task now is to look at Glen's feedback (#42544 (comment)) about potentially removing the duplication of the hook to check the feature in the block editor versus global styles. I'll give this a little more thought, because I'm not quite sold on the hook, given that it's not doing very much. Will follow up tomorrow. Thanks for the feedback, all! 🙇 |
|
Thanks @andrewserong, looks good to me! |
After playing with a couple of options I wound up removing the hook in 6b6dadb as it felt a bit neater to me to inline the settings check so that the code is closer to the setting value. Happy to continue iterating on it if anyone has a preferred way to handle it. I think this PR should be ready for a final review now, if I haven't missed anything! |
There was a problem hiding this comment.
Re-tested all the screens with both block and classic themes, and all is working well ✅
One question that occurs to me: should we still display the content width controls if theme opts out of layout? Especially with the custom ones there will be no way for the theme to access the user set values, though if #42664 or #42763 goes ahead, they could be able to grab the global value if we output the custom property at root level. We might not want to, of course! It depends to what point themes having full control over layout mean users have no control over customisation.
(This is fine to work out as a follow-up!)
|
Thanks for reviewing @tellthemachines! 🙇
That's a great question, I think I'm leaning toward us figuring that out in a follow-up particularly with the WIP PRs to adjust how the content width works. Ideally this particular theme support just switches off the style output, but like you mention there are probably a few cases where the UI controls also need to be hidden if they're completely useless without the generated styles 🤔 Given that this is a new theme feature, rather than something used out in the wild yet, what do you think if we merge this PR as-is and then dig into it more deeply when we settle on one of the approaches in #42664 or #42763? (If so, I'll add a task to the Layout tracking issue so we don't forget). |
Yup, that makes sense! |
|
I believe this feature will require a dev note for WordPress 6.1, so that themes that currently remove the By default, WordPress outputs structural layout styles for blocks such as Group, Columns, Social Icons and Buttons, as part of the layout block support. Up until this point, some themes have been opting out of the generated base layout styles by using an undocumented approach of overriding or removing the In WordPress 6.1, base layout styles have been consolidated as part of global styles output (introduced in this PR), so the existing approach of opting out of generated layout styles will no longer work. In its place, there is now a new theme support flag that themes can use, called // Opt in to remove layout styles.
add_theme_support( 'disable-layout-styles' );Using this new theme support flag ensures that the expected behaviour is handled explicitly in code, and allows themes that use this flag to still make use of semantic classnames that are injected into layout blocks at render time, such as content justification classes. Note that opting in to this feature will remove required styles for blocks such as Group, Columns, Social Icons and Buttons, so themes that use this feature will need to supply their own structural layout styles in order for these blocks to render correctly in the editor and on the site's frontend. In most cases, particularly for blocks-based themes, the generated layout styles are preferable for ensuring the correct rendering of core blocks. For themes that are looking to adjust block spacing, see the handbook entry for "What is blockGap and how can I use it?" instead. |
andrewserong
added
the
Needs Dev Note
This change backports the following changes from Gutenberg repository: - [WordPress/gutenberg#40875 gutenberg#40875] Layout: Use semantic classnames, centralize layout definitions, reduce duplication, and fix blockGap in theme.json - [WordPress/gutenberg#42544 gutenberg#42544] Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles gutenberg#42544 - [WordPress/gutenberg#42087 gutenberg#42087] Theme.json: Add block support feature level selectors for blocks gutenberg#42087 - [WordPress/gutenberg#43792 gutenberg#43792] Global Styles: Split root layout rules into a different function gutenberg#43792 - [WordPress/gutenberg#42544 gutenberg#42544] Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles gutenberg#42544 - [WordPress/gutenberg#42665 gutenberg#42665] Layout: Reduce specificity of fallback blockGap styles gutenberg#42665 - [WordPress/gutenberg#42085 gutenberg#42085] Core CSS support for root padding and alignfull blocks gutenberg#42085 Note that it doesn't entirely port over PR40875 — the remaining PHP changes for that PR will be explored in a separate PR targeting `layout.php`. Props andrewserong, aaronrobertshaw, isabel_brison. See #56467. git-svn-id: https://develop.svn.wordpress.org/trunk@54159 602fd350-edb4-49c9-b593-d223f7449a82
This change backports the following changes from Gutenberg repository: - [WordPress/gutenberg#40875 gutenberg#40875] Layout: Use semantic classnames, centralize layout definitions, reduce duplication, and fix blockGap in theme.json - [WordPress/gutenberg#42544 gutenberg#42544] Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles gutenberg#42544 - [WordPress/gutenberg#42087 gutenberg#42087] Theme.json: Add block support feature level selectors for blocks gutenberg#42087 - [WordPress/gutenberg#43792 gutenberg#43792] Global Styles: Split root layout rules into a different function gutenberg#43792 - [WordPress/gutenberg#42544 gutenberg#42544] Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles gutenberg#42544 - [WordPress/gutenberg#42665 gutenberg#42665] Layout: Reduce specificity of fallback blockGap styles gutenberg#42665 - [WordPress/gutenberg#42085 gutenberg#42085] Core CSS support for root padding and alignfull blocks gutenberg#42085 Note that it doesn't entirely port over PR40875 — the remaining PHP changes for that PR will be explored in a separate PR targeting `layout.php`. Props andrewserong, aaronrobertshaw, isabel_brison. See #56467. Built from https://develop.svn.wordpress.org/trunk@54159 git-svn-id: http://core.svn.wordpress.org/trunk@53718 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This change backports the following changes from Gutenberg repository: - [WordPress/gutenberg#40875 gutenberg#40875] Layout: Use semantic classnames, centralize layout definitions, reduce duplication, and fix blockGap in theme.json - [WordPress/gutenberg#42544 gutenberg#42544] Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles gutenberg#42544 - [WordPress/gutenberg#42087 gutenberg#42087] Theme.json: Add block support feature level selectors for blocks gutenberg#42087 - [WordPress/gutenberg#43792 gutenberg#43792] Global Styles: Split root layout rules into a different function gutenberg#43792 - [WordPress/gutenberg#42544 gutenberg#42544] Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles gutenberg#42544 - [WordPress/gutenberg#42665 gutenberg#42665] Layout: Reduce specificity of fallback blockGap styles gutenberg#42665 - [WordPress/gutenberg#42085 gutenberg#42085] Core CSS support for root padding and alignfull blocks gutenberg#42085 Note that it doesn't entirely port over PR40875 — the remaining PHP changes for that PR will be explored in a separate PR targeting `layout.php`. Props andrewserong, aaronrobertshaw, isabel_brison. See #56467. Built from https://develop.svn.wordpress.org/trunk@54159 git-svn-id: https://core.svn.wordpress.org/trunk@53718 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This change backports the following changes from Gutenberg repository: - [WordPress/gutenberg#40875 gutenberg#40875] Layout: Use semantic classnames, centralize layout definitions, reduce duplication, and fix blockGap in theme.json - [WordPress/gutenberg#42544 gutenberg#42544] Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles gutenberg#42544 - [WordPress/gutenberg#42087 gutenberg#42087] Theme.json: Add block support feature level selectors for blocks gutenberg#42087 - [WordPress/gutenberg#43792 gutenberg#43792] Global Styles: Split root layout rules into a different function gutenberg#43792 - [WordPress/gutenberg#42544 gutenberg#42544] Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles gutenberg#42544 - [WordPress/gutenberg#42665 gutenberg#42665] Layout: Reduce specificity of fallback blockGap styles gutenberg#42665 - [WordPress/gutenberg#42085 gutenberg#42085] Core CSS support for root padding and alignfull blocks gutenberg#42085 Note that it doesn't entirely port over PR40875 — the remaining PHP changes for that PR will be explored in a separate PR targeting `layout.php`. Props andrewserong, aaronrobertshaw, isabel_brison. See #56467. Built from https://develop.svn.wordpress.org/trunk@54159
andrewserong
added
the
[Feature] Layout
This change backports the following changes from Gutenberg repository: - [WordPress/gutenberg#40875 gutenberg#40875] Layout: Use semantic classnames, centralize layout definitions, reduce duplication, and fix blockGap in theme.json - [WordPress/gutenberg#42544 gutenberg#42544] Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles gutenberg#42544 - [WordPress/gutenberg#42087 gutenberg#42087] Theme.json: Add block support feature level selectors for blocks gutenberg#42087 - [WordPress/gutenberg#43792 gutenberg#43792] Global Styles: Split root layout rules into a different function gutenberg#43792 - [WordPress/gutenberg#42544 gutenberg#42544] Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles gutenberg#42544 - [WordPress/gutenberg#42665 gutenberg#42665] Layout: Reduce specificity of fallback blockGap styles gutenberg#42665 - [WordPress/gutenberg#42085 gutenberg#42085] Core CSS support for root padding and alignfull blocks gutenberg#42085 Note that it doesn't entirely port over PR40875 — the remaining PHP changes for that PR will be explored in a separate PR targeting `layout.php`. Props andrewserong, aaronrobertshaw, isabel_brison. See #56467. git-svn-id: https://develop.svn.wordpress.org/trunk@54159 602fd350-edb4-49c9-b593-d223f7449a82
| const _settings = select( blockEditorStore ).getSettings(); | ||
| return { | ||
| themeHasDisabledLayoutStyles: _settings.disableLayoutStyles, | ||
| themeSupportsLayout: _settings.supportsLayout, |
There was a problem hiding this comment.
Why a new key config when we already have settings.supportsLayout ? Are these two things different and why?
There was a problem hiding this comment.
The names are unfortunately similar, but disableLayoutStyles only disables the layout styles output, but not the controls or the classname output. The use case is for themes that wish to provide their own styling rules for layout, using the classnames output by the controls / layout support. It's been a while now since we implemented this feature, but from memory, this was one of those features that helped some theme developers to adopt layout controls when they weren't ready yet to adopt all the generated layout styles.
What?
Add a theme support flag to allow themes to opt out of all Layout block support styles via
add_theme_support( 'disable-layout-styles' );.A draft dev note for this feature can be found in this comment: #42544 (comment)
Why?
As discussed in #41487 (comment), #42333, #41431 and elsewhere, many themes decided to opt-out of Layout styles altogether via overriding or removing the
'gutenberg_render_layout_support_flag'filter. This results in two issues:So, this PR proposes providing an explicit approach for themes that wish to opt-out of Layout styles altogether to do so, via
add_theme_support( 'disable-layout-styles' );. Note that themes that opt-out of Layout styles will then be responsible for providing all CSS styling for block layouts. However, as discussed in related issues, many (particularly Classic themes) expect to do so.The goal here is to reduce friction caused by the Layout block support, and more explicitly define the expectations of the Layout support.
How?
disable-layout-stylestheme support flag, and expose it in block editor settings, so that it's available in JS.true, however continue to render semantic classnames. (For themes that also wish to opt-out of semantic classnames, they can do this via also overriding the'gutenberg_render_layout_support_flag'— this PR does not propose adding a flag to switch off semantic classnames)Testing Instructions
functions.phpfile (e.g. in TwentyTwentyTwo add it to thetwentytwentytwo_supportfunction):"is-content-justification-space-between is-layout-flex wp-block-buttons")Here's some markup for testing
Screenshots or screencast
Note, this feature intentionally removes generated Layout styles, however styles provided by individual blocks in their own CSS files are not affected by this feature.
The new feature should also appear in API requests to the
themesendpoint. One way to test this is to inspect network requests from the post editor, and filter bythemes. The response for the current theme should include the default value in thetheme_supportsobject: