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

v-show should be !important #3761

Closed
codysherman opened this issue Sep 25, 2016 · 24 comments
Closed

v-show should be !important #3761

codysherman opened this issue Sep 25, 2016 · 24 comments

Comments

@codysherman
Copy link

codysherman commented Sep 25, 2016

Vue.js version

2.0.0-rc.7

Reproduction Link

http://codepen.io/codysherman/pen/qaArqJ?editors=1010

Steps to reproduce

Have an element already have an !important display trait, such as one of Bootstrap's 'visible-*' classes.

What is Expected?

Vue should hide the element based on the v-show.

What is actually happening?

Because v-show doesn't have an !important tag, it is overruled. In my opinion, in the event of a conflict, the tie should obviously go to Vue, as it is getting its logic dynamically. Angular's ng-show applies the !important tag for this reason.

@sylvainpolletvillard
Copy link

Please no. The !important level is already used in some components to purposely override Vue v-show behaviour. The current level as inline style is the most appropriate.

Just put the condition on your visible- class instead of using v-show. It makes no sense to have a hidden element with some visible- classes anyway.

@posva
Copy link
Member

posva commented Sep 25, 2016

As @sylvainpolletvillard said, if you want to apply !important you should create a class for it.

@posva posva closed this as completed Sep 25, 2016
@BrainBacon
Copy link

@posva v-hide does not apply a class to the element when hidden. If it did this would be a non-issue. Wouldn't a class have to be applied via v-bind? Seems like a contrived way to go about hiding an element and would make v-hide completely useless when doing responsive design in a popular css framework.

@posva
Copy link
Member

posva commented Sep 26, 2016

v-hide doesn't exist but you can create your own using the v-show directive as a reference. You can even publish it to npm a post it here 😉
Yes, the class have to be applied with a v-bind

@BrainBacon
Copy link

@posva Sorry I meant v-show, sounds like I had a bit of a Freudian slip haha. That being said and expanding on what @sylvainpolletvillard said, is component overriding directives really something that should be widely supported? Seems to me something like that would be an edge-case that should be either handled by Vue's internal logic or some kind of decorator whereas something as common as responsive framework usage should be supported? I assume there's also a good reason why the directive doesn't apply a class for easy selecting, but I don't know enough to say what that is? Perhaps you can shed a bit of light on the reasoning?

@posva
Copy link
Member

posva commented Sep 26, 2016

By applying classes I mean something like this:

<div :class="{hide: hide}"></div>
.hide {
  display: none !important;
}

Maybe a modifier to v-show can come in handy v-show.important=""

@sylvainpolletvillard
Copy link

sylvainpolletvillard commented Sep 26, 2016

It would still be !important vs !important. !important is the nuclear bomb of CSS specificity: there is nothing above it. Betting on a double !important conflict for the application layout is a terrible idea IMO.

I do not know the reasons why Bootstrap visible- classes use !important in the first place, but by making this decision, it implies that Bootstrap claims the priority on the element visibility state. Vue should not interfere in this priority war, otherwise we would have the opposite problem where a user wants to override the styles in CSS but can't because of Vue.

@BrainBacon
Copy link

BrainBacon commented Sep 26, 2016

@posva Yeah I understood what that meant. Like I said before I thought that seemed like a contrived way to do it and would defeat the purpose of having v-show for those that use responsive classes.

I'm remarking on the philosophy of the design choice. If the design is to allow components to override the show directive I argue that is a poor design choice if support for popular CSS frameworks is intended. This issue will return many times in the future from folks using responsive classes since it will simply look like v-show is not working.

I agree with @codysherman in that Vue should take precedence over something like a responsive class since the model should represent a more accurate picture of display intent.

@BrainBacon
Copy link

@sylvainpolletvillard correct me if I'm wrong, but wouldn't the !important modifier bound directly to the element like how v-show is currently implemented take precedence regardless? If we were talking about a class that was applied to the element then I think you would be right, but that's not the way Vue is implemented.

@LinusBorg
Copy link
Member

LinusBorg commented Sep 26, 2016

I do not know the reasons why Bootstrap visible- classes use !important in the first place, but by making this decision, it implies that Bootstrap claims the priority on the element visibility state. Vue should not interfere in this priority war

I can agree to that, but:

otherwise we would have the opposite problem where a user wants to override the styles in CSS but can't because of Vue.

Statically overriding a dynamic decision of Vue with important in the CSS seems to be a strange idea was well.

I agree with @codysherman in that Vue should take precedence over something like a responsive class since the model should represent a more accurate picture of display intent.

I can agree to that but think we don't want to introduce this as a default behaviour as it would be a breaking change.

So we are left with @psova's suggestion of a modifier like v-show.important or telling people to build their own directive / use a class binding:

:class{'visible-xs': someCondition, hide: !someCondition}

@posva
Copy link
Member

posva commented Sep 26, 2016

