-
-
Notifications
You must be signed in to change notification settings - Fork 32k
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 back system props #24485
[Box] Add back system props #24485
Conversation
@material-ui/core: parsed: +0.08% , gzip: +0.10% |
Edited: Benchmarks result before merge - https://dev.azure.com/mui-org/Material-UI/_build/results?buildId=22509&view=logs&j=9ef21fd1-5d60-5fa4-f8b2-6dc79e173863&t=516d74c5-15a9-50a0-24d9-dba41a487a2d |
After the PR is merged, I'll add more benchmarks for the system props too. |
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.
Will we use the same approach to implement the props on the Typography or Grid component?
That's the plan, that's why I've added the |
packages/material-ui-system/src/styleFunctionSx/extendSxProp.js
Outdated
Show resolved
Hide resolved
packages/material-ui-system/src/styleFunctionSx/extendSxProp.js
Outdated
Show resolved
Hide resolved
packages/material-ui-system/src/styleFunctionSx/extendSxProp.js
Outdated
Show resolved
Hide resolved
* @ignore - do not document. | ||
*/ | ||
const Box = React.forwardRef(function Box(props, ref) { | ||
const BoxInner = React.forwardRef((props, ref) => { |
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 assume we will be able to remove this inner component later down the road.
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.
Hopefully yes. I had it inlined first, but I needed to add propTypes on it, that’s why I’ve extracted it..
Co-authored-by: Olivier Tassinari <[email protected]>
Co-authored-by: Olivier Tassinari <[email protected]>
This PR adds back the system props in the Box component. However ,the implementation is a bit different than the previous one. For it to work, I am just extending the value of the sx prop, by adding all system props at the start of the
sx
object. This means that the value of the sx prop will override these props consistently, which is what we want I believe.TODO: