Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for column and row spans in grid children. #6493

Closed
wants to merge 7 commits into from
Closed

Add support for column and row spans in grid children. #6493

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c9a0514
Add support for column and row spans in grid children.
tellthemachines May 3, 2024
66a0692
Check child layout exists before generating classname.
tellthemachines May 7, 2024
ed8549d
Add a test for child layout classes
tellthemachines May 7, 2024
0b97584
Use `wp_unique_prefixed_id` instead of `wp_unique_id`
tellthemachines May 8, 2024
e0c0cba
Convert filter to named function.
tellthemachines May 9, 2024
81a54f5
Update comment and ticket annotation.
tellthemachines May 9, 2024
0926acf
Add unit test for `wp_add_parent_layout_to_parsed_block`
tellthemachines May 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 97 additions & 18 deletions src/wp-includes/block-supports/layout.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,10 @@ function wp_get_layout_style( $selector, $layout, $has_block_gap_support = false

$layout_styles[] = array(
'selector' => $selector,
'declarations' => array( 'grid-template-columns' => 'repeat(auto-fill, minmax(min(' . $minimum_column_width . ', 100%), 1fr))' ),
'declarations' => array(
'grid-template-columns' => 'repeat(auto-fill, minmax(min(' . $minimum_column_width . ', 100%), 1fr))',
'container-type' => 'inline-size',
),
);
}

