Try adding a grid layout type #49018
Next
Next commit
Try adding a grid layout type.
- Loading branch information
commit 7c3ec88de3bfee89022ba48eab7afeaa71b65c34
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| /** | ||
| * WordPress dependencies | ||
| */ | ||
| import { __ } from '@wordpress/i18n'; | ||
|
|
||
| import { | ||
| Flex, | ||
| FlexItem, | ||
| __experimentalUnitControl as UnitControl, | ||
| } from '@wordpress/components'; | ||
|
|
||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import { appendSelectors, getBlockGapCSS } from './utils'; | ||
| import { getGapCSSValue } from '../hooks/gap'; | ||
| import { shouldSkipSerialization } from '../hooks/utils'; | ||
|
|
||
| export default { | ||
| name: 'grid', | ||
| label: __( 'Grid' ), | ||
| inspectorControls: function GridLayoutInspectorControls( { | ||
| layout = {}, | ||
| onChange, | ||
| } ) { | ||
| return ( | ||
| <> | ||
| <Flex> | ||
| <FlexItem> | ||
| <GridLayoutMinimumWidthControl | ||
| layout={ layout } | ||
| onChange={ onChange } | ||
| /> | ||
| </FlexItem> | ||
| </Flex> | ||
| </> | ||
| ); | ||
| }, | ||
| toolBarControls: function DefaultLayoutToolbarControls() { | ||
| return null; | ||
| }, | ||
| getLayoutStyle: function getLayoutStyle( { | ||
| selector, | ||
| layout, | ||
| style, | ||
| blockName, | ||
| hasBlockGapSupport, | ||
| layoutDefinitions, | ||
| } ) { | ||
| const { minimumColumnWidth = '12rem' } = layout; | ||
|
|
||
| // If a block's block.json skips serialization for spacing or spacing.blockGap, | ||
| // don't apply the user-defined value to the styles. | ||
| const blockGapValue = | ||
| style?.spacing?.blockGap && | ||
| ! shouldSkipSerialization( blockName, 'spacing', 'blockGap' ) | ||
| ? getGapCSSValue( style?.spacing?.blockGap, '0.5em' ) | ||
| : undefined; | ||
|
|
||
| let output = ''; | ||
| const rules = []; | ||
|
|
||
| if ( minimumColumnWidth ) { | ||
| rules.push( | ||
| `grid-template-columns: repeat(auto-fill, minmax(${ minimumColumnWidth }, 1fr))` | ||
| ); | ||
|
Contributor
There was a problem hiding this comment. Just playing around with this again, and I noticed that if we set a minimum column width that is greater than the viewport width, that the children will overflow the container:
Is it worth trying to add a guardrail to the rule so that the minimum value can never be more than 100%? Would it be possible to nest
That seems to work for me locally:
Contributor
Author
There was a problem hiding this comment. Ooooh good catch! I'll have a play with it
Contributor
Author
There was a problem hiding this comment. It worked, thanks!
Contributor
There was a problem hiding this comment. Yay, glad it was a simple fix! |
||
| } | ||
|
|
||
| if ( rules.length ) { | ||
| output = `${ appendSelectors( selector ) } { | ||
| ${ rules.join( '; ' ) }; | ||
| }`; | ||
| } | ||
|
|
||
| // Output blockGap styles based on rules contained in layout definitions in theme.json. | ||
| if ( hasBlockGapSupport && blockGapValue ) { | ||
| output += getBlockGapCSS( | ||
| selector, | ||
| layoutDefinitions, | ||
| 'grid', | ||
| blockGapValue | ||
| ); | ||
| } | ||
| return output; | ||
| }, | ||
| getOrientation() { | ||
| return null; | ||
| }, | ||
| getAlignments() { | ||
| return []; | ||
| }, | ||
| }; | ||
|
|
||
| // Enables setting minimum width of grid items. | ||
| function GridLayoutMinimumWidthControl( { layout, onChange } ) { | ||
| const { minimumColumnWidth = '20rem' } = layout; | ||
|
|
||
| return ( | ||
| <UnitControl | ||
| size={ '__unstable-large' } | ||
|
||
| onChange={ ( value ) => { | ||
| onChange( { | ||
| ...layout, | ||
| minimumColumnWidth: value, | ||
| } ); | ||
| } } | ||
| value={ minimumColumnWidth } | ||
| /> | ||
| ); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a future proofing question: if
minimumColumnWidthisn't set, this assumes a default of12rem. If we want to enable the control over individual columns again in follow-ups, how will we tell that we shouldn't be outputting12rem?By which I mean, what are the trade-offs between using
12remas an inferred value, versus explicitly defining12remwithin the Grid variation? For example, what if other blocks wished to use a different default minimum column width?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be pretty easy to introduce something like
isResponsiveagain, or any other property that indicates that the grid type is not based on column width (which would always be the default). We'll need a check before outputting the value because the whole template-columns declaration will be different for a static grid, and even if aminimumColumnWidthis set (e.g. if we don't reset it on switching to another grid type) we will only use the value if the grid is of the responsive type (or whatever we want to call it).I chose 12rem arbitrarily, it could be any other value but we do need to have some default value here or the
auto-fillwon't work.If other blocks want to use a different value they can set it when they define the layout type, e.g.
layout: { type: 'grid', minimumColumnWidth: '9rem' }.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So (just to make sure I'm following), we're essentially saying, a grid layout with absolutely no other values will default to
12remminimum column width. For future changes, we're adding additional attributes that will be set, so it'll be easy to determine when to switch off the current logic. And for changing the12remdefault on a block-by-block basis, we provide the default type type value as you mention.Sounds solid to me! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the idea is to have a sensible default so you can just add a grid and it'll look ok for the majority of cases? Then progressively add configurability.