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

.hide and .show without !important rule #9237

Closed
danielevigi opened this issue Aug 8, 2013 · 39 comments
Closed

.hide and .show without !important rule #9237

danielevigi opened this issue Aug 8, 2013 · 39 comments

Comments

@danielevigi
Copy link

I think a version of .hide and .show without !important rule might be useful.

In my case they create problems on working with jQuery show() and hide() functions.

@JustinDrake
Copy link

Could you provide a fiddle showing such a problem?

@danielevigi
Copy link
Author

Here is the fiddle: http://jsfiddle.net/kQ47C/

Same problem with css() and animate() jQuery functions.

@maxymczech
Copy link

I have just encountered a small related issue with .hidden-* and .visible-* classes - they define display rule for block and table elements, however, I needed to show/hide inline elements. I came up with some custom @media rules for the template I am currently working on, however, maybe .hidden-inline-* and .visible-inline-* would be of some use to the community.

@cvrebert
Copy link
Collaborator

cvrebert commented Aug 8, 2013

@dantistus The docs explicitly mention that those classes use display: block. See also #8869.

@maxymczech
Copy link

@cvrebert Thank you for your answer. Yeap, that's exactly what I was saying, I know it's block :) I also said it might be useful to add other similar class for inline (or inline-block) elements. That would be logical.

@mdo
Copy link
Member

mdo commented Aug 10, 2013

We'll leave the !important in for now.

As for additional classes, add a new issue or PR to continue those talks elsewhere.

@mdo mdo closed this as completed Aug 10, 2013
@snedled
Copy link

snedled commented Aug 16, 2013

As an alternative to .hide one can use the attribute hidden. This works great with bootstrap and jQuery show() in all browsers from IE6

@pherrymason
Copy link

I've had to remove the !important rule as it was avoiding all my jquery code to make visible some components.
I'm curious what problems would this cause @mdo

@jakebolam
Copy link

Likewise causing problems for me with jQuery. This also affects jQuery animations.

@lemoinem
Copy link

lemoinem commented Sep 6, 2013

👍
It's very inconvenient not to be able to use jQuery's .hide() and .show() method with these classes anymore...

That was the bootstrap 2 behavior and no reason has been provided as to why the !important has been introduced...

@phaidon
Copy link

phaidon commented Sep 13, 2013

The current behavior is very bad.

+1 for removing the important rag.

@kwisatz
Copy link

kwisatz commented Oct 31, 2013

Not sure why the !important flag is set, but I do also have trouble with jQuery's hide() and show() functions.

@carasmo
Copy link

carasmo commented Oct 31, 2013

.pull-right {
  float: right !important;
}
.pull-left {
  float: left !important;
}
.hide {
  display: none !important;
}
.show {
  display: block !important;
}

Why are any of these !important ? Since they are outside the media queries, they are important already.

@cvrebert
Copy link
Collaborator

@carasmo To avoid specificity conflicts with other styles. We're thinking about changing it in v4, when we can break backward-compatibility again.

@frediana
Copy link

I also believe the !important flag should be removed. We can't use jQuery .show and .hide anymore :(

@mrchess
Copy link

mrchess commented Nov 16, 2013

+1 ... basically nerfs all jquery animations.

Going around it for now like this:

$('.some-class').removeClass('hide').hide().fadeIn()

@cvrebert
Copy link
Collaborator

It is interesting to note that html5-boilerplate's .hidden also uses !important.

@cvrebert
Copy link
Collaborator

The h5bp folks seem to recommend not mixing jQuery-based and CSS-class-based showing/hiding in the first place: h5bp/html5-boilerplate#746

@jtheoof
Copy link

jtheoof commented Nov 26, 2013

I too believe that those !important flags should be removed.

$('#some-element').removeClass('hide'); /* to show an element */

is just counter intuitive.

@clutchwave
Copy link

"Me too", here.

So what is the best way of toggling an hidden-sm div (so that I can display it) without changing my bootstrap CSS file??

I'd also like to have the user be able to re-hide it. Looks like it will be some jQuery hackery

@carasmo
Copy link

carasmo commented Dec 12, 2013

http://jsbin.com/IxifuQEz/1/edit?html,js,output

  $(document).ready(function() {

     $('.toggleme').click(function () {
        $('.jumbotron').toggleClass('hidden-sm');
        return false;
    });
 }); //end

it's hidden-sm NOT sm-hidden.

@dmlyapin
Copy link

Workaround:

var show = $.fn.show;
$.fn.show = function() {
  this.removeClass('hidden');
  show.call(this);
};

@brainTrain
Copy link

adding important to hide and show breaks backward compatibility of this framework. (and it's bad practice to being with)

Not cool guys, not cool.

@digggggggggg
Copy link

I am keen to adopt bootstrap but this kind of thing is the reason why I've avoided it for so long. despite what this albeit brilliantly put article may say http://j.mp/1sESpoT , using !important is as all above suggested:

  1. counter intuitive - breaks $().show();, $().removeClass('hidden') is madness, even with an override
  2. inconsiderate to future developers and thus, counter productive to the frameworks expansion
  3. it's a trap that will cover up bad css... how many developers have put conflicting rules in place and are completely oblivious to the mess that will create once !important is removed?

I've made my own work-around for now and will continue to work with Bootstrap, but I will proceed with a great deal of caution and very wary as to what else and how many more of these quirks I will find.

@er1c
Copy link

er1c commented Apr 11, 2014

Since .hide was deprecated in favor of .hidden, I wonder if its time to allow .hide to be set to not !important - especially since you can't easily override .hide { } with the !important in it - people that need to put the reverse compatability back in could easily go back in and add a custom less file into their bootstrap.less at the end, - right now I'm forced to edit the utilities.less directly.

@cvrebert
Copy link
Collaborator

@er1c That's not how SemVer-compliant backward-compatibility works. We can't make those sorts of changes until v4.

@ryanmcnz
Copy link

@cvrebert
This isn't just about backwards compatibility, it's about bad design choices and essentially breaking some very simple and essential functionality in JavaScript. Functionality as simple as show/hide should NOT be impacted by a CSS library that has "!important" splashed everywhere in its global classes.

This sort of thing is making the task of moving something from Bootstrap v2 to v3 VERY difficult.
I was wondering why all of a sudden so many very simple show/hide calls weren't working in JavaScript.
This is very uncool.

There is no need for it on .hide, .pull-left, .pull-right, etc.
This makes me think that absolutely no precautions were taken for making decisions about the appropriate usage of !important.

I can understand it being approached in things such as media queries for printing, but it serves no purpose on global things such as .hide.

Correct me if I'm wrong on this, but is it essentially the scenario of "fix something that's breaking a large amount of everything else, or just leave it in favor of being compliant with versioning"?

To be honest it's very worrying if this usage of "!important" could possibly remain in Bootstrap until v4.

@er1c
Copy link

er1c commented Apr 16, 2014

@ryanmcnz Dude, I think we're in the same boat :) Just suggesting a possible short-term fix that doesn't require me to have my own custom bootstrap distribution.

@ryanmcnz
Copy link

@er1c
Woops! I had quoted the wrong person. Same boat indeed. Fixed :)

@dmlyapin
Nice fix, I ran into a problem with it missing a return on that last line where the call is made.
Here's what I've used:

var show = $.fn.show;
$.fn.show = function() {
    this.removeClass('hidden');
    this.removeClass('hide');
    return show.call(this);
};

@DaSchTour
Copy link
Contributor

I think that this doesn't need to be fixed.
Using jquery .show() .hide() together with hidden class is a mix that doesn't have to work.
jQuery's show and hide functions work on inline-style base and only work because this might override the given class. But that's really ugly and it's simple to avoid in jQuery. Just work with addClass and removeClass. So what's the problem? If you'd like to use show and hide just add inline style instead of using hidden class!

@ryanmcnz
Copy link

@DaSchTour
This is all good and true if you're starting off with Bootstrap v3.
Yes, from that perspective one would realize that obviously .hide would hide something no matter what the inline styling is..but what about those who are moving from Bootstrap v2 to v3?

After all that's where the change is most noticed.
This is where the backwards compatibility was heavily lost.

It's a terrible thing to debug when something is being overridden with !important.
Now imagine that effecting many places all throughout a large volume of code and spanning across multiple files.

@er1c
Copy link

er1c commented Apr 16, 2014

@DaSchTour @ryanmcnz

This is exactly the case I found myself in...migrating from v2.3.2 to 3.1.1 and all of my existing jquery code (which bootstrap does depend on - so it's not like it's a random library "non standard" library) was suddenly broken. The good news for me was .hide did get deprecated in 3.0.1 - so glad I procrastinated my upgrading to bootstrap3 :) I hate having to edit the utilities.less file directly, but it ends up being a great solution to just remove the !important from the .hide style. Everything else that uses the official bootstrap3 .hidden still operates fine.

@DaSchTour
Copy link
Contributor

@ryanmcnz @er1c
well as I see you relied on this bad practice overriding a class with inline style and know you want the library to change for backward compatibility for this bad practice? Just do it correctly from start on and you won't have any issues.
So this is not an issue is not of bootstrap but an issue of wrong usage of jquery from start on.

And the problem with a large volume of code in multiple files can me easily solved with a good IDE with search replace over multiple files. Did it myself with Eclipse and it worked great.

When using jQuery, CSS and Bootstrap V2 and V3 correctly than this is not an issue!

@ryanmcnz
Copy link

@DaSchTour
I never said that at all. Thanks.

To keep on topic, If this isn't an issue with Bootstrap, then please explain to me why ".hide" needs !important on it, and why usage of !important on a global class is suddenly not a bad practice anymore.

The point I'm trying to get across here is still this:
It's a terrible thing to debug when something is being overridden with !important.

@DaSchTour
Copy link
Contributor

@ryanmcnz
When you tell me that it's terrible to debug something with !important, then I you something totally wrong in debugging. When you need a class to hide items with a class and show them with jquery show than why don't you just create an own class to do this. I don't see the point why somebody's poor development should lead to a change in this library.

To the point why .hide should have !important: For example to have the possibility to hide an element by adding the class hide through jQuery for an element, that has a class with display:block that might override this. It takes care of the fact, that the class really get's applied.

@cvrebert
Copy link
Collaborator

X-Ref: foundation/foundation-sites#939

@dirkluijk
Copy link

+1 for removing !important to support jQuery.hide(). Or adding an additional class, for BC reasons.

@jeremyhaile
Copy link

I can't believe that bootstrap uses an !important flag on the hidden class. That prevents styles from overriding the hidden, and breaks jQuery.show(). Really disappointed in this and think it's poor CSS practice.

Please remove the !important flag from these attributes.

@twbs twbs locked and limited conversation to collaborators Jun 9, 2014
@cvrebert
Copy link
Collaborator

X-Ref: #9881

zhao-l-li pushed a commit to zhao-l-li/bootstrap-x-editable-rails that referenced this issue Jun 26, 2015
zhao-l-li pushed a commit to zhao-l-li/bootstrap-x-editable-rails that referenced this issue Jun 26, 2015
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