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

Support !important for styles? #1881

Closed
syranide opened this issue Jul 19, 2014 · 73 comments
Closed

Support !important for styles? #1881

syranide opened this issue Jul 19, 2014 · 73 comments

Comments

@syranide
Copy link
Contributor

We currently don't support !important as it has to be set using style.setProperty(name, value, priority). This should be trivially easy to implement if it's something want to support, although I'm not sure about the performance implications (EDIT: #1886), although I can't imagine it would be measurable as we would be doing the string test ourselves and those are cheap.

http://www.w3.org/TR/DOM-Level-2-Style/css.html#CSS-CSSStyleDeclaration

OK... apparently IE8 doesn't support this at all, it might still be worth implementing, for the future and for those who don't care about IE8. After further experimentation, apparently it is! style.setAttribute(name, value) (priority is part of the value). But apparently that was wrong as well, the only way to set it seems to be with cssText, an acceptable work-around may be to just use cssText when we detect an !important style, which should be seldom enough that performance is a non-issue.

An important consideration is that we already support this for the initial render, but subsequent updates will fail as !important isn't recognized for properties.

@AdamKyle
Copy link

!important is horribly abused in css, and if people write their css properly their is never a need for !important. That said I don't think it should be included, because then people can use "bad practices" in their apps and styles. Personally inline styling with react is bad enough.

@vjeux
Copy link
Contributor

vjeux commented Aug 8, 2014

On a random note, I thought that inline styles override !important but I was wrong ( http://jsfiddle.net/do0hpcvm/ ).

@zpao
Copy link
Member

zpao commented Aug 11, 2014

@vjeux I'm going to leave this to you. I think we should probably just maintain status quo here and not support !important, but there may be valid uses somewhere.

@syranide
Copy link
Contributor Author

@zpao We do support it though, we just don't support updating a style that has it.

@zpao
Copy link
Member

zpao commented Aug 11, 2014

That's not support, but I see what you mean (that's like saying we "support" hyphenated styles)

@nelix
Copy link

nelix commented Dec 14, 2014

Any word on the progress of this? I'm using react in a widget and a browser extension that work on third party sites and not being able to update !important styles is an issue for me

@waldreiter
Copy link
Contributor

@nelix Maybe you can use something like this as a workaround:

var sheet = document.createElement('style');
document.body.appendChild(sheet);
sheet.innerHTML = 'html {font-size: 1em !important;}';

@syranide
Copy link
Contributor Author

@cody It would probably be easier to just manually manage the style property of the node itself ;)

@ccnokes
Copy link

ccnokes commented Jul 5, 2015

+1 to Nelix's comment. For example, in Bootstrap CSS there are a number of utility classes that all use !important (hidden, pull-right, etc) that I wouldn't be able to override without !important inline styles. (I totally agree that !important is abused--and probably Bootstrap too, for that matter--but I think React should still be able to work with them.)

@thomassuckow
Copy link

We use !important to set a dynamic background color on an element and have it still show up when printing. Without !important the background is not printed.

EDIT: Since I only needed ie9+, I used setProperty in componentDidMount & componentDidUpdate

@quantizor
Copy link
Contributor

IMO, it should be supported. It's not React's place to be dogmatic about how people author their styles.

@zpao
Copy link
Member

zpao commented Aug 4, 2015

I had a longer post here and then lost it, oh well…

The summary is that it would be expensive to check every value for !important, so supporting this isn't awesome. Even if we do get a good option, it's unlikely we'd do this while we support IE8 because I don't want to go down that cssText path. This isn't really at all about dogma, I would say that it's more about perf. And the fact that we're faced with a number of limitations when working with styles in CSS.

What we could perhaps do is change the data structure for style.

style = {
  color: 'red',
  backgroundColor: {
    value: 'blue',
    important: 'true'
  }
}

