Repository navigation
[Type] Enhancement - Adds max limit to row span and column span in grid. #64895
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.
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
Merged
Merged
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
f4b1fcf
feat: Adds max limit to row span and column span.
vipul0425 a9c8f32
fix: Adds default value for auto mode.
vipul0425 e11923e
Merge branch 'trunk' of github.com:WordPress/gutenberg into feat/issu…
vipul0425 3e86284
fix: Adds checks related to experiment mode.
vipul0425 4d4df6e
feat: Remove default value for columns intially.
vipul0425 6255f74
Merge branch 'trunk' of github.com:WordPress/gutenberg into feat/issu…
vipul0425 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,6 @@ import { | |
| __experimentalToggleGroupControlOption as ToggleGroupControlOption, | ||
| __experimentalUnitControl as UnitControl, | ||
| __experimentalInputControl as InputControl, | ||
| __experimentalHStack as HStack, | ||
| __experimentalVStack as VStack, | ||
| __experimentalToolsPanelItem as ToolsPanelItem, | ||
| __experimentalUseCustomUnits as useCustomUnits, | ||
|
|
@@ -235,53 +234,78 @@ function GridControls( { | |
| } ); | ||
| }; | ||
|
|
||
| // Calculate max column span based on current position and grid width | ||
| const maxColumnSpan = columnCount | ||
| ? columnCount - ( columnStart ?? 1 ) + 1 | ||
| : undefined; | ||
|
|
||
| // Calculate max row span based on current position and grid height | ||
| const maxRowSpan = | ||
| window.__experimentalEnableGridInteractivity && rowCount | ||
| ? rowCount - ( rowStart ?? 1 ) + 1 | ||
| : undefined; | ||
|
|
||
| return ( | ||
| <> | ||
| <HStack | ||
| <Flex | ||
| as={ ToolsPanelItem } | ||
| hasValue={ hasSpanValue } | ||
| label={ __( 'Grid span' ) } | ||
| onDeselect={ resetGridSpans } | ||
| isShownByDefault={ isShownByDefault } | ||
| panelId={ panelId } | ||
| > | ||
| <InputControl | ||
| size="__unstable-large" | ||
| label={ __( 'Column span' ) } | ||
| type="number" | ||
| onChange={ ( value ) => { | ||
| // Don't allow unsetting. | ||
| const newColumnSpan = | ||
| value === '' ? 1 : parseInt( value, 10 ); | ||
| onChange( { | ||
| columnStart, | ||
| rowStart, | ||
| rowSpan, | ||
| columnSpan: newColumnSpan, | ||
| } ); | ||
| } } | ||
| 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: newRowSpan, | ||
| } ); | ||
| } } | ||
| value={ rowSpan ?? 1 } | ||
| min={ 1 } | ||
| /> | ||
| </HStack> | ||
| <FlexItem style={ { width: '50%' } }> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why change from
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| <InputControl | ||
| size="__unstable-large" | ||
| label={ __( 'Column span' ) } | ||
| type="number" | ||
| onChange={ ( value ) => { | ||
| // Don't allow unsetting. | ||
| const newColumnSpan = | ||
| value === '' ? 1 : parseInt( value, 10 ); | ||
| const constrainedValue = maxColumnSpan | ||
| ? Math.min( newColumnSpan, maxColumnSpan ) | ||
| : newColumnSpan; | ||
|
|
||
| onChange( { | ||
| columnStart, | ||
| rowStart, | ||
| rowSpan, | ||
| columnSpan: constrainedValue, | ||
| } ); | ||
| } } | ||
| value={ columnSpan ?? 1 } | ||
| min={ 1 } | ||
| max={ maxColumnSpan } | ||
| /> | ||
| </FlexItem> | ||
| <FlexItem style={ { width: '50%' } }> | ||
| <InputControl | ||
| size="__unstable-large" | ||
| label={ __( 'Row span' ) } | ||
| type="number" | ||
| onChange={ ( value ) => { | ||
| const newRowSpan = | ||
| value === '' ? 1 : parseInt( value, 10 ); | ||
| const constrainedValue = maxRowSpan | ||
| ? Math.min( newRowSpan, maxRowSpan ) | ||
| : newRowSpan; | ||
|
|
||
| onChange( { | ||
| columnStart, | ||
| rowStart, | ||
| columnSpan, | ||
| rowSpan: constrainedValue, | ||
| } ); | ||
| } } | ||
| value={ rowSpan ?? 1 } | ||
| min={ 1 } | ||
| max={ maxRowSpan } | ||
| /> | ||
| </FlexItem> | ||
| </Flex> | ||
|
|
||
| { window.__experimentalEnableGridInteractivity && columnCount && ( | ||
| // Use Flex with an explicit width on the FlexItem instead of HStack to | ||
| // work around an issue in webkit where inputs with a max attribute are | ||
|
|
||
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.
columnCountis always going to exist here because we're setting a default value of 3 when we get it fromparentLayoutabove. The problem with that is in auto layouts with no columnCount defined, we're now unable to set a columnCount above 3 on child blocks.I think we'll have to remove that default value in order to fix this, and maybe pass it instead to
useGetNumberOfBlocksBeforeCellifcolumnCountis undefined.We should also remove
columnCountfrom the condition that allows the grid placement controls to display below (that shouldn't make a difference becausecolumnCountis always true currently).(Later we might want to consider not displaying the grid placement controls for children of auto grids, but that can be done in another PR)
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.
Since we've now removed the default
columnCount, the grid placement controls in Auto mode are no longer visible, after removing thecolumnCountcheck for their visibility. So, this change has been included in the same PR.