Style Engine: Add support for dimensions.minHeight property #45334
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrewserong
added
[Type] Enhancement
1 task
|
Size Change: +207 B (0%) Total Size: 1.28 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Thanks for splitting this out @andrewserong 👍
✅ JS and PHP tests are passing
✅ No issues encountered while smoke testing
There was just one issue in BLOCK_STYLE_DEFINITIONS_METADATA. I've left an inline comment for it.
Other than that this is looking good to my eyes, as inexperienced with the style engine as they may be 🙂
Comment on lines
145
to
150
| 'classnames' => array( | ||
| 'has-$slug-font-size' => 'font-size', | ||
| ), | ||
| 'css_vars' => array( | ||
| 'spacing' => '--wp--preset--spacing--$slug', | ||
| ), |
There was a problem hiding this comment.
The font size class name looks like a copy-and-paste error.
I take it min-height support is leveraging spacing presets, and that isn't another leftover?
There was a problem hiding this comment.
The font size class name looks like a copy-and-paste error.
Good spotting. I don't think we need classnames here. Removed.
I take it min-height support is leveraging spacing presets, and that isn't another leftover?
Oh yeah, will it? I think it's safe to leave it for now, as it'll only output the CSS var if we ask it to by sending a var:preset|spacing|$slug string. Otherwise it'll work with the raw, custom values.
Similar to spacing properties, we don't generate classnames. Only inline styles.
|
Thanks for the reviews and catching and fixing up the copy + paste error, folks! 🙇
That was my thinking — it's most likely that folks won't usually use the spacing presets with this one, but I figured it's better to allow the parsing for now just in case. |
25 tasks
pento
pushed a commit
to WordPress/wordpress-develop
that referenced
this pull request
Feb 1, 2023
This changeset adds the new dimension feature's PHP code for supporting minimum height in the block editor inspector and in global styles. Minimum height is quite useful for defining the minimum vertical dimensions of a block, while allowing it to expand beyond that size. In this changeset: * Adds support in `theme.json`. * Adds support in the style engine. * Adds support in `wp_apply_dimensions_support()`. * Renames the setting from `'__experimentalDimensions'` to `dimensions` in `wp_register_dimensions_support()`. * Adds PHPUnit tests. Is renaming `'__experimentalDimensions'` a backwards-compatibility (BC) break? Though the setting has been in the code since 5.9.0, it was never wired to anything, ie it did not expose any controls or styles. Notice in `wp_register_dimensions_support()` and `wp_apply_dimensions_support()` prior to this changeset, there are inline comments as placeholders for height and width support, but no code. If a developer opted in to use it, it had no effect. A search in wp.org's plugin and themes repo showed no instances of this experimental setting. Given there was no functionality attached to it (until this changeset), no change in behavior or effect from removing it, and no usage found in the plugins and themes repository, it does appear to be a BC break. References: * [WordPress/gutenberg#45300 Gutenberg PR 45300] * [WordPress/gutenberg#45334 Gutenberg PR 45334] Follow-up to [53076], [52069]. Props andrewserong, aaronrobertshaw , costdev, hellofromTonya, isabel_brison, joen, paaljoachim, mukesh27, ntsekouras, oandregal, ramonopoly. Fixes #57582. git-svn-id: https://develop.svn.wordpress.org/trunk@55175 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith
pushed a commit
to markjaquith/WordPress
that referenced
this pull request
Feb 1, 2023
This changeset adds the new dimension feature's PHP code for supporting minimum height in the block editor inspector and in global styles. Minimum height is quite useful for defining the minimum vertical dimensions of a block, while allowing it to expand beyond that size. In this changeset: * Adds support in `theme.json`. * Adds support in the style engine. * Adds support in `wp_apply_dimensions_support()`. * Renames the setting from `'__experimentalDimensions'` to `dimensions` in `wp_register_dimensions_support()`. * Adds PHPUnit tests. Is renaming `'__experimentalDimensions'` a backwards-compatibility (BC) break? Though the setting has been in the code since 5.9.0, it was never wired to anything, ie it did not expose any controls or styles. Notice in `wp_register_dimensions_support()` and `wp_apply_dimensions_support()` prior to this changeset, there are inline comments as placeholders for height and width support, but no code. If a developer opted in to use it, it had no effect. A search in wp.org's plugin and themes repo showed no instances of this experimental setting. Given there was no functionality attached to it (until this changeset), no change in behavior or effect from removing it, and no usage found in the plugins and themes repository, it does appear to be a BC break. References: * [WordPress/gutenberg#45300 Gutenberg PR 45300] * [WordPress/gutenberg#45334 Gutenberg PR 45334] Follow-up to [53076], [52069]. Props andrewserong, aaronrobertshaw , costdev, hellofromTonya, isabel_brison, joen, paaljoachim, mukesh27, ntsekouras, oandregal, ramonopoly. Fixes #57582. Built from https://develop.svn.wordpress.org/trunk@55175 git-svn-id: http://core.svn.wordpress.org/trunk@54708 1a063a9b-81f0-0310-95a4-ce76da25c4cd
github-actions bot
pushed a commit
to platformsh/wordpress-performance
that referenced
this pull request
Feb 1, 2023
This changeset adds the new dimension feature's PHP code for supporting minimum height in the block editor inspector and in global styles. Minimum height is quite useful for defining the minimum vertical dimensions of a block, while allowing it to expand beyond that size. In this changeset: * Adds support in `theme.json`. * Adds support in the style engine. * Adds support in `wp_apply_dimensions_support()`. * Renames the setting from `'__experimentalDimensions'` to `dimensions` in `wp_register_dimensions_support()`. * Adds PHPUnit tests. Is renaming `'__experimentalDimensions'` a backwards-compatibility (BC) break? Though the setting has been in the code since 5.9.0, it was never wired to anything, ie it did not expose any controls or styles. Notice in `wp_register_dimensions_support()` and `wp_apply_dimensions_support()` prior to this changeset, there are inline comments as placeholders for height and width support, but no code. If a developer opted in to use it, it had no effect. A search in wp.org's plugin and themes repo showed no instances of this experimental setting. Given there was no functionality attached to it (until this changeset), no change in behavior or effect from removing it, and no usage found in the plugins and themes repository, it does appear to be a BC break. References: * [WordPress/gutenberg#45300 Gutenberg PR 45300] * [WordPress/gutenberg#45334 Gutenberg PR 45334] Follow-up to [53076], [52069]. Props andrewserong, aaronrobertshaw , costdev, hellofromTonya, isabel_brison, joen, paaljoachim, mukesh27, ntsekouras, oandregal, ramonopoly. Fixes #57582. Built from https://develop.svn.wordpress.org/trunk@55175 git-svn-id: https://core.svn.wordpress.org/trunk@54708 1a063a9b-81f0-0310-95a4-ce76da25c4cd
|
This PR shouldn't need its own dev note, but rather it forms part of the dev note for the minHeight dimensions block support. I have a draft for that dev note over in this comment: #45300 (comment) |
VenusPR
added a commit
to VenusPR/Wordpress_Richard
that referenced
this pull request
Mar 9, 2023
This changeset adds the new dimension feature's PHP code for supporting minimum height in the block editor inspector and in global styles. Minimum height is quite useful for defining the minimum vertical dimensions of a block, while allowing it to expand beyond that size. In this changeset: * Adds support in `theme.json`. * Adds support in the style engine. * Adds support in `wp_apply_dimensions_support()`. * Renames the setting from `'__experimentalDimensions'` to `dimensions` in `wp_register_dimensions_support()`. * Adds PHPUnit tests. Is renaming `'__experimentalDimensions'` a backwards-compatibility (BC) break? Though the setting has been in the code since 5.9.0, it was never wired to anything, ie it did not expose any controls or styles. Notice in `wp_register_dimensions_support()` and `wp_apply_dimensions_support()` prior to this changeset, there are inline comments as placeholders for height and width support, but no code. If a developer opted in to use it, it had no effect. A search in wp.org's plugin and themes repo showed no instances of this experimental setting. Given there was no functionality attached to it (until this changeset), no change in behavior or effect from removing it, and no usage found in the plugins and themes repository, it does appear to be a BC break. References: * [WordPress/gutenberg#45300 Gutenberg PR 45300] * [WordPress/gutenberg#45334 Gutenberg PR 45334] Follow-up to [53076], [52069]. Props andrewserong, aaronrobertshaw , costdev, hellofromTonya, isabel_brison, joen, paaljoachim, mukesh27, ntsekouras, oandregal, ramonopoly. Fixes #57582. Built from https://develop.svn.wordpress.org/trunk@55175 git-svn-id: http://core.svn.wordpress.org/trunk@54708 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This PR is split out from #45300 and looks at adding support in the style engine for outputting the
min-heightCSS property when it exists indimensions.minHeightwithin the style object.Note: this PR only adds support to the style engine for outputting this rule based on its presence in the style object. The full support for minHeight is being explored in #45300, but it's easier for review and cleaner for commit history if we split the work into component parts.
Why?
Min-height is already supported in
safecss_filter_attrand will be quite useful for Group and Post Content blocks. The structure of storing it under adimensionsobject is consistent with earlier explorations into width and height support.How?
minHeighthandling in the JS implementation of the style enginedimensionsarray toBLOCK_STYLE_DEFINITIONS_METADATAin PHPTesting Instructions
Ensure tests pass or run manually:
Smoke test adding a group block and site title block in the editor, save, and view on the site frontend — ensure nothing broke.
Here's some quick smoke test block markup