Add height block support to dimension #71914
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| https://github.com/WordPress/wordpress-develop/pull/10474 | ||
|
|
||
| * https://github.com/WordPress/gutenberg/pull/71914 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -159,6 +159,7 @@ export function hasDimensionsSupport( blockName, feature = 'any' ) { | |
| if ( feature === 'any' ) { | ||
| return !! ( | ||
| support?.aspectRatio || | ||
| !! support?.height || | ||
| !! support?.minHeight || | ||
| !! support?.width | ||
| ); | ||
|
|
@@ -169,13 +170,13 @@ export function hasDimensionsSupport( blockName, feature = 'any' ) { | |
|
|
||
| export default { | ||
| useBlockProps, | ||
| attributeKeys: [ 'height', 'minHeight', 'width', 'style' ], | ||
|
Member
There was a problem hiding this comment. Just checking: do we need to add logic similar to what's below (around line 201) for minHeight to handle height + aspectRatio conflicts? E.g., unsetting height when aspectRatio is set, or vice versa?
Contributor
There was a problem hiding this comment. Yeah, quite possibly. I'll work on updating that soon. |
||
| hasSupport( name ) { | ||
| return hasDimensionsSupport( name ); | ||
| }, | ||
| }; | ||
|
|
||
| function useBlockProps( { name, height, minHeight, style } ) { | ||
| if ( | ||
| ! hasDimensionsSupport( name, 'aspectRatio' ) || | ||
| shouldSkipSerialization( name, DIMENSIONS_SUPPORT_KEY, 'aspectRatio' ) | ||
|
|
@@ -192,16 +193,22 @@ function useBlockProps( { name, minHeight, style } ) { | |
| const inlineStyleOverrides = {}; | ||
|
|
||
| // Apply rules to unset incompatible styles. | ||
| // Note that a set `aspectRatio` will win out if both an aspect ratio and height-related properties are set. | ||
| // This is because the aspect ratio is a newer block support, so (in theory) any aspect ratio | ||
| // that is set should be intentional and should override any existing height properties. The Cover block | ||
| // and dimensions controls have logic that will manually clear the aspect ratio if height properties | ||
| // are set. | ||
| if ( style?.dimensions?.aspectRatio ) { | ||
| // To ensure the aspect ratio does not get overridden by `minHeight` or `height` unset any existing rule. | ||
| inlineStyleOverrides.minHeight = 'unset'; | ||
| inlineStyleOverrides.height = 'unset'; | ||
|
Contributor
There was a problem hiding this comment. I wonder why we're doing these things at the rendering level, can't we "unset" in the UI? Or do we even need to be this smart? (Maybe we do, just asking)
Contributor
There was a problem hiding this comment. I don't have full context on the original decision to handle this in this manner but @andrewserong might be able to shed further light. The PR introducing aspect ratio support was #56897 and details the following for the "why" behind it.
Before seeing that, my understanding was that the chosen approach was based on the block inspector not having access to the full set of global styles or third-party stylesheets. Now that global styles data is available in the post editor, most use cases should be addressable directly through the UI. This also relates to how style inheritance is handled in the inspector. With those values exposed, the UI could more reliably determine things like aspect ratio versus min-height versus height, even if theme-enqueued stylesheets remain outside that scope. Overall, the current approach seems slightly more robust. |
||
| } else if ( | ||
| minHeight || | ||
| style?.dimensions?.minHeight || | ||
| height || | ||
| style?.dimensions?.height | ||
| ) { | ||
| // To ensure height properties do not get overridden by `aspectRatio` unset any existing rule. | ||
| inlineStyleOverrides.aspectRatio = 'unset'; | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.