Update Line-Height Settings for opt-in (rather than opt-out) #23904
Conversation
This update adjusts the line-height theme support setting to be opt-in via `add_theme_support( 'experimental-line-height' );` rather than enabled by default. This keeps it consistent with the other recently added style theme support settings like padding (spacing).
ItsJonQ
added
the
[Feature] Design Tools
ItsJonQ
requested review from
ellatrix,
spacedmonkey,
TimothyBJacobs and
youknowriad
as code owners
|
Size Change: +3 B (0%) Total Size: 1.14 MB
ℹ️ View Unchanged
|
| @@ -655,7 +655,7 @@ function gutenberg_extend_settings_block_patterns( $settings ) { | |||
| * @return array Filtered editor settings. | |||
| */ | |||
| function gutenberg_extend_settings_custom_line_height( $settings ) { | |||
| $settings['__experimentalDisableCustomLineHeight'] = get_theme_support( 'disable-custom-line-height' ); | |||
There was a problem hiding this comment.
Since it was publicly discussed that line-height is shipping in 5.5. I don't think we should be reverting to an experimental theme support flag.
While ideally we'd focus on theme.json for the future. We can use theme support flags for padding and line height for this release.
There was a problem hiding this comment.
As pointed out by @afercia in this comment, it's probably a bad idea to be exposing line height controls separately from font size controls in the first place, so I think it definitely should be made experimental so our options for improving it in the future are more open.
There was a problem hiding this comment.
that's great feedback and we should improve on that. I consider these enhancements though and I don't think it's a valid reason to revert to the experimental stage. It's been used by folks in the plugin for a long time now.
There was a problem hiding this comment.
So far, the only usage of the line height control I've seen has been very questionable usage. A lot of block patterns use custom line heights for headings and such, when this is almost never something you should do on an actual website. Your line height system ought to be consistent and set from the site-wide level. It's really weird to me that a lot of core block patterns use custom line heights (and font sizes), when these will almost always conflict with the user's theme. Core block patterns ought to adapt to any theme... otherwise they're only useful as tech demos.
There was a problem hiding this comment.
Since we're powering a third of the web, we ought to stop promoting anti-patterns like one-off line heights and visual-hierarchy-breaking font-sizes for headings.
There was a problem hiding this comment.
It is actually already used in different block patterns. Even before FSE and Global Styles, the design possibilities for block patterns were very limited because of the lack of these controls, And sometimes they'd lead to developers/designers using non-semantic blocks to achieve the desired results (using headings instead of paragraphs, paragraphs instead of quotes...). The more control we'd give to blocks, the better block patterns we'd be able to produce.
There was a problem hiding this comment.
If the designers are using Paragraph blocks instead of Quotes, perhaps that's just because the Quote block needs to have the same styling controls as a Paragraph. (And also, the Quote block should probably use nested blocks anyway.)
Even back when some patterns were using Heading blocks when they should have been using Paragraph blocks, the Paragraph block already had font size options... even default font size presets provided by core! So I don't see how that's a good example.
Can you name any actual cases where a custom line height was required? I imagine it would have been a lot simpler if the core font size presets also changed the line height simultaneously, rather than providing a separate line height control.
There was a problem hiding this comment.
Both the current large-header and large-header-paragraph block patterns shipping in Core have custom line height without considering the third-party patterns that exist in the wild.
Your arguments suggest to me that you're arguing against the feature entirely and not the fact of it being experimental. I hope I explained above why this feature is needed but if you're still not convinced, feel free to open an issue to discuss it more.
There was a problem hiding this comment.
Hmm... you're right. I guess I am arguing against the feature entirely. I'd rather see line height be explicitly tied to font size global preset controls. I want the only place where a line height control is shown to be the place where you define the font size presets in the Global Styles UI. I see zero reason to provide one-off line height controls; I think that allowing the user to define global defaults and their own set of presets to choose from a drop-down later on is the way to go. Because of this, I think providing a line height control is very premature.
However, that is not directly related to this PR, and with that in mind, I'm okay with this merging with the line height control remaining "stable", since making it opt-in is still an improvement over master and so we ought to get this in sooner rather than later.
(If I have the time, I may make some more PRs in the future to remove custom line height usage from core block patterns to make them fit in better with themes.)
|
Thanks for the review + discussion all! I'll merge this up. |
|
@ItsJonQ I still don't think this should be experimental as it's not as of today and it's highlighted on the 5.5 beta1 post https://wordpress.org/news/2020/07/wordpress-5-5-beta-1/ so this PR can't be backported as is. Let's try to fix that on a follow-up PR. |
youknowriad
added
the
Backport to WP 6.7 Beta/RC
This update adjusts the line-height theme support setting to be opt-in via `add_theme_support( 'experimental-line-height' );` rather than enabled by default. This keeps it consistent with the other recently added style theme support settings like padding (spacing).
youknowriad
removed
the
Backport to WP 6.7 Beta/RC
This update adjusts the line-height theme support setting to be opt-in via
add_theme_support( 'experimental-line-height' );rather than enabled by default. This keeps it consistent with the other recently added style theme support settings like padding (spacing).How has this been tested?
Tested locally in Gutenberg
npm run devadd_theme_support( 'experimental-line-height' );to an applicable theme PHP file (e.g.functions.php)Types of changes
add_theme_supportvalue fromdisable-custom-line-heighttoexperimental-line-heightChecklist: