-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
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
[Reboot] New component #9661
[Reboot] New component #9661
Conversation
c7cae4b
to
1220db0
Compare
Yes! |
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.
Great idea
Nice idea, not sure about the name though... Perhaps |
|
Once it's released, we can update the examples. |
@oliviertassinari Actually, if this were a standalone project "Reboot" is a bit more catchy - it was just that most things in MD (so by extention in MUI) are named more literally, but that's cool. |
docs/src/pages/style/reboot.md
Outdated
|
||
### Typography | ||
|
||
- We enable the font antialiasing for better display of the Roboto font. |
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.
'Font antialiasing is enabled'
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.
Great idea!
}, | ||
'*, *::before, *::after': { | ||
boxSizing: 'inherit', | ||
}, |
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'm curious, is there a reason to prefer this approach over simply
*, *::before, *::after {
box-sizing: border-box;
}
?
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.
@pelotom This way, people can change the inherited value anywhere and propagate it down the tree. @nathanmarks Introduced this pattern. As far as I remember, Google Map was all broken with box-sizing: border-box;
. Bootstrap approach (the one you are suggesting) was making it harder than necessary to go back to box-sizing: content-box;
.
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.
What do you think?
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.
Interesting, I didn't know about that, but it makes sense.
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.
However, this approach is making it harder to cherry-pick a different box-sizing on a single element.
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.
So it's about cherry-picking vs, omg, it doesn't play well with this third party library.
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 have no idea what's best. Such a hard tradeoff 🤔 .
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 will benchmark. copying is often the best thing to do. I do it everyday.
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.
You could always add a prop which controls which approach you want to use.
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.
@pelotom It seems it's Bootstrap vs everybody else.
- Us
- https://developer.microsoft.com/en-us/fabric#/components/checkbox
- http://blueprintjs.com/docs/#core/typography
- https://semantic-ui.com/usage/theming.html
I will keep it this way. In the future, we can add a property if people ask for it with a solid use case :).
1220db0
to
72ac363
Compare
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 I imagine a common pattern will be
<MuiThemeProvider theme={theme}>
<Reboot />
<MyApp />
</MuiThemeProvider>
or similar, would it make sense to provide a convenience boolean prop on MuiThemeProvider
to accomplish the same, i.e.
<MuiThemeProvider theme={theme} reboot>
<MyApp />
</MuiThemeProvider>
?
@pelotom Adding the |
Good points 👍 |
Closes #9641
cc @mui-org/core-contributors