Skip to content

Commit

Permalink
Attempt to get unset rules playing nicely in the editor with global s…
Browse files Browse the repository at this point in the history
…tyles
  • Loading branch information
andrewserong committed Jan 19, 2024
1 parent ef7149d commit 2c9107d
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 41 deletions.
Expand Up @@ -378,7 +378,6 @@ export default function DimensionsPanel( {
const aspectRatioValue = decodeValue(
inheritedValue?.dimensions?.aspectRatio
);
// Gotta change something here
const setAspectRatioValue = ( newValue ) => {
const tempValue = setImmutably(
value,
Expand Down
Expand Up @@ -434,6 +434,12 @@ export function getStylesDeclarations(
);
}

// For aspect ratio to work, other dimensions rules (and Cover block defaults) must be unset.
// This ensures that a fixed height does not override the aspect ratio.
if ( cssProperty === 'aspect-ratio' ) {
output.push( 'min-height: unset' );
}

output.push( `${ cssProperty }: ${ ruleValue }` );
} );

Expand Down
24 changes: 21 additions & 3 deletions packages/block-editor/src/hooks/dimensions.js
Expand Up @@ -157,13 +157,13 @@ export function hasDimensionsSupport( blockName, feature = 'any' ) {

export default {
useBlockProps,
attributeKeys: [ 'style' ],
attributeKeys: [ 'minHeight', 'style' ],
hasSupport( name ) {
return hasDimensionsSupport( name, 'aspectRatio' );
},
};

function useBlockProps( { name, style } ) {
function useBlockProps( { name, minHeight, style } ) {
if (
! hasDimensionsSupport( name, 'aspectRatio' ) ||
shouldSkipSerialization( name, DIMENSIONS_SUPPORT_KEY, 'aspectRatio' )
Expand All @@ -175,7 +175,25 @@ function useBlockProps( { name, style } ) {
'has-aspect-ratio': !! style?.dimensions?.aspectRatio,
} );

return { className };
// Allow dimensions-based inline style overrides to override any global styles rules that
// might be set for the block, and therefore affect the display of the aspect ratio.
const inlineStyleOverrides = {};

// Apply rules to unset incompatible styles.
// Note that a set `aspectRatio` will win out if both an aspect ratio and a minHeight 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 minHeight. The Cover block
// and dimensions controls have logic that will manually clear the aspect ratio if a minHeight
// is set.
if ( style?.dimensions?.aspectRatio ) {
// To ensure the aspect ratio does not get overridden by `minHeight` unset any existing rule.
inlineStyleOverrides.minHeight = 'unset';
} else if ( minHeight || style?.dimensions?.minHeight ) {
// To ensure the minHeight does not get overridden by `aspectRatio` unset any existing rule.
inlineStyleOverrides.aspectRatio = 'unset';
}

return { className, style: inlineStyleOverrides };
}

/**
Expand Down
7 changes: 1 addition & 6 deletions packages/block-library/src/cover/style.scss
Expand Up @@ -2,6 +2,7 @@
.wp-block-cover {
position: relative;
background-position: center center;
min-height: 430px;
display: flex;
justify-content: center;
align-items: center;
Expand All @@ -15,12 +16,6 @@
// Keep the flex layout direction to the physical direction (LTR) in RTL languages.
/*rtl:raw: direction: ltr; */

// When an aspect ratio is set, the height is calculated based on the width, so don't
// manually set a min-height.
&:where(:not(.has-aspect-ratio)) {
min-height: 430px;
}

/**
* Set a default background color for has-background-dim _unless_ it includes another
* background-color class (e.g. has-green-background-color). The presence of another
Expand Down
6 changes: 0 additions & 6 deletions packages/style-engine/class-wp-style-engine.php
Expand Up @@ -203,12 +203,6 @@ final class WP_Style_Engine {
'spacing' => '--wp--preset--spacing--$slug',
),
),
'width' => array(
'property_keys' => array(
'default' => 'width',
),
'path' => array( 'dimensions', 'width' ),
),
),
'spacing' => array(
'padding' => array(
Expand Down
31 changes: 6 additions & 25 deletions packages/style-engine/src/styles/dimensions/index.ts
@@ -1,7 +1,7 @@
/**
* Internal dependencies
*/
import type { GeneratedCSSRule, Style, StyleOptions } from '../../types';
import type { Style, StyleOptions } from '../../types';
import { generateRule } from '../utils';

const minHeight = {
Expand All @@ -19,31 +19,12 @@ const minHeight = {
const aspectRatio = {
name: 'aspectRatio',
generate: ( style: Style, options: StyleOptions ) => {
const _aspectRatio = style?.dimensions?.aspectRatio;

const styleRules: GeneratedCSSRule[] = [];

if ( ! _aspectRatio ) {
return styleRules;
}

// To ensure the aspect ratio does not get overridden by `minHeight` unset any existing rule.
styleRules.push( {
selector: options.selector,
key: 'minHeight',
value: 'unset',
} );

styleRules.push(
...generateRule(
style,
options,
[ 'dimensions', 'aspectRatio' ],
'aspectRatio'
)
return generateRule(
style,
options,
[ 'dimensions', 'aspectRatio' ],
'aspectRatio'
);

return styleRules;
},
};

Expand Down

0 comments on commit 2c9107d

Please sign in to comment.