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.

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

[Box] Add breakpoint value support to maxWidth prop #26984

Merged
merged 5 commits into from
Jul 14, 2021

Conversation

ansh-saini
Copy link
Contributor

@ansh-saini ansh-saini commented Jun 27, 2021

Adds breakpoint value support on Box maxWidth prop as suggested in this comment

Closes #25771

Preview: https://deploy-preview-26984--material-ui.netlify.app/system/sizing/#max-width

<Box sx={{ maxWidth: 'md' }}>

@mui-pr-bot
Copy link

mui-pr-bot commented Jun 27, 2021

Details of bundle changes (experimental)

Generated by 🚫 dangerJS against 622f494

@oliviertassinari
Copy link
Member

@ansh-saini could we add a test case?

@oliviertassinari oliviertassinari added package: system Specific to @mui/system new feature New feature or request labels Jun 27, 2021
@ansh-saini
Copy link
Contributor Author

Sure, but I'm a little confused, there are two test files for Box and I'm not sure which file should I write the test case in...
packages\material-ui\src\Box\Box.test.js
packages\material-ui-system\src\Box\Box.test.js

@oliviertassinari
Copy link
Member

Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

@oliviertassinari
Copy link
Member

@mnajdova The sx or the https://next.material-ui.com/system/sizing/#width page? I thought we said that sx was to give an overview and the flat props to go into the details :)

@mnajdova
Copy link
Member

@mnajdova The sx or the https://next.material-ui.com/system/sizing/#width page? I thought we said that sx was to give an overview and the flat props to go into the details :)

My opinion was that every custom transformation should be on the https://github.com/mui-org/material-ui/blob/next/docs/src/pages/system/the-sx-prop (not including the pure CSS props that does not have any transformation). As the maxWidth now has custom transformation, it's useful for it to be described in both places.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 10, 2021

My opinion was that every custom transformation should be on the https://github.com/mui-org/material-ui/blob/next/docs/src/pages/system/the-sx-prop (not including the pure CSS props that do not have any transformation). As the maxWidth now has custom transformation, it's useful for it to be described in both places.

@mnajdova This would mean that we need to duplicate the information between the sx page and the detailed sections. I don't think that it can work: 1. it would make it easier to get unsynchronized information, 2. the "sx props" page is better at documenting some of the custom behaviors than in the dedication pages. This seems counterproductive. the sx prop is only about the sx prop, while the other sections cover the flattened props (which are important too).

@oliviertassinari oliviertassinari merged commit 6d10a75 into mui:next Jul 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: Box The React component. new feature New feature or request package: system Specific to @mui/system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Box] doesn't apply maxWidth prop
4 participants