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 classes with !important breaks $(elem).show(); #9881

Closed
Bartmax opened this issue Aug 19, 2013 · 15 comments
Closed

.hide and .show classes with !important breaks $(elem).show(); #9881

Bartmax opened this issue Aug 19, 2013 · 15 comments
Labels

Comments

@Bartmax
Copy link

Bartmax commented Aug 19, 2013

I think this is lazy implementation and it breaks when using $(elem).show() or $(elem).hide() using jQuery since the hide class get more importance since the badly (IMO) use of !important.

I think BT should not declare this so generics classes with !important, it makes difficult to customize. I don't like the idea of having another class for doing display: none because BT uses !important.

In my opinion, having a rule like : .hide { btn&, ul li&, etc.. { display:none; }} isn't much work and won't break anything. also footprint shouldn't increase a lot since the .hide pattern will be greatly gzipped.

right now, having a <div id="someId" class="hide"></div> will break $('#someId').show() function of jQuery.

@mdo
Copy link
Member

mdo commented Aug 19, 2013

We had a decently long discussion about this and opted for it for simplicity sake when quickly and easily toggling content. If you're using jQuery, just toggle the classes instead of using show/hide.

@mdo mdo closed this as completed Aug 19, 2013
@mdo
Copy link
Member

mdo commented Aug 19, 2013

In my opinion, having a rule like : .hide { btn&, ul li&, etc.. { display:none; }} isn't much work and won't break anything. also footprint shouldn't increase a lot since the .hide pattern will be greatly gzipped.

It is a lot of work—you'll need to include all the things and then you'll have a super non-performant selector that the browsers need to handle. Not an ideal solution I'm afraid.

@Bartmax
Copy link
Author

Bartmax commented Aug 19, 2013

why not make an 'internal-only' class then ? this .hide is unusable, I don't think anyone will be using class hide on their markup or will use it and will probably will have a really bad headache. Since .hide is a way too generic, would you mind using another 'internal-name' class like bootstrap-hide class or something ?

There's no mentioning of it in the docs, not even as a helper class, so I don't think you are using it for end-user (like pull-right and pull-left).

@mdo
Copy link
Member

mdo commented Aug 19, 2013

Can't rename it now, not until v4.

And it is in the docs—see helper utilities.

On Monday, August 19, 2013, Bartmax wrote:

why not make an 'internal-only' class then ? this .hide is unusable, I
don't think anyone will be using class hide on their markup or will use it
and will probably will have a really bad headache. Since .hide is a way too
generic, would you mind using another 'internal-name' class like
bootstrap-hide class or something ?

There's no mentioning of it in the docs, not even as a helper class, so I
don't think you are using it for end-user (like pull-right and pull-left).


Reply to this email directly or view it on GitHubhttps://github.com//issues/9881#issuecomment-22912446
.

@Bartmax
Copy link
Author

Bartmax commented Aug 19, 2013

Ok would be great if you consider for v4.

BTW, I don't see it on helper classes nor responsive utilities. maybe it's not published in http://getbootstrap.com/ atm

@Bartmax
Copy link
Author

Bartmax commented Sep 4, 2013

.hide and ALSO .hidden classes are defined and both classes use !important.

I really mean it, this kind of thing makes it really hard to extend bootstrap. Please remove or document hidden class. I hope you remove it.

@phaidon
Copy link

phaidon commented Sep 13, 2013

I really mean it, this kind of thing makes it really hard to extend bootstrap.

I can agree.

@japgolly
Copy link

+1

@phaidon
Copy link

phaidon commented Sep 23, 2013

I did some test. If the important flag would be removed some stuff is not working anymore with the hide class. For example the navbar with display: inline. So I think the current solution has its reasons.

@Bartmax
Copy link
Author

Bartmax commented Sep 23, 2013

yes, that 'some stuff' should add a rule like navbar.hide {display:none}.
looks simple to me but I don't know the whole implications of it.

On Mon, Sep 23, 2013 at 1:55 PM, phaidon [email protected] wrote:

I did some test. If the important flag would be removed some stuff is not
working anymore with the hide class. For example the navbar with display:
inline. So I think the current solution has its reasons.


Reply to this email directly or view it on GitHubhttps://github.com//issues/9881#issuecomment-24934853
.

@glen-84
Copy link

glen-84 commented Oct 24, 2013

👍

@dirkluijk
Copy link

+1 for removing !important

@cvrebert cvrebert added the css label May 10, 2014
@thornebrandt
Copy link

+1 for removing !important

@twbs twbs locked and limited conversation to collaborators Aug 8, 2014
@mdo
Copy link
Member

mdo commented Aug 8, 2014

It's on the v4 list to revisit.

@cvrebert
Copy link
Collaborator

X-Ref: foundation/foundation-sites#6024

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.
Labels
Projects
None yet
Development

No branches or pull requests

8 participants