@sylvainpolletvillard correct me if I'm wrong, but wouldn't the !important modifier bound directly to the element like how v-show is currently implemented take precedence regardless? If we were talking about a class that was applied to the element then I think you would be right, but that's not the way Vue is implemented.

@BrainBacon Yes it will, because inline style has the highest specificity

@BrainBacon
Copy link

BrainBacon commented Sep 26, 2016

I can agree to that but think we don't want to introduce this as a default behaviour as it would be a breaking change.

[email protected] perhaps? Having a mid-term solution like v-show.important would work as long as the documentation is highlighting this exact use case. Maybe a name like v-show.responsive would be more appropriate?

I think this behavior should be default in the future if we're talking about breaking changes.

@sylvainpolletvillard
Copy link

sylvainpolletvillard commented Sep 26, 2016

Statically overriding a dynamic decision of Vue with important in the CSS seems to be a strange idea was well.

I personnally use this for @media print queries. I have some layout handled with v-show that I want to be always visible or hidden when printed. My usecase is a form composed of multiple steps, one visible at a time on computer, but they all need to be visible when printed. With this change, it won't be possible to do this anymore since you can't override inline !important with media queries. I am sure there are other valid use cases where overriding in CSS is necessary.

Something like <div :class="{ 'hidden-lg': message === 'true' }"> looks like the right thing to do here.

@BrainBacon
Copy link

BrainBacon commented Sep 26, 2016

Something like <div :class="{ 'hidden-lg': message === 'true' }"> looks like the right thing to do here

Seems like a lot of added complexity for something that is very common. You're depending on every designer to think about the interaction between the responsive logic and the model logic.

@yyx990803
Copy link
Member

I would say this is not Vue's problem, but Bootstrap's problem. It's Bootstrap using !important that forces everyone into this specificity war. If I were you I'd not use static CSS that has !important unless for very specific reasons like printing. And I don't think this is a common case.

@codysherman
Copy link
Author

Here is a pure CSS solution that might help anyone caught in this dilemma out:

*[style*="display: none"] { display: none !important }

@gerardreches
Copy link

gerardreches commented May 25, 2017

@yyx990803 It could be a Bootstrap's problem, but at the same time Bootstrap also has reasons to use !important on display properties.

Vue shouldn't change the default behaviour, and Bootstrap can't remove !important from its utilities. But I still like @posva suggestion of adding an option like v-show.important for those who want to use it.


Another option could be adding a new global configuration property for this:

# conditionalRenderingPriority

  • Type: boolean

  • Default: false

  • Usage:

Vue.config.conditionalRenderingPriority = true

Adds !important to inline display: none; styles added by v-show.

@gijo-varghese
Copy link

@codysherman that was cool

@codysherman
Copy link
Author

Thank you very much @gijo-varghese, I'm very glad that I was able to help out!

@benmiller86
Copy link

benmiller86 commented Feb 28, 2019

I know this is old, but I feel that I should add that it is not only Bootstrap's visability classes that interfer with v-show, but all of their display classes! I think it is much less of an edge case for me to have an element with the d-flex class that I want to show/hide with v-show, and having something like v-show.important as suggested above would be ideal for cases where such a conflict arises.

My current solution is to use class bindings to add Bootstrap's display class with the same condition as v-show so it is removed when hiding the element:

<div v-show="someCondition" :class="{ 'd-flex': someCondition }">
    <!--content-->
</div>

@ramwin
Copy link

ramwin commented Jun 17, 2019

I will use a template to include the element with Bootstrap's 'visible-*' classes or "d-flex" class.

@ramwin
Copy link

ramwin commented Jun 17, 2019

or you can put the "visible-*" class in v-bind:class like this:

<div :class="{'d-flex': show}" v-show="show">       
  <div class="border border-danger small-box"></div>
  <div class="border border-danger small-box"></div>
  <div class="border border-danger small-box"></div>
  <div class="border border-danger small-box"></div>
</div>                                              
<button @click="toggle_show()">toggle_show</button>    

Here is an example.

@ttodua
Copy link

ttodua commented Nov 26, 2021

I still think that between design and logic part, the logic should be given the priority. Bootstrap's imporant is meant for design, and vue's v-show is logical part, and it should take superiority. When we say v-show="myCondition that's all - if myCondition is false, element should be hidden. period. We can blame this or that, but it's obvious. But till date, not addressed, the fact that thousands of people use Bootstrap, is ignored. And moreover the valid suggestions by @gerardreches , not implemented till date.

@caryuval
Copy link

caryuval commented Dec 19, 2022

I also ran into this.

I had a component that I wanted to use v-show with, and he had a d-flex coming from bootstrap with !important.

I ended up just moving the v-show to the parent tag without the important.

This didn't work:
<Alert v-show="shouldShowAlert" />

I moved it to a parent tag and it worked:
<div v-show="shouldShowAlert">
<Alert />
</div>

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

No branches or pull requests