-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
Use @material-ui/system in the core components #15561
Comments
@dmtrKovalenko The best solution, for now, is to do: <Box textAlign={{ xs: 'left', md: 'center' }} clone>
<Typography />
</Box> The long term strategy is to find a way to have the core components host the system directly so we don't have to use an extra Box + clone. So people can do: <Typography textAlign={{ xs: 'left', md: 'center' }} /> |
You can use es6 proxies to add the props on demand or nest them 1 level and put them all on one prop, or use the generateId logic from glamor jss to use data- selectors and just spread into the element like <div {...boxprops}/> and just rerender style def on change |
+1 for this. I don't know if it is a crazy idea, but I will suggest to wrap all components with Box. On this way, we can apply the same margin and padding properties for example on a button instead of having to wrap it with a |
I would prefer to wrap the components with I think is a good idea to separate the use |
@yordis Yes, it's the idea. There is a related opportunity we can explore. We could have a component that generates thousands of global class names, a là tailwind, that we could apply directly as class names. It would extend the system support to all elements on the page. <button className="p-3" /> |
I think that tailwind is targeting different methodologies on how we design and develop applications in my opinion, the more you rely on something being there (global classes) the harder it is to move code since you trade-off cohesion. I love the idea, but I would push it into the |
@oliviertassinari is this going to be added for v5? Or is there a canary version where that's already available? Changing the font-weight of Right now, there's no way to have responsive font sizes for typography with different font weights than what's specified in the theme. This is a very noticeable inconsistency if you're already using |
This comment has been minimized.
This comment has been minimized.
@oliviertassinari I see mentioned above applying system to the core components, but I am also curious about applying breakpoint-esque functionality to non-system props. It would be amazing to be able to have something like For example, AppBar <AppBar elevation={{xs: 1, md: 2, lg: 4}}>...</AppBar> Is that something that would be considered? |
@matthewwolfe Regarding the elevation, I think that we can consider it as part of the system, and is already in https://material-ui.com/system/shadows/#example. We will need to remove duplication. However, if we are pushing in the I wonder if we shouldn't have a:
What do you think about it? |
|
Why do we need this for all components? Just because it might make sense for one component doesn't mean it makes sense for all of them. As far as I can tell it'll be a footgun for widgets since it leads down a blind alley: you start styling with I would understand if this would be replaced at compile time with the styling engine but it doesn't make sense to add another runtime API that does the same thing but different. This means that either API is insufficient and overcoming it by adding both just adds technical debt we have to pay long term. |
@eps1lon we can target child components with CSS selectors in the sx prop, but it's probably to be avoided. I think that it's best to only style the element it's applied on.
I don't understand, what duplication are you referring to? @mnajdova is planning to add a section in the documentation about when using the sx vs styled API. We should probably explain the pros and cons too, the why.
For the same reason it's useful on a Box. Why does Bootstrap has CSS utils? I think that the best case is when we can use it on every DOM node mounted. It's where it get its true power. I think that there is nothing simpler, easier to understand, and faster that to declaratively set the style on the element.
Are you saying sx and styled are solving the same problems? We definitely need to document the differences, why this is important to have both and the best use cases for each. Emotion has the two, styled-components has the two too. We might be able to close, we could argue that the issue was solved with #23205. We are also exploring how to extend the support to all primitives in #23220. |
So we don't even want people to use
Duplication of styles in
The Box and Slider are by defition not the same component. It does not follow that if X makes sense for A that X makes also sense for B. Otherwise you're arguing that it makes sense to have an exhaust on your smartphone because it makes sense to have it on your car.
This needs to be explored first though. We can't just implement things and then look at the API we created.
Both APIs are used on primitives first and not composite components. It also still doesn't follow that if A uses X that B should use X. You're arguing that we should use Flash because there are some sites that are still using flash. We go by requirements not existence. |
@eps1lon Agree, I believe we have tried to cover it in the pull requests/issues/blog post, but we can improve here. For the "why?", there are 4 written content listed in https://trello.com/c/IUve1J6o/1577-system-core-component that are on the topic. I think that the timing is perfect to cover the documentation side of things in more detail. We have had a question on the same subject from a template author on the store recently. Maybe we can first get to a plan of action here and then convert it to the documentation, then close this issue? cc @mnajdova |
I'm closing for #24405. Each migrated component gets the feature. |
Expected Behavior 🤔
It will be super useful to have per-breakpoint
align
prop. E.g.alignSm
,alignLg
-- them will add an ability to make responsive designs with no tears 😢Current Behavior 😯
It is required to manually change text align for
Typography
Something like that:
Examples 🌈
Smth similar is already implemented in vuetify -> https://vuetifyjs.com/ru/framework/alignment
The text was updated successfully, but these errors were encountered: