Since the change to inline styles how am I supposed to:
All the above would be possible (albeit hacky) if every component produced elements with classes set (using !important
in my CSS). At the moment it creates elements with inline styles but no class, so they cant be targeted and overridden in CSS.
Since the change, this project is extremely difficult to work with if you are building a responsive site with any degree of customisation on top of the stock components. I have since given up on it and moved to something else as it is a constant battle with this library to display elements how you want them to. Beforehand, we could just use CSS as everyone has for years (decades?).
+1 Inline style implementation breaks how most devs want to build sites.
At the moment, I'm having to do things like this:
.new-booking > div > div:first-child > div:first-child > div:first-child > div:first-of-type {
overflow: visible !important;
}
@adamscybot for media queries and pseudo-classes you can use Radium (http://projects.formidablelabs.com/radium -> check out the API Docs). For example, with Radium you can have things like a media query @media (min-width: 992px)
or a pseudo-classes :hover
right in your inline style - which is super useful.
Pseudo-elements are nothing more than JavaScript variables and math -> check out this cheatsheet https://youtu.be/ERB1TJBn32c?t=17m49s
For "overriding the styles of inner components" I guess you're referring to CSS descendant
and child
selectors. For this I don't have an answer...unless an external class written in old-fashion CSS is considered a good answer. Maybe someone could clarify what is the appropriate way of achieving this with inline styles.
Cc @chantastic, @jquense
why am I being pinged here?
@jquense maybe you can help us a bit. Here https://github.com/react-bootstrap/react-bootstrap/issues/362 you mention that descendant and sibling selectors are hard with inline styling. Also the React Widgets as well as React Bootstrap libraries heavily relies on external CSS classes rather than inline styling. I was wondering if you could share some code with how hard it is to achieve descendant or child selectors with inline styling, given the fact that media queries, pseudo-classes and pseudo-elements can be in a way or another elegantly solved.
Regarding selecting inner elements -- old fashioned CSS would be an OK solution to me. However, as mentioned, its very difficult to reliably do this without lots of:
.new-booking > div > div:first-child > div:first-child > div:first-child > div:first-of-type {
overflow: visible !important;
}
This works but is obviously a bad idea as any updates in material UI that change the DOM layout will break these selectors. This would be fixed if every component in material-ui was given a classname, including inner ones. Is this something that would be accepted in a pull request?
It's a real shame that this project did its own implementation of CSS for components. In general, I disagree with the idea completely, but if this is inevitably going to be done an off the shelf solution should of been picked such as radium which is more feature-full. Though I don't know how mature those solutions were when the decision was made.
@adamscybot agree on the fact that relying on those kind of selectors is fragile because they're tightly coupled to the HTML structure and also not opened for further customization because of !important
.
However, I don't see why Material UI won't accept pull requests for including customization points (CSS classNames or inline styling parameters or whatever) for every inner component. jQuery UI has customization points (CSS classes) for all its inner components. Take a look at the jQuery UI Selectmenu widget below:
It would be naive to think that a component library like Material UI is flexible enough to fit everyone's needs by only providing top level styling customization points and ignore the customization points for inner components.
Including @hai-cea in the loop for helping with this problem
Sometimes, even if you want to keep the components the same and just want to change the styles of the root component -- you can't do things you'd expect to be able to. For example, if you change the width and height of FloatingActionButton, it doesn't alter the width and height on the inner components meaning the ripple and hover affects look wrong. This is actually because some components have bad CSS (using hardcoded magic numbers instead of percentages). However, it's very irritating to have to submit an issue/pull request every time this happens rather than just fixing it ad hoc at the time by using inner selectors.
I've been pinged on various networks to come weigh in on this.
Breakpoint, hover, and pseudos can be pretty tricky in inline-style systems. As mentioned above Radium is a really nice abstraction that feels very Sass-like.
I wouldn't recommend going it alone, it's a tricky problem.
I can't really speak to the rest. I am not an active part of the material-ui community. It would be disrespectful voice an opinion on any of the other matters raised here.
I saw that you're using that slide as a pseudo-class cheatsheet. I have this codified in a repo, if that provides an help: https://github.com/chantastic/pseudo-class
Cheers!
Chan
The way we deal with this on React Native which uses inline styles is by providing "customization points" as @cosminnicula mentioned. Using the same example, it would look like this:
<SelectMenu menuStyle={{paddingRight: -5}} menuItemStyle={{margin: 10}} />
and to implement it
var SelectMenu = React.createClass({
render: function() {
return (
<div>
<ul style={[styles.menu, this.props.menuStyle]}>
{this.props.items.map(item => <li style={[styles.item, this.props.itemStyle]} />)}
</ul>
</div>
);
}
});
@hai-cea, @oliviertassinari any thoughts on this?
Those is are good questions!
Override the styles of inner components
The CSS approach we had before was adding an extra layer of API (className). And was used by people to customized component. IMO it wasn't great since it was making our implementation detail a global API by default and likely to break.
With the inline style approach, you have to use specific style properties for inner components.
If you need a property that doesn't exist, we will happy to accept pull request. (as proposed @vjeux with customization points)
Add media queries so I can change the styles per breakpoint of material ui components.
Add pseudo elements.
Add hover effects.
That's the most annoying part after our migration to the inline style approach.
A better long-term solution would be to replace our custom solution with something like Look _(or Radium once this issue is resolved)_.
However, it's very irritating to have to submit an issue/pull request every time this happens rather than just fixing it ad hoc at the time by using inner selectors.
@adamscybot You are probably not the only one how faces this type of issue. It would be great contributions :+1:.
Radium has resolved that issue and released version 0.15. :yum:
This looks promising https://github.com/rofrischmann/react-look, although I haven't dug into the details.
@mbrookes I was thinking about using Radium. That's more bulletproof.
Oh yes, I see you Look'ed (groan) at it already.
Radium is using onMouseEnter
/ onMouseLeave
events and state for tracking state for :hover.
It is slower than css :hover.
On complex page with many elements, slower machine and quick mouse movement it can even miss some events. So your element stays 'hovered' even after mouse leave it.
For hover, any inline styles solution is worse than css :hover.
With more development in an application that uses material-ui, I am starting to think that inline styles are a fool's errand in a library. I greatly enjoy CSS Modules in an _application_, but they're not usable out-of-the-box in a library. At this point in time, a library should simply use namespaced classes.
With proper css class names, developing with material-ui would be a breeze. Sure, it would be possible (and likely) that you'd include more CSS than you need, bloating your build. But Radium is a 50kb minified (114k unminified) library. This means that importing even the smallest component would add 50kb+ to your build. With CSS, at least you have the ability to cut dependencies you don't need, or offer 'material-ui/styles/menus/menu-item.css'
-style imports for Webpack users.
Regardless, web developers are already set up to handle dependencies with css. If we namespaced all the classes it would work fine. Yeah, it would pollute the global namespace.
Is it ideal? No. But I don't think the ecosystem is there yet to handle modular CSS. <style scoped>
will help a lot in the future, allowing us to choose predictable classNames for easy extension (without needing JS to lookup the generated names) while avoiding global pollution. But it's just not there yet.
As mentioned in many comments and threads, including this one and #684, inline styles just have too many compromises.
@STRML have you looked at https://github.com/gajus/react-css-modules? it is pretty much css modules for webpack and browserify. Their for you can have same size optimizations on build - since you can reference only what you need. One thing I do not like for radium is that in the end it is still pure inline styles that are horrible to read in dev tools
@dlebedynskyi Yes, issue is that as a library, you can't rely on all consumers using webpack/browserify. So CSS Modules is more or less out of the question - there's not really a good solution to this that I'm aware of, for all packaging possibilities.
@STRML I would assume that webpack/browserify will actually make a bundle for you. And I would expect css to be bundled as well there. And at this point you could create whatever bundle you want - as a whole lib or as umd module per each component, if we are talking about lib deliverable.
As for case where this package is actually used in development of something bigger then hello world I would expect browserify/webpack/system-js builder anyway.
Of course it can be handy to have something like https://github.com/ludohenin/gulp-inline-ng2-template for this purpose.
IMHO, using anything that in the end will be producing inline styles is very bad idea. It will still be unreadable and hard to customize/override.
It will still be unreadable
I would say it's more an issue with your debugger tools than with the approach itself.
and hard to customize/override.
I'm wondering if it would help to append a property in development so people know which dom element a style
property is affecting. In the same way, a className is a public API that helps to identify dom element with the dev tools.
I would say it's more an issue with your debugger tools than with the approach itself.
If a library does not work well with every inspector in every browser, is it a problem with the inspector, or the library?
I'm wondering if it would help to append a property in development so people know which dom element a style property is affecting. In the same way, a className is a public API that helps to identify dom element with the dev tools.
That would be nice, but it shouldn't be just in development, because users _will_ write CSS selectors to target those elements and will be surprised to see them break in production. There should simply be namespaced classNames on these elements.
I was doubling down in inline-styles for a while, but the main issue with inline-styles is that you can't do media-queries in combination with server-side rendering. Radium is really smart about it, by injecting a real style tag. Still I'm not sure if this is a ideal solution. To me className based systems like CSS Modules or JSS feel like a cleaner solution since they still allow media queries & pseudo elements out of the box.
I like @vjeux's suggestion regarding "customization points". We did that in Belle, but we are considering to move to a single theme
prop that contains all the styles. This significantly reduces the amount of props and you can prepare the theme in your code before without polluting your JSX. Could be nice for MaterialUI as well?!?
In addition I wonder if merging or overwriting styles is a better solution when providing data to a "customization point".
@nikgraf That's my one concern about inline styling for production -- scalable inline styling with support for media queries requires significant a engineering investment just to keep performance reasonable.
@STRML having namespaced classnames also aids in writing E2E/acceptance tests where react/vue/angular/whatever is irrelevant -- you just want to be able to easily select the critical elements which is hard when you essentially have to duck type your selectors with the absence of classes or other identifiers.
It certainly feels like a massive compromise just to eliminate a css
dependency, especially when so many toolchains are using less/sass. I'm
not saying there's a better solution than less/sass or simply delivering
a full css file (or a common/component structure), but it's certainly
better than this.
Nathan wrote:
@nikgraf https://github.com/nikgraf That's my one concern about
inline styling for production -- scalable inline styling with support
for media queries requires significant a engineering investment just
to keep performance reasonable.@STRML https://github.com/STRML having namespaced classnames also
aids in writing E2E/acceptance tests where react/vue/angular/whatever
is irrelevant -- you just want to be able to easily select the
critical elements which is hard when you essentially have to duck type
your selectors with the absence of classes or other identifiers.—
Reply to this email directly or view it on GitHub
https://github.com/callemall/material-ui/issues/1951#issuecomment-189888702.
@nikgraf:
we are considering to move to a single theme prop that contains all the styles. This significantly reduces the amount of props and you can prepare the theme in your code before without polluting your JSX. Could be nice for MaterialUI as well?!?
We discussed it here #3130, but there were performance concerns.
@mbrookes I agree that a level of nesting means a bit more effort in shouldComponent update. We should be able to create helper function which checks the theme and then it shouldn't be more than accessing a prop one level deeper and to the same amount of props checking. Unfortunately I don't have numbers on the level of impact. Anyone tested this so far?
Re classNames: on the React Conf I asked in the Q&A what they think the future of styling will be. ClassName based or inline styles? Sebastian Markbåge mentioned he is opinionated about it and thinks it will be inline-styles.
To give some context: the React team spoke a bit about how React in the future might be able to take care of layout on the Web like with React Native on iOS/Android. Right now the API's are missing in the browsers, but maybe sometime in the future this might be possible and a styling system in JS might be the way to style components.
I put together a quick test, to show the reduced API, but didn't optimise in shouldComponent
, or test for performance impact. Not sure I have the code around any more though, such as it was.
@oliviertassinari:
IMHO it would be far better to adopt a solution, where we write the style in JS. Hence, I would just drop CSS Modules.
Could you elaborate on why you think inline JS styles is a better approach than CSS Modules? Obviously CSS in React is all over the place these days, and I'm trying to get a sense of what I'd like to use in my projects going forward.
@ffxsam You get me wrong, I don't think that inline JS styles is a better approach than CSS Modules. But I do think that writing the style in JS is more valuable.
We have already seen this pattern in the past with HTML and React.
IMHO the main advantage of React over any other framework, like Angular is that React in bringing the HTML to the JS and not the other way around.
I think that doing the same with the CSS is as valuable. For instance, I really like the styling approach used by react native.
However, the implementation isn't easy. He are intensively using the inline style approach here. We are using it enough to see that this approach isn't perfect and has big issues.
But, inline style isn't the only answer to the styles in JS. The implementation could be using a combination of CSS classes and inline style. Have a look at react-look. I'm really interested in this lib.
Regarding the future, as far as I know, Facebook see React as a platform and not a web library. They are really pushing forward to unify the web and the native environment, for instance they are considering using css-layout for react-dom. That could be another answer to how to implement styles in JS.
Watching this topic with great interest. Hopefully the MUI folks decide on a solution, we love this approach and the library and are actively thinking about ways to help improve it.
Closing this in favor of https://github.com/callemall/material-ui/issues/4066
@STRML I fully agree that using namespaced css modules would be so much better than in-line styles; so many problems could have been avoided had that been chosen. I've used BEM with React with great success and I would love to see Material-UI maintainers rethink this issue and opt to rewrite the library using BEM or some other suitable namespaced approach. I rather like the UI that this lib provides but after cutting a branch of an app that I am currently developing with it I have decided to abandon the library due to its use of inline styles.
Yeah. This whole library has been hamstrung by this unconventional approach to styles. It's unfortunate but I'm hopeful a solution will emerge in #4066.
Any thoughts on implementing the approach taken by react-toolbox
@devarsh Could you elaborate on what you mean with approach. I'm not familiar with it. Thanks.
~@oliviertassinari is there no way to set ~::before
/::after
pseudo elements? Sorry for asking here but I can't seem to find a _related_ issue 😬
EDIT: Damn, I feel stupid, I was doing content: ''
, as opposed to content: '""'
🤦♂️
For anyone trying to use ::before selector, checkout @breadadams comment. Thats the right thing to do, and really easy it's really easy to miss.
Most helpful comment
~@oliviertassinari is there no way to set~::before
/::after
pseudo elements? Sorry for asking here but I can't seem to find a _related_ issue 😬EDIT: Damn, I feel stupid, I was doing
content: ''
, as opposed tocontent: '""'
🤦♂️https://github.com/mui-org/material-ui/blob/b7ce7e5fa57c215bf759e5e31d1436adf212b5a2/packages/material-ui/src/Input/Input.js#L34-L35