Add a minHeight block support under dimensions #45300
Conversation
andrewserong
added
[Type] Enhancement
|
Size Change: +443 B (0%) Total Size: 1.28 MB
ℹ️ View Unchanged
|
| @@ -136,6 +136,20 @@ final class WP_Style_Engine { | |||
| ), | |||
| ), | |||
| ), | |||
| 'dimensions' => array( | |||
There was a problem hiding this comment.
This is a good top-level label.
Carry on.
There was a problem hiding this comment.
Side note: I'm working on a PR that will allow us to extend base style definitions
Nothing to do here. I think having a set of definitions for dimensions: { width, height, ... } in the base set of definitions makes sense 👍
There was a problem hiding this comment.
When the time comes, after testing this branch and so on, what do you think about splitting the style engine additions into a dependency PR?
I'm just thinking of the need for unit tests and how to avoid bloating this PR.
There was a problem hiding this comment.
what do you think about splitting the style engine additions into a dependency PR?
Thanks for taking a look!
Absolutely, I'm all for splitting this up into component parts to make the merge and review process easier. It's super easy for these new block support addition PRs to become unwieldy 👍
There was a problem hiding this comment.
Splitting this up a bit sounds like a solid plan. Might make tracking down when individual blocks had their bespoke controls moved over to this block support easier as well.
|
This is testing well manually. 2022-10-27.14.51.56.mp4Love the fact that I can add 2022-10-27.14.55.46.mp4 |
There was a problem hiding this comment.
Awesome work @andrewserong, nicely done 👍
Overall this tests very well.
I did find one small issue around not being able to skip serialization though and left an inline comment on that.
| @@ -136,6 +136,20 @@ final class WP_Style_Engine { | |||
| ), | |||
| ), | |||
| ), | |||
| 'dimensions' => array( | |||
There was a problem hiding this comment.
Splitting this up a bit sounds like a solid plan. Might make tracking down when individual blocks had their bespoke controls moved over to this block support easier as well.
|
Thanks for reviewing and testing @ramonjd and @aaronrobertshaw! I'll make a start splitting out the style engine changes into a separate PR as I think that'll be the logical thing to land first (the ability to output |
|
Update: I've split out the style engine changes and opened up a separate PR for just that part (with tests) over in #45334 |
andrewserong
force-pushed
the
try/add-min-height-block-support
branch
from
b7758ca
to
dd51e9f
Compare
andrewserong
requested review from
spacedmonkey,
ellatrix,
ajitbohra,
ajlende,
ryanwelcher,
juanmaguitar and
fabiankaegy
as code owners
|
Update: here's how the feature currently looks in the post editor:
@WordPress/gutenberg-design does this look like an acceptable MVP for adding in a
My goal for this PR is to try to get to a good MVP for the feature. We can then land it in smaller more easily reviewed PRs (beginning with #45334), and once the MVP of the feature is in, we can then explore more UI follow-ups like adding in a slider next to the UI control / exploring whether a resizeable box is suitable, too. Note: I'm AFK on and off this week and next, so no rush on this one! |
When it comes time to address the resizable box side of this new support, one option may be to leverage a block popover for the resizable box. An example can be found in #41153 where I explored exactly this for the Cover block's min-height. While that PR is a little stale now, it only needs the block popover to have the ability not to be constrained to the viewport to be ready for review. |
|
Nice, this is cool. I'm assuming we can separately refactor Cover to use this?
And can we call it "Min. height" and add a slider to the mix?
Same componentry as font size, in other words:
Nice enhancement. |
|
Thank you for your work on this! I think this could definitely help solve the original issue. One important question that arises is if we set a specific minimum height (700px for instance) on desktop, would that adjust for mobile? Where naturally 700px height would be too much. |
|
Very nice! Testing using WP 6.0.3 In a page. I added the Minimum Height with 100vh. Checked the frontend. It looks good! This is very helpful to have this feature added to the dimensions panel! Thank you! |
andrewserong
added
the
Needs Dev Note
|
I've opened up a follow-up issue to keep track of the tasks and suggestions here: #45501 — feel free to add anything over there if anyone has any other tasks they think should be addressed! While it's very early for 6.2, I've added a Needs Dev Note label to this one so we don't forget when the time comes for the 6.2 release, as it'll likely need a little highlighting. |
|
Update: I've opened up a follow-up PR to add in the slider alongside the unit control: #45875 — it's the end of my week now, so I very well might have missed something in that PR, but I'll test it some more on Monday. |
|
Thanks for the heads-up, glad to hear it's been resolved! 🙂 |
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
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
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
Dev notes for 6.2Note: if there's room, this dev note could potentially be combined with other design tools in a larger dev note. Draft below: Minimum Height dimensions block supportIn WordPress 6.2, a new minimum height block support feature has been added, with the Group and Post Content blocks opted-in by default. For themes using the The feature allows users to set a Group or Post Content block to have a minimum height, similar to the control that exists for the Cover block. An example use case is to ensure that a post content area has a minimum height, forcing a footer area to render lower on the page, even if a page has little content. For this reason, it can be quite useful to use viewport relative units when setting minimum height, e.g. Setting minimum height for the Stack variation of the Group block is also useful when combined with the new vertical alignment tools added in 6.2 introduced in this Gutenberg PR, as it allows users to align the blocks within the minimum height of the Stack to be at the How to add minHeight support to a themeThere are two ways to add support for For themes that wish to have more granular control over which UI tools are enabled, the For styling blocks globally, How to add minHeight support to a custom blockIn the block's Further reading
@bph for visibility, here's a draft dev note for the minHeight dimensions block support. It could potentially be grouped together with other design tools (e.g. position support?) in a bigger dev note. |
bph
added
has dev note
|
@andrewserong thank you so much for taking care of this dev note. I am leaning towards single post dev note for visibility in the Fieldguide and because it adds so much more flexibility for themes. Would you have bandwidth to add it to the Make blog as a draft, so it can be added to the publishing queue in time for the Fieldguide deadline (later this week)? LMK, if not, and I can do it for you. |
|
Thanks @bph, here's a draft on the Make blog for minHeight: https://make.wordpress.org/core/?p=103001&preview=1&_ppp=6452826240 |
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
|
Awesome work, will minHeight will be supported for the image block as well? |
Dependencies
What?
Add a
minHeightblock support feature that sits under Dimensions in the block editor inspector and in global styles.This PR builds on the good earlier work by @aaronrobertshaw in #32499 which looked at adding in a height block support feature, and the work by @ramonjd on min-height block support in #33970.
Why?
I think we might be at a point where adding min-height will be really useful, so let's revisit it now that we've got the style engine in 😄
Minimum height is quite useful for defining the minimum vertical dimensions of a block, while allowing it to expand beyond that size. As raised in #45117 having control over minimum height could be particularly useful for blocks like Post Content when used in page layouts, and in the Group block and its Row and Stack variations.
Issues that might be resolved or partially resolved by including this block support
How?
dimensionskey to the block supports and to the style object in block attributes, and aminHeightproperty. (I avoided the__experimentalprefix for now, since we've been pretty settled ondimensionsin earlier explorations, but can re-instate it if folks prefer)UnitControlfor the minimum height inputminHeightin the list of controls enabled by the appearance tools opt-inTesting Instructions
Minimum heightcontrol and test that it works as expected.vhunits.Screenshots or screencast