facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
227.51k stars 46.42k forks source link

Support !important for styles? #1881

Closed syranide closed 4 years ago

syranide commented 10 years ago

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.

The-Code-Monkey commented 5 years ago

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 commented 5 years ago

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

tomhaydn commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

emildatcu commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

Tectract commented 5 years ago

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

ChristianHardy commented 5 years ago

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

GGRRRR...!!! 😏😏😏😏

necolas commented 4 years ago

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

ckhicks commented 4 years ago

Rejected or just requesting changes? 🤔

Tectract commented 4 years ago

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 commented 4 years ago

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: https://github.com/facebook/react/issues/1881#issuecomment-262257503.

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

gaearon commented 4 years ago

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!