It would be backwards compatible (if the value isn't an object, we do the exact same thing). And it would allow us to start using the setProperty API with important. Downside is obviously more objects. But we don't have to do any string parsing.

@quantizor
Copy link
Contributor

@zpao That seems reasonable. Perf is obviously a very important factor to consider and I appreciate the creative thinking... your solution would at least allow a developer to support the need, even if it's a little more effort.

@Charca
Copy link
Contributor

Charca commented Feb 19, 2016

I'm facing the same issue as @nelix. It is quite an edge case, but it would be nice to have support for this.

@nelix
Copy link

nelix commented Feb 26, 2016

@zpao that api suites me fine, I will be really happy to rip out dynamically generated css + manual style manipulation I have from my chrome extension's IFrame armor.

@mwschall
Copy link

Ran into this today, trying to dynamically set the background color of a Semantic UI Icon. They have those colors set as !important, presumably for legitimate reasons. Fortunately I could work around it like @thomassuckow did. Would be nice to avoid the extra step though.

@6ewis
Copy link

6ewis commented Apr 15, 2016

what's the status on this?

@zpao
Copy link
Member

zpao commented Apr 15, 2016

No change. As we invest more in JS styles I think we'll end up with some changes to how styles work. In the mean time I don't think we'll make any changes without a fuller picture of how that will play out.

@japgolly
Copy link

This makes it pretty hard for React to play nice with Semantic UI (which frustratingly uses !important nearly everywhere).

@harmony7
Copy link

For the record, since 15.x it no longer works even on initial render (since moving away from innerHTML). All rules with "!important" are now being thrown out.

@syranide
Copy link
Contributor Author

syranide commented Jul 22, 2016

For the record, since 15.x it no longer works even on initial render (since moving away from innerHTML). All rules with "!important" are now being thrown out.

For non-IE8 (which we actually don't actively care about anymore) this can technically be solved. EDIT: If that is something we want to look into.

@yanickrochon
Copy link

yanickrochon commented Aug 16, 2016

Over two years ago, and this is still debated? To me restricting it's use because of some subjective argument is just regressive. If it's mainly to prevent people to abuse it, well, it is quite idiotic as it's use is... important on many use cases. If it is part of the specification, then why not supporting it in React?

However, for balance, an "!important" declaration (the delimiter token "!" and keyword "important" follow the declaration) takes precedence over a normal declaration. Both author and user style sheets may contain "!important" declarations, and user "!important" rules override author "!important" rules. This CSS feature improves accessibility of documents by giving users with special requirements (large fonts, color combinations, etc.) control over presentation.

@toddself
Copy link

toddself commented Oct 4, 2016

I literally just lost an hour debugging this. Given the amount of assistance react provides users when they do something unexpected (suggesting camel-case when using hyphenated class names, etc), i would have expected something in the console in this.

It would be awesome to be non-dogmatic and support people doing something unnecessary or at least warning about it.

@Venryx
Copy link

Venryx commented Oct 12, 2016

I'm having this issue as well.

The reason I need the " !important" override on an inline style is that my project is in a transition state, and still has CSS being applied using classes--one of which is using a css " !important" tag. (originally to override some JQueryUI css, I believe)

Having support for the tag would prevent me from having to use another css class (I'm trying to transition away from css classes), or modifying the old css class to no longer have the !important tag--which is a pain since it's used globally and would break other areas of the UI. (which are still using the old, JQuery rendering)

@brigand
Copy link
Contributor

brigand commented Oct 13, 2016

For the moment, I wrote a library that should address this use case: brigand/react-with-important-style.

It uses this.element.style.setProperty to set the style with the important modifier.

@The-Code-Monkey
Copy link

The-Code-Monkey commented Nov 9, 2018

this is something that should really be implemented even if we don't use the ! and just do say

style={{ width: '50% important' }} or even style={{ width: '50%', widthImportant: true }}

I would be comfortable with either being implemented.

edit: or even just let me do style="width: 50% !important"

@lubber-de
Copy link

We managed to remove lots of !important from the SUI components in V2.7.x recently 🙂
https://fomantic-ui.com/introduction/new.html

@tomhaydn
Copy link

tomhaydn commented Jan 17, 2019

We managed to remove lots of !important from the SUI components in V2.7.x recently 🙂
https://fomantic-ui.com/introduction/new.html

I was running into this issue because I was using 2.6, so lucky I saw this. Drop jQuery next 🙂

@okbasa
Copy link

okbasa commented Jan 29, 2019

So still no easy solution to this problem? I'm unable to change a Semantic UI element's color dynamically without access to !important. My workaround was to create my own class that emulated what Semantic UI was doing for a certain element..but this is obviously far from ideal.

@ckhicks
Copy link

ckhicks commented Jan 29, 2019

Yeah, extending something just to fix one property doesn't seem like a scalable solution, given that you will have to adjust it to match the lib on every update. Uncool.

@lCavazzani
Copy link

Same thing @champeleon , I need to change some Semantic UI properties and really needed this !important. It should be supported for such a big library as React...

@Tectract
Copy link

I suggest the react team should consider implementing a !super-important tag.

@emildatcu
Copy link

This is stupid. I need !important in @media print, to remove the body from the page and setting the visibility of the dialog to be visible !important and overflow visible, when printing the contents of a dialog with 'multiple pages' within, like this ->
image

So yeah, it should be included and supported. I know it's NOT OK to use !important, but sometimes it's necessary.

@milesj
Copy link
Contributor

milesj commented Feb 21, 2019

What if the styles supported a tuple (an array)?

style={{ width: ['50%', 'important'] }} to support other possible flags?
style={{ width: ['50%', true] }} to only support important?

@ckhicks
Copy link

ckhicks commented Feb 21, 2019

Love that idea, @milesj. It gets us a lot closer to having the support when needed, but making it stand out beyond the normal practice of writing CSS, so hopefully it would be flagged by casual PR reviews as bad practice. 😆

@behnood-eghbali
Copy link

behnood-eghbali commented Apr 9, 2019

It's not actually a react problem. is it?! I see the other frameworks had the same problem with !important:
vue: v-show should be !important
angular: NgStyle and style can not set !important

@steodor
Copy link

steodor commented Apr 11, 2019

@behnood-eghbali just because other frameworks exhibit the same behavior doesn't make it a non-issue. It just means that they have the same problem as well.

@luke-pritch
Copy link

luke-pritch commented Jul 10, 2019

I can't believe that it has been 5 years and nothing has been done to address this at all. Working with frameworks that already have pre-defined !important styling and going through a roundabout way to fix them is very frustrating.

@anarchist912
Copy link

anarchist912 commented Jul 31, 2019

I recommend using styled components, if you have a good reason to make use of "!important":

Here is an example where we overwrite Semantic-UI's padding on grid columns. You simply can't without important.

const StyledColumn = styled.div(({size}) => ({className: `${size} wide column`})`
    &&&&& {
        padding-top: 0.3rem !important;
        padding-bottom: 0.3rem !important;
    }
`
<StyledColumn size="three"></StyledColumn>

&&&&&& <- bumps up specifity

@pilniczek
Copy link

I recommend using styled components, if you have a good reason to make use of "!important":

Here is an example where we overwrite Semantic-UI's padding on grid columns. You simply can't without important.

const StyledColumn = styled.div(({size}) => ({className: `${size} wide column`})`
    &&&&& {
        padding-top: 0.3rem !important;
        padding-bottom: 0.3rem !important;
    }
`
<StyledColumn size="three"></StyledColumn>

&&&&&& <- bumps up specifity

&&&&& seems to be very similar to this category of selectors. https://speakerdeck.com/csswizardry/refactoring-css-without-losing-your-mind?slide=64

@ChristianHardy
Copy link

ChristianHardy commented Sep 3, 2019

Meeehhh!!! React y sus cosas baratas!!! No acepta !important...

@Tectract
Copy link

Tectract commented Sep 3, 2019

¡Ay, caramba! Tendrás que usar clases y una hoja de estilo vinculada en lugar de estilos en línea, mi hombre.

@ChristianHardy
Copy link

¡Ay, caramba! Tendrás que usar clases y una hoja de estilo vinculada en lugar de estilos en línea, mi hombre.

GGRRRR...!!! 😏😏😏😏

@necolas
Copy link
Contributor

necolas commented Oct 15, 2019

I submitted a PR for this which was rejected #12181, so I'm going to close this as "wontfix"

@necolas necolas closed this as completed Oct 15, 2019
@ckhicks
Copy link

ckhicks commented Oct 15, 2019

Rejected or just requesting changes? 🤔

@Tectract
Copy link

It appears it was rejected because they didn't like how the patch was implemented, from comments like these:

I don't think we're opposed to supporting this feature per se. It's just that it's not clear a string-based API is the best one here.

The part that feels wrong to me is that everybody has to pay for an indexOf lookup that nobody uses today (because the feature doesn't exist) and won't be commonly used in the future.

I don't have time to dig into the code, really... But it feels like there is a high demand for this feature so maybe it should be left re-opened, just a thought.

@gaearon
Copy link
Collaborator

gaearon commented Oct 16, 2019

I think a lot of commenters are probably missing the fact that setting styles as important is already possible with a single line of code in React: #1881 (comment).

I’m going to lock this thread to prevent further misinformation — it really does seem like most people don’t realize it, and adding more replies causes this information to get lost.

If you have a particular proposal for a more ergonomic alternative, please send an RFC here: https://github.com/reactjs/rfcs

@facebook facebook locked as resolved and limited conversation to collaborators Oct 16, 2019
@gaearon
Copy link
Collaborator

gaearon commented Oct 16, 2019

To be fair, the ref solution doesn’t help for server rendering. Again, if you have a particular API proposal, sending an RFC would be a better place to discuss it. Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests