diff --git a/backport-changelog/7.1/12326.md b/backport-changelog/7.1/12326.md new file mode 100644 index 00000000000000..ca87f446b2d844 --- /dev/null +++ b/backport-changelog/7.1/12326.md @@ -0,0 +1,3 @@ +https://github.com/WordPress/wordpress-develop/pull/12326 + +* https://github.com/WordPress/gutenberg/pull/79520 \ No newline at end of file diff --git a/lib/block-supports/layout.php b/lib/block-supports/layout.php index 6a0615ef860220..1968868461aab8 100644 --- a/lib/block-supports/layout.php +++ b/lib/block-supports/layout.php @@ -472,13 +472,16 @@ function gutenberg_get_child_layout_style_rules( $selector, $child_layout, $pare * @return string CSS styles, or empty string. */ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support = false, $gap_value = null, $should_skip_gap_serialization = false, $fallback_gap_value = '0.5em', $block_spacing = null, $options = array() ) { - $base_layout = is_array( $layout ) ? $layout : array(); - $viewport_overrides = $options['viewport_overrides'] ?? null; - $layout_for_styles = null === $viewport_overrides ? $base_layout : array_replace( $base_layout, $viewport_overrides ); - $layout_type = $base_layout['type'] ?? 'default'; - $rules_group = $options['rules_group'] ?? null; - $has_block_gap_override = ! empty( $options['has_block_gap_override'] ); - $should_output_block_gap = null === $viewport_overrides || $has_block_gap_override; + $base_layout = is_array( $layout ) ? $layout : array(); + $viewport_overrides = $options['viewport_overrides'] ?? null; + $layout_for_styles = null === $viewport_overrides ? $base_layout : array_replace( $base_layout, $viewport_overrides ); + $layout_type = $base_layout['type'] ?? 'default'; + $rules_group = $options['rules_group'] ?? null; + $has_block_gap_override = ! empty( $options['has_block_gap_override'] ); + $should_output_block_gap = null === $viewport_overrides || $has_block_gap_override; + // Viewport styles only store changed fields. If a field is present with null, + // the user cleared a value inherited from the default viewport, so check + // whether the key exists rather than whether the value is truthy. $has_viewport_property_override = static function ( $property ) use ( $viewport_overrides ) { return array_key_exists( $property, $viewport_overrides ); }; @@ -521,8 +524,28 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support $wide_size = $layout_for_styles['wideSize'] ?? ''; $justify_content = $layout_for_styles['justifyContent'] ?? 'center'; - $all_max_width_value = $content_size ? $content_size : $wide_size; - $wide_max_width_value = $wide_size ? $wide_size : $content_size; + // Check if viewport-specific ("override") values exist. Null values are valid and mean the user cleared a value inherited from the default viewport. + $has_justify_content_override = null !== $viewport_overrides && $has_viewport_property_override( 'justifyContent' ); + $has_content_size_override = null !== $viewport_overrides && $has_viewport_property_override( 'contentSize' ); + $has_wide_size_override = null !== $viewport_overrides && $has_viewport_property_override( 'wideSize' ); + + /* Styles should be output either if there are no viewport overrides (this is the default case), or if the user has set a new viewport-specific + * value for contentSize or wideSize. If a viewport clears a custom constrained size, reset to the global layout variable. + */ + $should_output_constrained_sizes = null === $viewport_overrides || $has_content_size_override || $has_wide_size_override; + $is_resetting_constrained_sizes = null !== $viewport_overrides && + ( + ( $has_content_size_override && ! $content_size ) || + ( $has_wide_size_override && ! $wide_size ) + ); + + // If a viewport clears a custom constrained size, reset to the global layout variable. + $all_max_width_value = $content_size + ? $content_size + : ( $wide_size && ! $has_content_size_override ? $wide_size : 'var(--wp--style--global--content-size, none)' ); + $wide_max_width_value = $wide_size + ? $wide_size + : ( $content_size && ! $has_wide_size_override ? $content_size : 'var(--wp--style--global--wide-size, none)' ); // Make sure there is a single CSS rule, and all tags are stripped for security. $all_max_width_value = safecss_filter_attr( explode( ';', $all_max_width_value )[0] ); @@ -531,9 +554,7 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support $margin_left = 'left' === $justify_content ? '0 !important' : 'auto !important'; $margin_right = 'right' === $justify_content ? '0 !important' : 'auto !important'; - $has_justify_content_override = null !== $viewport_overrides && $has_viewport_property_override( 'justifyContent' ); - $should_output_constrained_sizes = null === $viewport_overrides || $has_viewport_property_override( 'contentSize' ) || $has_viewport_property_override( 'wideSize' ); - if ( $should_output_constrained_sizes && ( $content_size || $wide_size ) ) { + if ( $should_output_constrained_sizes && ( $content_size || $wide_size || $is_resetting_constrained_sizes ) ) { $content_size_declarations = array( 'max-width' => $all_max_width_value, ); @@ -673,6 +694,9 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support $vertical_alignment_options += array( 'space-between' => 'space-between' ); } + /* Styles should be output either if there are no viewport overrides (this is the default case), or if the user has set a new viewport-specific + * value for any of the flex properties. + */ $should_output_flex_wrap = null === $viewport_overrides || $has_viewport_property_override( 'flexWrap' ); $should_output_flex_orientation = null === $viewport_overrides || $has_viewport_property_override( 'orientation' ); $should_output_flex_justification = null === $viewport_overrides || $has_viewport_property_override( 'justifyContent' ) || $has_viewport_property_override( 'orientation' ); @@ -803,6 +827,9 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support $responsive_gap_value = '0px'; } + /* Styles should be output either if there are no viewport overrides (this is the default case), or if the user has set a new viewport-specific + * value for any of the grid properties. + */ $should_output_grid_columns = null === $viewport_overrides || $has_viewport_property_override( 'minimumColumnWidth' ) || $has_viewport_property_override( 'columnCount' ) || $has_viewport_property_override( 'autoFit' ); $uses_gap_in_grid_columns = ! empty( $layout_for_styles['columnCount'] ) && ! empty( $layout_for_styles['minimumColumnWidth'] ); if ( $has_block_gap_override && $uses_gap_in_grid_columns ) { @@ -812,8 +839,9 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support $should_output_grid_rows = ( null === $viewport_overrides || $has_viewport_property_override( 'rowCount' ) ) && ! empty( $layout_for_styles['columnCount'] ) && ! empty( $layout_for_styles['rowCount'] ); $grid_declarations = array(); - // When enabled, columns stretch to fill the available space using - // `auto-fit`; otherwise empty tracks are preserved with `auto-fill`. + /* When enabled, columns stretch to fill the available space using + * `auto-fit`; otherwise empty tracks are preserved with `auto-fill`. + */ $auto_placement = ! empty( $layout_for_styles['autoFit'] ) ? 'auto-fit' : 'auto-fill'; if ( $should_output_grid_columns && ! empty( $layout_for_styles['columnCount'] ) && ! empty( $layout_for_styles['minimumColumnWidth'] ) ) { diff --git a/packages/block-editor/src/hooks/layout.js b/packages/block-editor/src/hooks/layout.js index 5dff051c20d8ff..70b1d0f5058bf6 100644 --- a/packages/block-editor/src/hooks/layout.js +++ b/packages/block-editor/src/hooks/layout.js @@ -85,7 +85,7 @@ export function getResetLayout( layoutBlockSupport = {}, blockVariation ) { } ); } -function getLayoutStateOverrides( +export function getLayoutStateOverrides( layout = {}, baseLayout = {}, existingLayout = {} @@ -98,11 +98,13 @@ function getLayoutStateOverrides( ); Object.entries( layout || {} ).forEach( ( [ key, value ] ) => { + const baseHasValue = Object.hasOwn( baseLayout || {}, key ); if ( ! CHILD_LAYOUT_KEYS.includes( key ) && value !== baseLayout?.[ key ] ) { - overrides[ key ] = value; + overrides[ key ] = + value === undefined && baseHasValue ? null : value; } } ); @@ -466,7 +468,7 @@ function LayoutPanelPure( { const nextStateStyle = cleanEmptyObject( { ...stateStyle, layout: getLayoutStateOverrides( - cleanEmptyObject( newLayout ), + newLayout, baseLayout, stateStyle?.layout ), diff --git a/packages/block-editor/src/hooks/test/layout.js b/packages/block-editor/src/hooks/test/layout.js index e17e42a4113204..e4b53fca176583 100644 --- a/packages/block-editor/src/hooks/test/layout.js +++ b/packages/block-editor/src/hooks/test/layout.js @@ -1,7 +1,11 @@ /** * Internal dependencies */ -import { getResetLayout, getResponsiveLayoutStyles } from '../layout'; +import { + getLayoutStateOverrides, + getResetLayout, + getResponsiveLayoutStyles, +} from '../layout'; describe( 'layout', () => { describe( 'getResetLayout()', () => { @@ -46,6 +50,28 @@ describe( 'layout', () => { } ); } ); + describe( 'getLayoutStateOverrides()', () => { + it( 'preserves explicit unsets for layout values inherited from the default state', () => { + expect( + getLayoutStateOverrides( + { type: 'grid', columnCount: undefined }, + { type: 'grid', columnCount: 3 } + ) + ).toEqual( { + columnCount: null, + } ); + } ); + + it( 'removes undefined layout values that are not inherited from the default state', () => { + expect( + getLayoutStateOverrides( + { type: 'grid', columnCount: undefined }, + { type: 'grid' } + ) + ).toBeUndefined(); + } ); + } ); + describe( 'getResponsiveLayoutStyles()', () => { it( 'generates responsive block gap styles for flow layouts', () => { expect( @@ -135,6 +161,54 @@ describe( 'layout', () => { ); } ); + it( 'generates responsive auto grid columns when column count is unset', () => { + expect( + getResponsiveLayoutStyles( { + attributes: { + style: { + '@mobile': { + layout: { + columnCount: null, + }, + }, + }, + }, + blockName: 'core/group', + selector: '.wp-container-test', + layout: { type: 'grid', columnCount: 3 }, + hasBlockGapSupport: true, + } ) + ).toBe( + '@media (width <= 480px){.wp-container-test { grid-template-columns: repeat(auto-fill, minmax(min(12rem, 100%), 1fr)); container-type: inline-size; }}' + ); + } ); + + it( 'generates responsive constrained size resets when content width is unset', () => { + const result = getResponsiveLayoutStyles( { + attributes: { + style: { + '@mobile': { + layout: { + contentSize: null, + }, + }, + }, + }, + blockName: 'core/group', + selector: '.wp-container-test', + layout: { type: 'constrained', contentSize: '800px' }, + hasBlockGapSupport: true, + } ); + + expect( result ).toContain( '@media (width <= 480px)' ); + expect( result ).toContain( + 'max-width: var(--wp--style--global--content-size, none);' + ); + expect( result ).toContain( + 'max-width: var(--wp--style--global--wide-size, none);' + ); + } ); + it( 'keeps responsive grid column overrides when block gap is also changed', () => { expect( getResponsiveLayoutStyles( { diff --git a/packages/block-editor/src/layouts/constrained.js b/packages/block-editor/src/layouts/constrained.js index a660752d8538a2..0665a9a6d8491b 100644 --- a/packages/block-editor/src/layouts/constrained.js +++ b/packages/block-editor/src/layouts/constrained.js @@ -30,6 +30,9 @@ import { BlockControls, JustifyContentControl } from '../components'; import { cleanEmptyObject, shouldSkipSerialization } from '../hooks/utils'; import { LAYOUT_DEFINITIONS } from './definitions'; +const GLOBAL_CONTENT_SIZE = 'var(--wp--style--global--content-size, none)'; +const GLOBAL_WIDE_SIZE = 'var(--wp--style--global--wide-size, none)'; + export default { name: 'constrained', label: __( 'Constrained' ), @@ -115,21 +118,23 @@ export default { __next40pxDefaultSize label={ __( 'Content width' ) } labelPosition="top" - value={ contentSize || wideSize || '' } + value={ + contentSize === null + ? '' + : contentSize || wideSize || '' + } onChange={ ( nextWidth ) => { nextWidth = 0 > parseFloat( nextWidth ) ? '0' : nextWidth; - onChange( - cleanEmptyObject( { - ...layout, - contentSize: - nextWidth !== '' - ? nextWidth - : undefined, - } ) - ); + onChange( { + ...layout, + contentSize: + nextWidth !== '' + ? nextWidth + : undefined, + } ); } } units={ units } prefix={ @@ -149,21 +154,23 @@ export default { __next40pxDefaultSize label={ __( 'Wide width' ) } labelPosition="top" - value={ wideSize || contentSize || '' } + value={ + wideSize === null + ? '' + : wideSize || contentSize || '' + } onChange={ ( nextWidth ) => { nextWidth = 0 > parseFloat( nextWidth ) ? '0' : nextWidth; - onChange( - cleanEmptyObject( { - ...layout, - wideSize: - nextWidth !== '' - ? nextWidth - : undefined, - } ) - ); + onChange( { + ...layout, + wideSize: + nextWidth !== '' + ? nextWidth + : undefined, + } ); } } units={ units } prefix={ @@ -273,12 +280,30 @@ export default { const hasJustificationOverride = hasViewportOverrides && hasViewportOverride( 'justifyContent' ); + const hasContentSizeOverride = + hasViewportOverrides && hasViewportOverride( 'contentSize' ); + const hasWideSizeOverride = + hasViewportOverrides && hasViewportOverride( 'wideSize' ); const shouldOutputConstrainedSizes = ! hasViewportOverrides || - hasViewportOverride( 'contentSize' ) || - hasViewportOverride( 'wideSize' ); + hasContentSizeOverride || + hasWideSizeOverride; + const isResettingConstrainedSizes = + hasViewportOverrides && + ( ( hasContentSizeOverride && ! contentSize ) || + ( hasWideSizeOverride && ! wideSize ) ); + const contentMaxWidth = + contentSize || + ( wideSize && ! hasContentSizeOverride + ? wideSize + : GLOBAL_CONTENT_SIZE ); + const wideMaxWidth = + wideSize || + ( contentSize && ! hasWideSizeOverride + ? contentSize + : GLOBAL_WIDE_SIZE ); const constrainedSizeDeclarations = [ - `max-width: ${ contentSize ?? wideSize }`, + `max-width: ${ contentMaxWidth }`, ]; if ( ! hasViewportOverrides || hasJustificationOverride ) { constrainedSizeDeclarations.push( @@ -287,7 +312,8 @@ export default { ); } let output = - shouldOutputConstrainedSizes && ( !! contentSize || !! wideSize ) + shouldOutputConstrainedSizes && + ( !! contentSize || !! wideSize || isResettingConstrainedSizes ) ? ` ${ appendSelectors( selector, @@ -296,7 +322,7 @@ export default { ${ constrainedSizeDeclarations.join( '; ' ) }; } ${ appendSelectors( selector, '> .alignwide' ) } { - max-width: ${ wideSize ?? contentSize }; + max-width: ${ wideMaxWidth }; } ${ appendSelectors( selector, '> .alignfull' ) } { max-width: none; diff --git a/packages/block-editor/src/layouts/grid.js b/packages/block-editor/src/layouts/grid.js index 786b6834724a69..d85ca9949fa730 100644 --- a/packages/block-editor/src/layouts/grid.js +++ b/packages/block-editor/src/layouts/grid.js @@ -469,7 +469,7 @@ function GridLayoutColumnsAndRowsControl( { columnCount: newColumnCount, } ); } } - value={ columnCount } + value={ columnCount ?? '' } min={ 1 } label={ __( 'Columns' ) } hideLabelFromVision={ ! isManualPlacement } diff --git a/packages/block-editor/src/layouts/test/constrained.js b/packages/block-editor/src/layouts/test/constrained.js index 5ac1d00c1874fa..0e4dfc41573154 100644 --- a/packages/block-editor/src/layouts/test/constrained.js +++ b/packages/block-editor/src/layouts/test/constrained.js @@ -1,8 +1,35 @@ +/** + * External dependencies + */ +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +/** + * WordPress dependencies + */ +import { __experimentalToolsPanel as ToolsPanel } from '@wordpress/components'; + /** * Internal dependencies */ import constrained from '../constrained'; +const ConstrainedLayoutInspectorControls = constrained.inspectorControls; +const PANEL_ID = 'test-panel'; + +function renderInspectorControls( props = {} ) { + return render( + + + + ); +} + describe( 'getLayoutStyle', () => { it( 'should return an empty string if no non-default params are provided', () => { const expected = ''; @@ -18,4 +45,68 @@ describe( 'getLayoutStyle', () => { expect( result ).toBe( expected ); } ); + + it( 'should reset constrained sizes when content width is unset in a viewport', () => { + const result = constrained.getLayoutStyle( { + selector: '.my-container', + layout: { type: 'constrained', contentSize: '800px' }, + viewportOverrides: { contentSize: null }, + style: {}, + blockName: 'test-block', + hasBlockGapSupport: false, + layoutDefinitions: undefined, + } ); + + expect( result ).toContain( + 'max-width: var(--wp--style--global--content-size, none);' + ); + expect( result ).toContain( + 'max-width: var(--wp--style--global--wide-size, none);' + ); + } ); +} ); + +describe( 'ConstrainedLayoutInspectorControls', () => { + it( 'renders an unset content width as an empty control value', () => { + renderInspectorControls( { + layout: { type: 'constrained', contentSize: null }, + resetLayout: { type: 'constrained', contentSize: '800px' }, + } ); + + expect( + screen.getByRole( 'spinbutton', { name: 'Content width' } ) + ).toHaveDisplayValue( '' ); + } ); + + it.each( [ + [ 'Content width', 'contentSize' ], + [ 'Wide width', 'wideSize' ], + ] )( + 'preserves a cleared %s value for viewport layout overrides', + async ( label, layoutKey ) => { + const user = userEvent.setup(); + const onChange = jest.fn(); + renderInspectorControls( { + layout: { type: 'constrained', [ layoutKey ]: '800px' }, + resetLayout: { type: 'constrained', [ layoutKey ]: '800px' }, + onChange, + } ); + + await user.click( + screen.getByRole( 'button', { name: /Layout options/i } ) + ); + await user.click( + screen.getByRole( 'menuitemcheckbox', { + name: `Show ${ label }`, + } ) + ); + await user.clear( + screen.getByRole( 'spinbutton', { name: label } ) + ); + + const nextLayout = onChange.mock.lastCall?.[ 0 ]; + expect( Object.hasOwn( nextLayout, layoutKey ) ).toBe( true ); + expect( nextLayout[ layoutKey ] ).toBeUndefined(); + } + ); } ); diff --git a/packages/block-editor/src/layouts/test/grid.js b/packages/block-editor/src/layouts/test/grid.js index ecf69439c024dd..cdf7e42f3afb1b 100644 --- a/packages/block-editor/src/layouts/test/grid.js +++ b/packages/block-editor/src/layouts/test/grid.js @@ -1,8 +1,34 @@ +/** + * External dependencies + */ +import { render, screen } from '@testing-library/react'; + +/** + * WordPress dependencies + */ +import { __experimentalToolsPanel as ToolsPanel } from '@wordpress/components'; + /** * Internal dependencies */ import grid from '../grid'; +const GridLayoutInspectorControls = grid.inspectorControls; +const PANEL_ID = 'test-panel'; + +function renderInspectorControls( props = {} ) { + return render( + + + + ); +} + describe( 'getLayoutStyle', () => { it( 'should return only `grid-template-columns` and `container-type` properties if no non-default params are provided', () => { const expected = `.my-container { grid-template-columns: repeat(auto-fill, minmax(min(12rem, 100%), 1fr)); container-type: inline-size; }`; @@ -79,3 +105,16 @@ describe( 'getLayoutStyle', () => { expect( result ).toBe( expected ); } ); } ); + +describe( 'GridLayoutInspectorControls', () => { + it( 'renders an unset column count as an empty control value', () => { + renderInspectorControls( { + layout: { type: 'grid', columnCount: null }, + resetLayout: { type: 'grid', columnCount: 3 }, + } ); + + expect( + screen.getByRole( 'spinbutton', { name: 'Columns' } ) + ).toHaveDisplayValue( '' ); + } ); +} ); diff --git a/phpunit/block-supports/layout-test.php b/phpunit/block-supports/layout-test.php index fe96b7e2c55e50..fa737062ca8521 100644 --- a/phpunit/block-supports/layout-test.php +++ b/phpunit/block-supports/layout-test.php @@ -147,6 +147,7 @@ public function test_outer_container_not_restored_for_aligned_image_block_with_t 'should_skip_gap_serialization' => false, 'fallback_gap_value' => '0.5em', 'block_spacing' => null, + 'options' => array(), ); /** @@ -168,7 +169,8 @@ public function test_gutenberg_get_layout_style( $args, $expected_output ) { $args['gap_value'], $args['should_skip_gap_serialization'], $args['fallback_gap_value'], - $args['block_spacing'] + $args['block_spacing'], + $args['options'] ); $this->assertSame( $expected_output, $layout_styles ); @@ -256,6 +258,21 @@ public function data_gutenberg_get_layout_style() { ), 'expected_output' => '.wp-layout > :where(:not(.alignleft):not(.alignright):not(.alignfull)){max-width:800px;margin-left:auto !important;margin-right:auto !important;}.wp-layout > .alignwide{max-width:1200px;}.wp-layout .alignfull{max-width:none;}.wp-layout > .alignfull{margin-right:calc(10px * -1);margin-left:calc(20px * -1);}', ), + 'constrained layout with content size unset in viewport' => array( + 'args' => array( + 'selector' => '.wp-layout', + 'layout' => array( + 'type' => 'constrained', + 'contentSize' => '800px', + ), + 'options' => array( + 'viewport_overrides' => array( + 'contentSize' => null, + ), + ), + ), + 'expected_output' => '.wp-layout > :where(:not(.alignleft):not(.alignright):not(.alignfull)){max-width:var(--wp--style--global--content-size, none);}.wp-layout > .alignwide{max-width:var(--wp--style--global--wide-size, none);}.wp-layout .alignfull{max-width:none;}', + ), 'constrained layout with block gap support' => array( 'args' => array( 'selector' => '.wp-layout',