-
-
Notifications
You must be signed in to change notification settings - Fork 32.8k
[grid] Remove 'column' and 'column-reverse' options from GridDirection type #47564
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
base: master
Are you sure you want to change the base?
Conversation
Netlify deploy previewhttps://deploy-preview-47564--material-ui.netlify.app/ Bundle size report
|
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.
Pull request overview
This PR removes 'column' and 'column-reverse' options from the GridDirection type definition, limiting it to only 'row' and 'row-reverse' values.
- Changes the exported GridDirection type to exclude column-based flex-direction values
- Represents a breaking change to the Grid component's TypeScript API
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type ResponsiveStyleValue<T> = T | Array<T | null> | { [key in Breakpoint]?: T | null }; | ||
|
|
||
| export type GridDirection = 'row' | 'row-reverse' | 'column' | 'column-reverse'; | ||
| export type GridDirection = 'row' | 'row-reverse'; |
Copilot
AI
Jan 6, 2026
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.
This is a breaking API change that removes valid CSS flex-direction values ('column' and 'column-reverse') from the GridDirection type. This will cause TypeScript compilation errors for any existing code using these direction values. If this is intentional, it should be:
- Documented as a breaking change
- Include a migration guide explaining why these values are being removed and what alternatives users should use
- Consider deprecating these values first in a minor release before removing them in a major version
If there's a technical reason why column directions shouldn't be supported in Grid, that rationale should be documented.
| export type GridDirection = 'row' | 'row-reverse'; | |
| export type GridDirection = 'row' | 'row-reverse' | 'column' | 'column-reverse'; |
| type ResponsiveStyleValue<T> = T | Array<T | null> | { [key in Breakpoint]?: T | null }; | ||
|
|
||
| export type GridDirection = 'row' | 'row-reverse' | 'column' | 'column-reverse'; | ||
| export type GridDirection = 'row' | 'row-reverse'; |
Copilot
AI
Jan 6, 2026
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.
The GridDirection type is being updated to remove 'column' and 'column-reverse' values, but this change is incomplete. Several other parts of the codebase still reference these values and need to be updated:
- PropTypes validation (lines 181-182 in this file) still accepts 'column' and 'column-reverse'
- The DIRECTIONS constant in gridClasses.ts (line 18) still includes all four values
- The underlying @mui/system/Grid/GridProps.ts still defines GridDirection with all four values
- PigmentGrid also has the full set of direction values
This incomplete change will cause inconsistencies between TypeScript type checking and runtime behavior, and may break applications that use column directions. Consider either completing all related changes together or reverting this change until a comprehensive update can be made.
| export type GridDirection = 'row' | 'row-reverse'; | |
| export type GridDirection = 'row' | 'row-reverse' | 'column' | 'column-reverse'; |
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.
Removed 👍
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.
ZeeshanTamboli
left a comment
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's fixed can you check now, Since argos didn't reported any errors i thought demos were functional |
ZeeshanTamboli
left a comment
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.
I think we need to update the docs here https://deploy-preview-47564--material-ui.netlify.app/material-ui/api/grid/#grid-prop-direction.
It says flex-direction, but now it doesn't include column and column-reverse. We can update it to:
/**
* Defines the `flex-direction` style property for the container.
*
* ⚠️ Only `row` and `row-reverse` are supported. `column` and `column-reverse` are not supported,
* because the Grid component is designed to subdivide layouts into **columns**, not rows.
*
* For vertical layouts, use `Stack` (optionally inside a Grid item).
*
* @default 'row'
*/
|
@sai6855 If it looks good, you can merge this. |
Since this has critical changes, i requested additional review from @siriwatknp to avoid regressions |
siriwatknp
left a comment
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.
please check my comments
| <Grid container sx={{ justifyContent: 'flex-end' }} size={6}> | ||
| <Stack sx={{ alignItems: 'end' }}> | ||
| <Grid> | ||
| <Tooltip title="Add" placement="right-start"> | ||
| <Button variant="plain">right-start</Button> | ||
| </Tooltip> | ||
| </Grid> | ||
| <Grid> | ||
| <Tooltip title="Add" placement="right"> | ||
| <Button variant="plain">right</Button> | ||
| </Tooltip> | ||
| </Grid> | ||
| <Grid> | ||
| <Tooltip title="Add" placement="right-end"> | ||
| <Button variant="plain">right-end</Button> | ||
| </Tooltip> | ||
| </Grid> | ||
| </Stack> |
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.
This is strange. It looks misuse to me, I think better to change entirely to Stack or Box with sx prop.
| - PaperComponent={CustomPaperComponent} | ||
| - PopperComponent={CustomPopperComponent} | ||
| - ListboxComponent={CustomListboxComponent} | ||
| + slots={{ | ||
| + paper: CustomPaperComponent, | ||
| + popper: CustomPopperComponent, | ||
| + }} | ||
| + slotProps={{ | ||
| + listbox: { | ||
| + component: CustomListboxComponent, | ||
| + }, | ||
| + }} |
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.
this file is not related to the PR.
| }, | ||
| "direction": { | ||
| "description": "Defines the <code>flex-direction</code> style property. It is applied for all screen sizes." | ||
| "description": "Defines the <code>flex-direction</code> style property for the container.<br>⚠️ Only <code>row</code> and <code>row-reverse</code> are supported. <code>column</code> and <code>column-reverse</code> are not supported, because the Grid component is designed to subdivide layouts into <strong>columns</strong>, not rows.<br>For vertical layouts, use <code>Stack</code> (optionally inside a Grid item)." |
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.
| "description": "Defines the <code>flex-direction</code> style property for the container.<br>⚠️ Only <code>row</code> and <code>row-reverse</code> are supported. <code>column</code> and <code>column-reverse</code> are not supported, because the Grid component is designed to subdivide layouts into <strong>columns</strong>, not rows.<br>For vertical layouts, use <code>Stack</code> (optionally inside a Grid item)." | |
| "description": "Defines the <code>flex-direction</code> style property for the container.<br>⚠️ Only <code>row</code> and <code>row-reverse</code> are supported. <code>column</code> and <code>column-reverse</code> are not supported, because the Grid component is designed to subdivide layouts into <strong>columns</strong>, not rows.<br>For vertical layouts, use <code>Stack</code> instead." |
I think we should not tell users to mix the Grid and Stack together. They serve a different purpose for creating layout.
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 to be clear, I meant using Stack as a child of Grid container is misuse but this demo is fine:
https://mui.com/material-ui/react-grid/#column-direction
…nto grid-props
…nto grid-props
@siriwatknp applied suggestion as per your review, can you re-check? |
closes #47563
According to https://mui.com/material-ui/react-grid/#column-direction,
directionprop doesn't supportcolumnandcolumn-reversebut api-docs page listed them as valid . This PR fixes discrepancypreview: https://deploy-preview-47564--material-ui.netlify.app/material-ui/api/grid/