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 span in grid children. #58539

Merged
merged 26 commits into from
Feb 14, 2024
Merged

Add support for column and row span in grid children. #58539

Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
f0e6c77
Add support for column and row span in grid children.
tellthemachines Feb 1, 2024
d746fb5
Column styles plus row styles
tellthemachines Feb 1, 2024
514c16c
Define array
tellthemachines Feb 1, 2024
578900a
Array should be wrapped in an array
tellthemachines Feb 5, 2024
1832758
Remove booboo
tellthemachines Feb 5, 2024
cb72ade
Add container queries to determine column span breakpoints
tellthemachines Feb 6, 2024
2a113cd
Fix empty space output.
tellthemachines Feb 7, 2024
43dfae2
Add isset
tellthemachines Feb 7, 2024
35bff74
Fix column width logic
tellthemachines Feb 7, 2024
a0449e9
Replace "query" with "at_rule"
tellthemachines Feb 7, 2024
38c7c27
Update variable name.
tellthemachines Feb 7, 2024
ea39794
Refine resizing behaviour.
tellthemachines Feb 8, 2024
7518c85
Add container query to the editor.
tellthemachines Feb 8, 2024
a70ddeb
Minimum col and row span
tellthemachines Feb 8, 2024
23b8fc8
Fix resets
tellthemachines Feb 8, 2024
9d86505
Reset child layout every time it changes.
tellthemachines Feb 8, 2024
8edd002
Fix JS rule output and update test.
tellthemachines Feb 9, 2024
50f5f1b
Add more tests.
tellthemachines Feb 9, 2024
d69b14d
Remove style engine changes from this PR.
tellthemachines Feb 9, 2024
a8fc9f9
fix conflict
tellthemachines Feb 9, 2024
f253b13
Check both value and unit are viable for the computation.
tellthemachines Feb 9, 2024
16b5753
gap value should be number
tellthemachines Feb 9, 2024
24a76f9
Fix up the editor side logic
tellthemachines Feb 9, 2024
cf964d9
Move child layout control above min-height
tellthemachines Feb 12, 2024
4efe2ec
Merge remote-tracking branch 'origin/trunk' into add/grid-child-spans
tellthemachines Feb 14, 2024
56f2934
change param
tellthemachines Feb 14, 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
110 changes: 82 additions & 28 deletions lib/block-supports/layout.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,10 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support

$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 @@ -557,44 +560,95 @@ function gutenberg_incremental_id_per_prefix( $prefix = '' ) {
function gutenberg_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, array( 'layout' ), false ) || block_has_support( $block_type, array( '__experimentalLayout' ), false );
$layout_from_parent = $block['attrs']['style']['layout']['selfStretch'] ?? null;
// If there is any value in style -> layout, the block has a child layout.
$child_layout = $block['attrs']['style']['layout'] ?? null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Now whenever I see the nullish coalescing operator, all I think is "one day we'll be able to use this properly"! 😄

(This one seems fine to leave in to me since it was already there)


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

$outer_class_names = array();
$outer_class_names = array();
$container_content_class = wp_unique_id( 'wp-container-content-' );
$child_layout_declarations = array();
$child_layout_styles = array();

