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

Layout child fixed size should not be fixed by default and should always have a value set #46139

Merged
merged 5 commits into from
Dec 4, 2022
+17 −4
Merged

Layout child fixed size should not be fixed by default and should always have a value set #46139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
477908f
Change behaviour of fixed child size to not be fixed by default
tellthemachines Nov 29, 2022
07de731
Fix boo-boo
tellthemachines Nov 29, 2022
40d0ac2
Remove unshrink control
tellthemachines Nov 29, 2022
2ec5ac7
Determine fixed size automatically
tellthemachines Nov 30, 2022
c9e50f7
Revert "Determine fixed size automatically"
tellthemachines Dec 4, 2022
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
5 changes: 2 additions & 3 deletions lib/block-supports/layout.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,9 +337,8 @@ function gutenberg_render_layout_support_flag( $block_content, $block ) {
$child_layout_styles[] = array(
'selector' => ".$container_content_class",
'declarations' => array(
'flex-shrink' => '0',
'flex-basis' => $block['attrs']['style']['layout']['flexSize'],
'box-sizing' => 'border-box',
'flex-basis' => $block['attrs']['style']['layout']['flexSize'],
'box-sizing' => 'border-box',
),
);
} elseif ( 'fill' === $block['attrs']['style']['layout']['selfStretch'] ) {
Expand Down
15 changes: 15 additions & 0 deletions packages/block-editor/src/hooks/child-layout.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
__experimentalUnitControl as UnitControl,
} from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { useEffect } from '@wordpress/element';

/**
* Internal dependencies
Expand Down Expand Up @@ -43,6 +44,20 @@ export function ChildLayoutEdit( {
const { layout: childLayout = {} } = style;
const { selfStretch, flexSize } = childLayout;

useEffect( () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I remember we needed to remove the useEffect in the Group block back in #44176. Will adding a useEffect here have similar performance concerns as the earlier Group block useEffect?

I'm wondering if we need to change the attribute via setAttribute or if it'd be possible to use a different temporary variable for determining which control to display, if it's more about which tab is shown when the block is initially selected, rather than changing the attributes for the block 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Offhand I'd think not because this control only loads once the block is selected. I think the perf issues we were seeing with using an effect in the Group edit function were related to the editor content taking longer to load.

In any case, I'm happy to change if there's a better approach! I played around with it a bit but couldn't find another way to tell if the component is rendering for the first time, or just updating as a result of a change of value.

Copy link
Contributor

Choose a reason for hiding this comment

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

Offhand I'd think not because this control only loads once the block is selected.

Ah, good point, yes, the rendering is neatly tucked away inside the DimensionsPanel isn't it — sounds like we can go with this approach for now and revisit if need be 👍

if ( selfStretch === 'fixed' && ! flexSize ) {
setAttributes( {
style: {
...style,
layout: {
...childLayout,
selfStretch: 'fit',
},
},
} );
}
}, [] );

return (
<>
<ToggleGroupControl
Expand Down
1 change: 0 additions & 1 deletion packages/block-editor/src/hooks/layout.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,6 @@ export const withChildLayoutStyles = createHigherOrderComponent(

if ( selfStretch === 'fixed' && flexSize ) {
css += `${ selector } {
flex-shrink: 0;
flex-basis: ${ flexSize };
box-sizing: border-box;
}`;
Expand Down