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

Grid layout: Use numbers instead of strings for layout attributes #63112

Merged
merged 7 commits into from
Jul 8, 2024
Merged

Grid layout: Use numbers instead of strings for layout attributes #63112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2e58915
Grid layout: Use numbers instead of strings for layout attributes
noisysocks Jul 4, 2024
844dbd9
I am dumb
noisysocks Jul 4, 2024
f7f6c63
Update migration comment
noisysocks Jul 5, 2024
9f90065
Unset columnCount instead of setting it to null
noisysocks Jul 5, 2024
019a5cb
Unset minimumColumnWidth instead of setting it to an empty string
noisysocks Jul 5, 2024
35db978
Show slider at 0 if there is no columnCount set
noisysocks Jul 5, 2024
46993b5
Handle empty strings and invalid numbers in migration
noisysocks Jul 5, 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
47 changes: 28 additions & 19 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 @@ -193,16 +193,15 @@ function GridControls( {
panelId,
} ) {
const { columnStart, rowStart, columnSpan, rowSpan } = childLayout;
const { columnCount } = parentLayout ?? {};
const gridColumnNumber = parseInt( columnCount, 10 ) || 3;
const { columnCount = 3, rowCount } = parentLayout ?? {};
const rootClientId = useSelect( ( select ) =>
select( blockEditorStore ).getBlockRootClientId( panelId )
);
const { moveBlocksToPosition, __unstableMarkNextChangeAsNotPersistent } =
useDispatch( blockEditorStore );
const getNumberOfBlocksBeforeCell = useGetNumberOfBlocksBeforeCell(
rootClientId,
gridColumnNumber
columnCount
);
const hasStartValue = () => !! columnStart || !! rowStart;
const hasSpanValue = () => !! columnSpan || !! rowSpan;
Expand Down Expand Up @@ -234,29 +233,35 @@ function GridControls( {
label={ __( 'Column span' ) }
type="number"
onChange={ ( value ) => {
// Don't allow unsetting.
const newColumnSpan =
value === '' ? 1 : parseInt( value, 10 );
onChange( {
columnStart,
rowStart,
rowSpan,
columnSpan: value,
columnSpan: newColumnSpan,
} );
} }
value={ columnSpan }
value={ columnSpan ?? 1 }
min={ 1 }
/>
<InputControl
size="__unstable-large"
label={ __( 'Row span' ) }
type="number"
onChange={ ( value ) => {
// Don't allow unsetting.
const newRowSpan =
value === '' ? 1 : parseInt( value, 10 );
onChange( {
columnStart,
rowStart,
columnSpan,
rowSpan: value,
rowSpan: newRowSpan,
} );
} }
value={ rowSpan }
value={ rowSpan ?? 1 }
min={ 1 }
/>
</HStack>
Expand All @@ -278,8 +283,11 @@ function GridControls( {
label={ __( 'Column' ) }
type="number"
onChange={ ( value ) => {
// Don't allow unsetting.
const newColumnStart =
value === '' ? 1 : parseInt( value, 10 );
onChange( {
columnStart: value,
columnStart: newColumnStart,
rowStart,
columnSpan,
rowSpan,
Expand All @@ -290,16 +298,16 @@ function GridControls( {
rootClientId,
rootClientId,
getNumberOfBlocksBeforeCell(
value,
newColumnStart,
rowStart
)
);
} }
value={ columnStart }
value={ columnStart ?? 1 }
min={ 1 }
max={
gridColumnNumber
? gridColumnNumber - ( columnSpan ?? 1 ) + 1
columnCount
? columnCount - ( columnSpan ?? 1 ) + 1
: undefined
}
/>
Expand All @@ -310,9 +318,12 @@ function GridControls( {
label={ __( 'Row' ) }
type="number"
onChange={ ( value ) => {
// Don't allow unsetting.
const newRowStart =
value === '' ? 1 : parseInt( value, 10 );
onChange( {
columnStart,
rowStart: value,
rowStart: newRowStart,
columnSpan,
rowSpan,
} );
Expand All @@ -323,17 +334,15 @@ function GridControls( {
rootClientId,
getNumberOfBlocksBeforeCell(
columnStart,
value
newRowStart
)
);
} }
value={ rowStart }
value={ rowStart ?? 1 }
min={ 1 }
max={
parentLayout?.rowCount
? parentLayout.rowCount -
( rowSpan ?? 1 ) +
1
rowCount
? rowCount - ( rowSpan ?? 1 ) + 1
: undefined
}
/>
Expand Down
30 changes: 7 additions & 23 deletions packages/block-editor/src/components/grid/grid-item-movers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,9 @@ export function GridItemMovers( {
const columnCount = parentLayout?.columnCount;
const rowCount = parentLayout?.rowCount;

const columnCountNumber = parseInt( columnCount, 10 );
const rowStartNumber = parseInt( rowStart, 10 );
const columnStartNumber = parseInt( columnStart, 10 );

const getNumberOfBlocksBeforeCell = useGetNumberOfBlocksBeforeCell(
gridClientId,
columnCountNumber
columnCount
);

return (
Expand All @@ -56,10 +52,7 @@ export function GridItemMovers( {
[ blockClientId ],
gridClientId,
gridClientId,
getNumberOfBlocksBeforeCell(
columnStartNumber,
rowStartNumber - 1
)
getNumberOfBlocksBeforeCell( columnStart, rowStart - 1 )
);
} }
/>
Expand All @@ -76,10 +69,7 @@ export function GridItemMovers( {
[ blockClientId ],
gridClientId,
gridClientId,
getNumberOfBlocksBeforeCell(
columnStartNumber,
rowStartNumber + 1
)
getNumberOfBlocksBeforeCell( columnStart, rowStart + 1 )
);
} }
/>
Expand All @@ -89,17 +79,14 @@ export function GridItemMovers( {
disabled={ columnStart <= 1 }
onClick={ () => {
onChange( {
columnStart: columnStartNumber - 1,
columnStart: columnStart - 1,
} );
__unstableMarkNextChangeAsNotPersistent();
moveBlocksToPosition(
[ blockClientId ],
gridClientId,
gridClientId,
getNumberOfBlocksBeforeCell(
columnStartNumber - 1,
rowStartNumber
)
getNumberOfBlocksBeforeCell( columnStart - 1, rowStart )
);
} }
/>
Expand All @@ -109,17 +96,14 @@ export function GridItemMovers( {
disabled={ columnCount && columnEnd >= columnCount }
onClick={ () => {
onChange( {
columnStart: columnStartNumber + 1,
columnStart: columnStart + 1,
} );
__unstableMarkNextChangeAsNotPersistent();
moveBlocksToPosition(
[ blockClientId ],
gridClientId,
gridClientId,
getNumberOfBlocksBeforeCell(
columnStartNumber + 1,
rowStartNumber
)
getNumberOfBlocksBeforeCell( columnStart + 1, rowStart )
);
} }
/>
Expand Down
28 changes: 18 additions & 10 deletions packages/block-editor/src/hooks/layout-child.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,23 @@ function useBlockPropsChildLayoutStyles( { style } ) {
const id = useInstanceId( useBlockPropsChildLayoutStyles );
const selector = `.wp-container-content-${ id }`;

// Check that the grid layout attributes are of the correct type, so that we don't accidentally
// write code that stores a string attribute instead of a number.
if ( process.env.NODE_ENV === 'development' ) {
if ( columnStart && typeof columnStart !== 'number' ) {
throw new Error( 'columnStart must be a number' );
}
if ( rowStart && typeof rowStart !== 'number' ) {
throw new Error( 'rowStart must be a number' );
}
if ( columnSpan && typeof columnSpan !== 'number' ) {
throw new Error( 'columnSpan must be a number' );
}
if ( rowSpan && typeof rowSpan !== 'number' ) {
throw new Error( 'rowSpan must be a number' );
}
}

let css = '';
if ( shouldRenderChildLayoutStyles ) {
if ( selfStretch === 'fixed' && flexSize ) {
Expand Down Expand Up @@ -81,16 +98,6 @@ function useBlockPropsChildLayoutStyles( { style } ) {
( columnSpan || columnStart ) &&
( minimumColumnWidth || ! columnCount )
) {
// Check if columnSpan and columnStart are numbers so Math.max doesn't break.
const columnSpanNumber = columnSpan ? parseInt( columnSpan ) : null;
const columnStartNumber = columnStart
? parseInt( columnStart )
: null;
const highestNumber = Math.max(
columnSpanNumber,
columnStartNumber
);

let parentColumnValue = parseFloat( minimumColumnWidth );
/**
* 12rem is the default minimumColumnWidth value.
Expand All @@ -112,6 +119,7 @@ function useBlockPropsChildLayoutStyles( { style } ) {
parentColumnUnit = 'rem';
}

const highestNumber = Math.max( columnSpan, columnStart );
const defaultGapValue = parentColumnUnit === 'px' ? 24 : 1.5;
const containerQueryValue =
highestNumber * parentColumnValue +
Expand Down
76 changes: 60 additions & 16 deletions packages/block-editor/src/layouts/grid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,23 @@ export default {
rowCount = null,
} = layout;

// Check that the grid layout attributes are of the correct type, so that we don't accidentally
// write code that stores a string attribute instead of a number.
if ( process.env.NODE_ENV === 'development' ) {
if (
minimumColumnWidth &&
typeof minimumColumnWidth !== 'string'
) {
throw new Error( 'minimumColumnWidth must be a string' );
}
if ( columnCount && typeof columnCount !== 'number' ) {
throw new Error( 'columnCount must be a number' );
}
if ( rowCount && typeof rowCount !== 'number' ) {
throw new Error( 'rowCount must be a number' );
}
}

// 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 =
Expand Down Expand Up @@ -241,7 +258,8 @@ function GridLayoutMinimumWidthControl( { layout, onChange } ) {
onChange={ ( newValue ) => {
onChange( {
...layout,
minimumColumnWidth: newValue,
minimumColumnWidth:
newValue === '' ? undefined : newValue,
} );
} }
onUnitChange={ handleUnitChange }
Expand Down Expand Up @@ -274,7 +292,15 @@ function GridLayoutColumnsAndRowsControl( {
onChange,
allowSizingOnChildren,
} ) {
const { columnCount = 3, rowCount, isManualPlacement } = layout;
// If the grid interactivity experiment is enabled, allow unsetting the column count.
const defaultColumnCount = window.__experimentalEnableGridInteractivity
? undefined
: 3;
const {
columnCount = defaultColumnCount,
rowCount,
isManualPlacement,
} = layout;

return (
<>
Expand All @@ -290,18 +316,31 @@ function GridLayoutColumnsAndRowsControl( {
<NumberControl
size="__unstable-large"
onChange={ ( value ) => {
/**
* If the input is cleared, avoid switching
* back to "Auto" by setting a value of "1".
*/
const validValue = value !== '' ? value : '1';
onChange( {
...layout,
columnCount:
window.__experimentalEnableGridInteractivity
? parseInt( value, 10 ) || null
: parseInt( validValue, 10 ),
} );
if (
window.__experimentalEnableGridInteractivity
) {
// Allow unsetting the column count when in auto mode.
const defaultNewColumnCount =
isManualPlacement ? 1 : undefined;
const newColumnCount =
value === ''
? defaultNewColumnCount
: parseInt( value, 10 );
onChange( {
...layout,
columnCount: newColumnCount,
} );
} else {
// Don't allow unsetting the column count.
const newColumnCount =
value === ''
? 1
: parseInt( value, 10 );
onChange( {
...layout,
columnCount: newColumnCount,
} );
}
} }
value={ columnCount }
min={ 0 }
Expand All @@ -320,9 +359,14 @@ function GridLayoutColumnsAndRowsControl( {
<NumberControl
size="__unstable-large"
onChange={ ( value ) => {
// Don't allow unsetting the row count.
const newRowCount =
value === ''
? 1
: parseInt( value, 10 );
onChange( {
...layout,
rowCount: parseInt( value, 10 ),
rowCount: newRowCount,
} );
} }
value={ rowCount }
Expand All @@ -331,7 +375,7 @@ function GridLayoutColumnsAndRowsControl( {
/>
) : (
<RangeControl
value={ parseInt( columnCount, 10 ) } // RangeControl can't deal with strings.
value={ columnCount ?? 0 }
onChange={ ( value ) =>
onChange( {
...layout,
Expand Down
Loading
Loading