if ( 'fixed' === $layout_from_parent || 'fill' === $layout_from_parent ) {
$container_content_class = wp_unique_id( 'wp-container-content-' );
$self_stretch = isset( $block['attrs']['style']['layout']['selfStretch'] ) ? $block['attrs']['style']['layout']['selfStretch'] : null;

$child_layout_styles = array();
if ( 'fixed' === $self_stretch && isset( $block['attrs']['style']['layout']['flexSize'] ) ) {
$child_layout_declarations['flex-basis'] = $block['attrs']['style']['layout']['flexSize'];
$child_layout_declarations['box-sizing'] = 'border-box';
} elseif ( 'fill' === $self_stretch ) {
$child_layout_declarations['flex-grow'] = '1';
}

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(
'selector' => ".$container_content_class",
'declarations' => array(
'flex-grow' => '1',
),
);
if ( isset( $block['attrs']['style']['layout']['columnSpan'] ) ) {
$column_span = $block['attrs']['style']['layout']['columnSpan'];
$child_layout_declarations['grid-column'] = "span $column_span";
}
if ( isset( $block['attrs']['style']['layout']['rowSpan'] ) ) {
$row_span = $block['attrs']['style']['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 ( isset( $block['attrs']['style']['layout']['columnSpan'] ) && isset( $block['attrs']['style']['layout']['parentColumnWidth'] ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a blocker for now, since we could always change it in a follow-up if it were possible, but I wonder if there'd be a way to pass down the parent's style or layout attributes by auto-enabling blocks in to providesContext for these block supports properties? A bit like how the Query block passes down the queryId... so we'd then access it via something like $block->context['layout'] or $block->context['style'] or something like that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm providesContext is read from the blockType which I'm not sure can/should be dynamically changed; all the blocks that use it have it declared in their block.json.

$column_span_number = floatval( $block['attrs']['style']['layout']['columnSpan'] );
$parent_column_width = $block['attrs']['style']['layout']['parentColumnWidth'];
$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';
}
}

gutenberg_style_engine_get_stylesheet_from_css_rules(
$child_layout_styles,
array(
'context' => 'block-supports',
'prettify' => false,
)
/**
* 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.
Comment on lines +623 to +624
Copy link
Contributor

Choose a reason for hiding this comment

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

This only really impacts the max-width value, right? I was just wondering if we could look up to find the real value, but I imagine it's probably fine to use the default here for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure, for example, could gap ever be a calc()? There's also the issue of how to deal with relative units like % and vw because those won't work for the purpose of this calculation 😅

*/
$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;
andrewserong marked this conversation as resolved.
Show resolved Hide resolved
$container_query_value = $container_query_value . $parent_column_unit;
andrewserong marked this conversation as resolved.
Show resolved Hide resolved

$child_layout_styles[] = array(
'at_rule' => "@container (max-width: $container_query_value )",
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this'll need a rebase now that #58922 has landed and for this to be updated to rules_group?

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!

'selector' => ".$container_content_class",
'declarations' => array(
'grid-column' => '1/-1',
),
);
}

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

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

Expand Down
14 changes: 14 additions & 0 deletions lib/experimental/kses.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,17 @@ function allow_filter_in_styles( $allow_css, $css_test_string ) {
}
}
add_filter( 'safecss_filter_attr_allow_css', 'allow_filter_in_styles', 10, 2 );