Expand Down Expand Up @@ -555,45 +558,102 @@ function wp_get_layout_style( $selector, $layout, $has_block_gap_support = false
function wp_render_layout_support_flag( $block_content, $block ) {
$block_type = WP_Block_Type_Registry::get_instance()->get_registered( $block['blockName'] );
$block_supports_layout = block_has_support( $block_type, 'layout', false ) || block_has_support( $block_type, '__experimentalLayout', false );
$layout_from_parent = isset( $block['attrs']['style']['layout']['selfStretch'] ) ? $block['attrs']['style']['layout']['selfStretch'] : null;
$child_layout = isset( $block['attrs']['style']['layout'] ) ? $block['attrs']['style']['layout'] : null;

if ( ! $block_supports_layout && ! $layout_from_parent ) {
if ( ! $block_supports_layout && ! $child_layout ) {
return $block_content;
}

$outer_class_names = array();

if ( 'fixed' === $layout_from_parent || 'fill' === $layout_from_parent ) {
$container_content_class = wp_unique_id( 'wp-container-content-' );
// Child layout specific logic.
if ( $child_layout ) {
$container_content_class = wp_unique_prefixed_id( 'wp-container-content-' );
$child_layout_declarations = array();
$child_layout_styles = array();

$child_layout_styles = array();
$self_stretch = isset( $child_layout['selfStretch'] ) ? $child_layout['selfStretch'] : null;

if ( 'fixed' === $self_stretch && isset( $child_layout['flexSize'] ) ) {
$child_layout_declarations['flex-basis'] = $child_layout['flexSize'];
$child_layout_declarations['box-sizing'] = 'border-box';
} elseif ( 'fill' === $self_stretch ) {
$child_layout_declarations['flex-grow'] = '1';
}

if ( isset( $child_layout['columnSpan'] ) ) {
$column_span = $child_layout['columnSpan'];
$child_layout_declarations['grid-column'] = "span $column_span";
}
if ( isset( $child_layout['rowSpan'] ) ) {
$row_span = $child_layout['rowSpan'];
$child_layout_declarations['grid-row'] = "span $row_span";
}
$child_layout_styles[] = array(
'selector' => ".$container_content_class",
'declarations' => $child_layout_declarations,
);

/*
* If columnSpan is set, and the parent grid is responsive, i.e. if it has a minimumColumnWidth set,
* the columnSpan should be removed on small grids. If there's a minimumColumnWidth, the grid is responsive.
* But if the minimumColumnWidth value wasn't changed, it won't be set. In that case, if columnCount doesn't
* exist, we can assume that the grid is responsive.
*/
if ( isset( $child_layout['columnSpan'] ) && ( isset( $block['parentLayout']['minimumColumnWidth'] ) || ! isset( $block['parentLayout']['columnCount'] ) ) ) {
$column_span_number = floatval( $child_layout['columnSpan'] );
$parent_column_width = isset( $block['parentLayout']['minimumColumnWidth'] ) ? $block['parentLayout']['minimumColumnWidth'] : '12rem';
$parent_column_value = floatval( $parent_column_width );
$parent_column_unit = explode( $parent_column_value, $parent_column_width );

/*
* If there is no unit, the width has somehow been mangled so we reset both unit and value
* to defaults.
* Additionally, the unit should be one of px, rem or em, so that also needs to be checked.
*/
if ( count( $parent_column_unit ) <= 1 ) {
$parent_column_unit = 'rem';
$parent_column_value = 12;
} else {
$parent_column_unit = $parent_column_unit[1];

if ( ! in_array( $parent_column_unit, array( 'px', 'rem', 'em' ), true ) ) {
$parent_column_unit = 'rem';
}
}

/*
* A default gap value is used for this computation because custom gap values may not be
* viable to use in the computation of the container query value.
*/
$default_gap_value = 'px' === $parent_column_unit ? 24 : 1.5;
$container_query_value = $column_span_number * $parent_column_value + ( $column_span_number - 1 ) * $default_gap_value;
$container_query_value = $container_query_value . $parent_column_unit;

if ( 'fixed' === $layout_from_parent && isset( $block['attrs']['style']['layout']['flexSize'] ) ) {
$child_layout_styles[] = array(
'selector' => ".$container_content_class",
'declarations' => array(
'flex-basis' => $block['attrs']['style']['layout']['flexSize'],
'box-sizing' => 'border-box',
),
);
} elseif ( 'fill' === $layout_from_parent ) {
$child_layout_styles[] = array(
'rules_group' => "@container (max-width: $container_query_value )",
'selector' => ".$container_content_class",
'declarations' => array(
'flex-grow' => '1',
'grid-column' => '1/-1',
),
);
}

wp_style_engine_get_stylesheet_from_css_rules(
/*
* Add to the style engine store to enqueue and render layout styles.
* Return styles here just to check if any exist.
*/
$child_css = wp_style_engine_get_stylesheet_from_css_rules(
$child_layout_styles,
array(
'context' => 'block-supports',
'prettify' => false,
)
);

$outer_class_names[] = $container_content_class;
if ( $child_css ) {
$outer_class_names[] = $container_content_class;
}
}

// Prep the processor for modifying the block output.
Expand Down Expand Up @@ -851,6 +911,25 @@ function wp_render_layout_support_flag( $block_content, $block ) {
return $processor->get_updated_html();
}

/*
* Add a `render_block_data` filter to fetch the parent block layout data.
*/
add_filter(
'render_block_data',
function ( $parsed_block, $source_block, $parent_block ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just double-checked the WP coding standards, and it looks like it's discouraged to use anonymous functions in filters:

Closures should not be passed as filter or action callbacks, as removing these via remove_action() / remove_filter() is complex (at this time) (see #46635 for a proposal to address this).

Should we move this to a named function so that plugins (if they really wanted to) are able to remove_filter this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

/*
* Check if the parent block exists and if it has a layout attribute.
* If it does, add the parent layout to the parsed block.
*/
if ( $parent_block && isset( $parent_block->parsed_block['attrs']['layout'] ) ) {
$parsed_block['parentLayout'] = $parent_block->parsed_block['attrs']['layout'];
}
return $parsed_block;
},
10,
3
);

// Register the block support.
WP_Block_Supports::get_instance()->register(
'layout',
Expand Down
3 changes: 3 additions & 0 deletions src/wp-includes/kses.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2441,11 +2441,13 @@ function safecss_filter_attr( $css, $deprecated = '' ) {
'grid-auto-columns',
'grid-column-start',
'grid-column-end',
'grid-column',
'grid-column-gap',
'grid-template-rows',
'grid-auto-rows',
'grid-row-start',
'grid-row-end',
'grid-row',
'grid-row-gap',
'grid-gap',

Expand Down Expand Up @@ -2475,6 +2477,7 @@ function safecss_filter_attr( $css, $deprecated = '' ) {
'z-index',
'box-shadow',
'aspect-ratio',
'container-type',

// Custom CSS properties.
'--*',
Expand Down
22 changes: 22 additions & 0 deletions tests/phpunit/tests/block-supports/layout.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ public function test_outer_container_not_restored_for_aligned_image_block_with_t
* @param string $expected_output The expected output.
*/
public function test_layout_support_flag_renders_classnames_on_wrapper( $args, $expected_output ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I notice this function includes references to the tickets directly above it. Should the linked trac ticket be added to that list?

switch_theme( 'default' );
$actual_output = wp_render_layout_support_flag( $args['block_content'], $args['block'] );
$this->assertSame( $expected_output, $actual_output );
}
Expand Down Expand Up @@ -251,6 +252,27 @@ public function data_layout_support_flag_renders_classnames_on_wrapper() {
),
'expected_output' => '<div class="wp-block-group"><div class="wp-block-group__inner-wrapper is-layout-flow wp-block-group-is-layout-flow"></div></div>',
),
'block with child layout' => array(
'args' => array(
'block_content' => '<p>Some text.</p>',
'block' => array(
'blockName' => 'core/paragraph',
'attrs' => array(
'style' => array(
'layout' => array(
'columnSpan' => '2',
),
),
),
'innerBlocks' => array(),
'innerHTML' => '<p>Some text.</p>',
'innerContent' => array(
'<p>Some text.</p>',
),
),
),
'expected_output' => '<p class="wp-container-content-1">Some text.</p>', // The generated classname number assumes `wp_unique_id` will not have run previously in this test.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny nit: the code now uses wp_unique_prefixed_id so should the comment refer to that now, too?

Suggested change
'expected_output' => '<p class="wp-container-content-1">Some text.</p>', // The generated classname number assumes `wp_unique_id` will not have run previously in this test.
'expected_output' => '<p class="wp-container-content-1">Some text.</p>', // The generated classname number assumes `wp_unique_prefixed_id` will not have run previously in this test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably worth including the prefix in Andrew's comment too, wp_unique_prefixed_id( 'wp-container-content-' ), as the counter is unique to each prefix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks!

),
'skip classname output if block does not support layout and there are no child layout classes to be output' => array(
'args' => array(
'block_content' => '<p>A paragraph</p>',
Expand Down
Loading