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

[docs] Overriding with classes is a bit unclear #9050

Closed
1 task done
pandaiolo opened this issue Nov 8, 2017 · 13 comments
Closed
1 task done

[docs] Overriding with classes is a bit unclear #9050

pandaiolo opened this issue Nov 8, 2017 · 13 comments
Labels
docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process. v1.x

Comments

@pandaiolo
Copy link
Contributor

pandaiolo commented Nov 8, 2017

  • I have searched the issues of this repository and believe that this is not a duplicate.

This is just a discussion over documentation of v1.

In this section : https://material-ui.com/customization/overrides/#overriding-with-classes it seems in both the text description, and the example, that the classes override replace the default one.This is not very intuitive as classes are string, not objects that can merge, which is why I thought it worked that way in the first place.

An explicitly description of how it works could make things clearer, . ie. the styles objects pointed by both the overriding class name and the default one, are merged together.

What do you think?

@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Nov 8, 2017
@oliviertassinari
Copy link
Member

I agree, it would be good to update the documentation to clarify the behavior.

@oliviertassinari
Copy link
Member

@pandaiolo Do you want to update the documentation?

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Nov 8, 2017
@oliviertassinari oliviertassinari changed the title [Doc][v1] Overriding with classes is a bit unclear [docs] Overriding with classes is a bit unclear Nov 8, 2017
@pandaiolo
Copy link
Contributor Author

I'll give it a try, probably this week end or next one

@ottosichert
Copy link

I came across this issue as well because I assumed this is possible or even the right way to do it.

@oliviertassinari Looking at your answers to similar issues I can see you recommend using styling with a higher specificity instead, i.e. .foo .bar, inline styles or even !important.

Is this the intended way to customize internal material-ui styles?
I'd rather replace the internal style completely (by copying a modified class) and accept that I have to take care of changes than use one of the above mentioned options.

The reason for this are CSS modules I am using:

  • .foo .bar will not work because I use composes: Baz in the overridden style
  • inline styles will not work because CSS modules export classNames instead of style objects
  • !important ... not going to happen

So can there be an option to modify this behaviour of @withStyles?
https://github.com/callemall/material-ui/blob/2da9988abf239cdabd730c1d0d3ab73deca3231a/src/styles/withStyles.js#L312-L314

@oliviertassinari
Copy link
Member

@ottosichert One way to increase the specificity is to change the injection order of the CSS. We have one mechanism to do so. Let us know if this fix your issue.

@ottosichert
Copy link

Thanks for the hint.

So to the answer to my question is it was (and will?) never be intended to replace internal classes. We have to stick to overriding solutions.

@pandaiolo can you add this information to the docs as well if you give it a try? :)

@oliviertassinari
Copy link
Member

I'd rather replace the internal style completely

@ottosichert I have missed this sentence, it's something we have been discussing under #6218. It's a different topic.

@peteratticusberg
Copy link
Contributor

Adding a comment here for posterity.

If you're using JSS and you want to override Material-UI styles you can, as @oliviertassinari mentioned, change the injection order to achieve this. However the latest Material UI build is not using the latest react-jss build. It's using a fork of it. So if you're using react-jss in your project the following isn't going to help:

import injectSheet from 'react-jss';
...
export default injectSheet(styles, { index: 99999 })(MyComponent)

instead you're going to need to use the material-ui HOC like so:

import { withStyles } from 'material-ui/styles'
...
export default withStyles(styles)(MyComponent)

If you use withStyles you don't need to specify the index option to increase specifity, your stylesheet should just get inserted after all of the material ui sheets and you should be good to go.

I would put up a PR to add this to the docs but from what I understand this situation is temporary. Material UI is planning on modifying and then incorporating the latest react-jss build into the project and once that happens people should just be able to look at the react-jss docs on stylesheet injection order to solve this issue.

@mbrookes
Copy link
Member

@pandaiolo Does the current wording make things any clearer?

@pandaiolo
Copy link
Contributor Author

You were faster than me... thank you :)

The component's existing classes will continue to be injected, so it is only necessary to provide the specific styles you wish to add or override.

That sentence is very clear to me and makes the underlying logic more understandable

👍

@mbrookes
Copy link
Member

Thanks.

@oliviertassinari
Copy link
Member

@mbrookes Great :) !

@nerdmax
Copy link

nerdmax commented Feb 25, 2019

Just in case some one else find this issue later.

I've created a component to simplify the material ui's css over writing process.

override-material-ui-css

You just need to wrap your whole application in this OverrideMaterialUICss component. This library is a wrapper component which only takes the children prop and renders it without any modification but just moving Material-UI's <style> tag to the top of the tag, which will make style overwriting very easy.

Eg:

import { OverrideMaterialUICss } from "override-material-ui-css"

<OverrideMaterialUICss>
  <YourComponent />
  <YourComponent />
  <YourComponent />
  ...
</OverrideMaterialUICss>

Live sample is here: Live examples

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process. v1.x
Projects
None yet
Development

No branches or pull requests

6 participants