/**
* Update allowed inline style attributes list.
*
* @param string[] $attrs Array of allowed CSS attributes.
* @return string[] CSS attributes.
*/
function gutenberg_safe_grid_attrs( $attrs ) {
$attrs[] = 'grid-column';
$attrs[] = 'grid-row';
$attrs[] = 'container-type';
return $attrs;
}
add_filter( 'safe_style_css', 'gutenberg_safe_grid_attrs' );
137 changes: 93 additions & 44 deletions packages/block-editor/src/components/child-layout-control/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import {
__experimentalToggleGroupControl as ToggleGroupControl,
__experimentalToggleGroupControlOption as ToggleGroupControlOption,
__experimentalUnitControl as UnitControl,
__experimentalInputControl as InputControl,
__experimentalHStack as HStack,
} from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { useEffect } from '@wordpress/element';
Expand Down Expand Up @@ -38,7 +40,18 @@ export default function ChildLayoutControl( {
onChange,
parentLayout,
} ) {
const { selfStretch, flexSize } = childLayout;
const { selfStretch, flexSize, columnSpan, rowSpan } = childLayout;
const {
type: parentLayoutType,
minimumColumnWidth = '12rem',
columnCount,
} = parentLayout;

/**
* If columnCount is set, the parentColumnwidth isn't needed because
* the grid has a fixed number of columns with responsive widths.
*/
const parentColumnWidth = columnCount ? null : minimumColumnWidth;

useEffect( () => {
if ( selfStretch === 'fixed' && ! flexSize ) {
Expand All @@ -51,49 +64,85 @@ export default function ChildLayoutControl( {

return (
<>
<ToggleGroupControl
__nextHasNoMarginBottom
size={ '__unstable-large' }
label={ childLayoutOrientation( parentLayout ) }
value={ selfStretch || 'fit' }
help={ helpText( selfStretch, parentLayout ) }
onChange={ ( value ) => {
const newFlexSize = value !== 'fixed' ? null : flexSize;
onChange( {
...childLayout,
selfStretch: value,
flexSize: newFlexSize,
} );
} }
isBlock={ true }
>
<ToggleGroupControlOption
key={ 'fit' }
value={ 'fit' }
label={ __( 'Fit' ) }
/>
<ToggleGroupControlOption
key={ 'fill' }
value={ 'fill' }
label={ __( 'Fill' ) }
/>
<ToggleGroupControlOption
key={ 'fixed' }
value={ 'fixed' }
label={ __( 'Fixed' ) }
/>
</ToggleGroupControl>
{ selfStretch === 'fixed' && (
<UnitControl
size={ '__unstable-large' }
onChange={ ( value ) => {
onChange( {
...childLayout,
flexSize: value,
} );
} }
value={ flexSize }
/>
{ parentLayoutType === 'flex' && (
<>
<ToggleGroupControl
__nextHasNoMarginBottom
size={ '__unstable-large' }
label={ childLayoutOrientation( parentLayout ) }
value={ selfStretch || 'fit' }
help={ helpText( selfStretch, parentLayout ) }
onChange={ ( value ) => {
const newFlexSize =
value !== 'fixed' ? null : flexSize;
onChange( {
selfStretch: value,
flexSize: newFlexSize,
} );
} }
isBlock={ true }
>
<ToggleGroupControlOption
key={ 'fit' }
value={ 'fit' }
label={ __( 'Fit' ) }
/>
<ToggleGroupControlOption
key={ 'fill' }
value={ 'fill' }
label={ __( 'Fill' ) }
/>
<ToggleGroupControlOption
key={ 'fixed' }
value={ 'fixed' }
label={ __( 'Fixed' ) }
/>
</ToggleGroupControl>
{ selfStretch === 'fixed' && (
<UnitControl
size={ '__unstable-large' }
onChange={ ( value ) => {
onChange( {
selfStretch,
flexSize: value,
} );
} }
value={ flexSize }
/>
) }
</>
) }
{ parentLayoutType === 'grid' && (
<HStack>
<InputControl
size={ '__unstable-large' }
label={ __( 'Column Span' ) }
type="number"
noisysocks marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, not a blocker for this PR, but it might be good to swap this out for NumberControl after all to hide the browser default number spinners. Otherwise with one of these fields focused, if you go to scroll the mouse, the number can scroll unexpectedly:

2024-02-14.15.12.18.mp4

Alternately we could potentially add a max value, too? In any case, not to worry about for this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm I don't think hiding the spinners will remove the scrolling behaviour, at least playing with storybook it seems to persist. I'm not sure we'd want to remove it in any case? It's default input behaviour so folks will be expecting it to work.

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 default input behaviour so folks will be expecting it to work.

Ah, fair enough! Might just be me who found it a bit confusing 🙂

onChange={ ( value ) => {
onChange( {
rowSpan,
columnSpan: value,
parentColumnWidth,
Copy link
Member

Choose a reason for hiding this comment

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

Is it necessary to store parentColumnWidth into style here? Could the PHP calculate it just in time from columnCount and minimumColumnWidth?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

parentColumnWidth is exactly the parent's minimumColumnWidth. It's being added as an attribute here so we can access it when render_layout_support_flag runs, because at that point we only know about the block being processed, and have no access to its ancestor tree.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's being cached here will that be a problem if someone goes to update the parent column width after they've set the row/column span, or if they drag and drop between two grids?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's being cached after drag. I'm testing some nested layouts and have dragged a child to another grid parent and it still has the value of the former parent's minimumColumnWidth in parentColumnWidth

"layout":{"columnSpan":"3","parentColumnWidth":"300px"}}

Is that what you mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, that's what I was getting at. The parentColumnWidth should be the current parent's column width, but the approach right now means that it only gets set when the column and row span values are updated 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh sorry I didn't see this one. It seems pretty unlikely someone will just drag and drop a block with a column span from one grid to another differently sized grid and not make any further adjustments (and if both grids are the same size it won't be an issue) but I'm open to suggestions if anyone can find a better way to do it!

Even if someone does drag and drop a multi-column block between differently sized grids the very worst that can happen is the container query gets a bit wonky and doesn't break at exactly the point it should 🤷

Copy link
Member

Choose a reason for hiding this comment

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

so we can access it when render_layout_support_flag runs, because at that point we only know about the block being processed, and have no access to its ancestor tree.

Oh really? Bugger. That seems like a shortcoming of the render_block filter that's maybe worth fixing. We maybe could work around it by using render_block_data which is passed $parent_block.

https://github.com/WordPress/wordpress-develop/blob/8ec4d9dfc7e2043485d83d4af9e505ec1cc21470/src/wp-includes/class-wp-block.php#L442

add_filter( 'render_block_data', function( $parsed_block, $source_block, $parent_block ) {
    $parsed_block['parent_block'] = $parent_block;
}, 10, 3 );

add_filter( 'render_block', function( $block_content, $block ) {
    var_dump($block['parent_block']);
}, 10, 2 );

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be clear, is that something you think should be addressed now, as part of this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

To be clear, is that something you think should be addressed now, as part of this PR?

IMO, I don't think it'd be a blocker to landing, but would be good to look at in a follow-up before Grid is taken out of experimental.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I might do this as a follow-up because if we want to remove the parentColumnWidth attrib altogether it'll need some work in both editor and front-end logic.

} );
} }
value={ columnSpan }
min={ 1 }
/>
<InputControl
size={ '__unstable-large' }
label={ __( 'Row Span' ) }
type="number"
onChange={ ( value ) => {
onChange( {
columnSpan,
parentColumnWidth,
rowSpan: value,
} );
} }
value={ rowSpan }
min={ 1 }
/>
</HStack>
) }
</>
);
Expand Down
Loading
Loading