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

Conditional comment for IE6 breaking IE7 + .hide #746

Closed
wants to merge 2 commits into from
Closed

Conditional comment for IE6 breaking IE7 + .hide #746

wants to merge 2 commits into from

Conversation

Gavrisimo
Copy link

Part 1:

Hello,

conditional comment for IE6 was breaking completely some page i was working on today only in IE7.

Everything i got from a page was styling on html element, nothing else, just like it parsed only html element and then stopped.

This was ran on IE7 on XP SP3(installed on Windows7 via http://www.virtualbox.org/ ) as part of http://utilu.com/IECollection/ install i have on that virtual machine.

So it's fresh install of XP with only IECollection installed(IE6, IE7 and IE8).

IE6 and IE8 worked ok, but this:

<!--[if lt IE 7]> <html class="no-js ie6 oldie" lang="en"> <![endif]-->

just wasn't letting IE7 to work. So just as a goof i tried:

<!--[if IE 6]> <html class="no-js ie6 oldie" lang="en"> <![endif]-->

After that IE7 worked like a charm.

I really have no idea why is this happening, just wanted to report this to you guys. <3

Part 2:

I really don't know how to create two separate pull requests and i hope it's not a problem if i do it like this? ;(

Anyway, whenever i am using something like .hide(), .show(), .slideToggle() in jQuery those methods are only changing display value of given element. And if i want to hide it initially i don't have any class in h5bp which i can use, so i always add .hide class which handles only display value.

For example, .hidden will apply visibility: hidden so after .show() jQuery call the element is still invisible.

I added comment for that class based on comments for other hide-a-like classes that are currently in h5bp, please update it if you think it should be any different.

@paulirish
Copy link
Member

I'm really :/ on adding .hide { display:none; } and think jquery's hide() does the same thing but easier..

the conditional comment thing is BIZARRE and im surprised this is the first time we've heard of it. but it makes me want to distrust IECollection before we change our markup.

@necolas
Copy link
Member

necolas commented Sep 9, 2011

Sounds like you're having a problem with IECollection's IE7...it's known to be a bit shaky. If you can reproduce the issue in the official or Spoon version of IE7, then please drop us a testcase to look at.

I'm with Paul about .hide. You can either use jQuery.hide() instead of the class, or removeClass()/toggleClass().

@Gavrisimo
Copy link
Author

Regarding .hide:

I am adding this class mostly because of FOUC, ie when i have some element on which i am using jQuery to show/hide it. In that case i need to do one of there:

  • add .hide { display: none } to hide the element initially(ie stop FOUC) and after that just use jQuery to alter display value on some events. jQuery will set display inline on element so it will overwrite rule in this .hide class.
  • use .hidden to hide the element initially and stop FOUC and than use .hide().removeClass('hidden') so jQuery inline selector will be > this !important you have on .hidden.
  • set display: none inline and after that let jQuery do its thing also inline

I'm cool about not adding this to h5bp, but it's just one small helper class and it seems logical to me to add this since it seems there is case for everything(for example .invisible) except something that alters only display value. :) If the size in kb is the issue here then we can try removing ; from last rule in each ruleset, that will save us bunch of bytes... =D

To wrap things up, this is something i added in my fork and something that i found it's easiest way for me to stop FOUC from happening. So i am using this and will be using this. If you guys say no for this to go into h5bp - that's cool... :)

Regardgin IE7:

@necolas if you mean spoon.net IE is not on that site for long time, or i am completely blind and can't find it. :) I'm in the process of installing Windows Vista via virtualbox, so i will test things out and post update here.

@necolas
Copy link
Member

necolas commented Sep 12, 2011

You shouldn't be hiding content before JS loads just to avoid a FOUC. It can cause accessibility problems. We don't have any helper classes that are targeting JS-enhanced presentations. Using .hidden (for reasons that do not involve JavaScript) and then removing it with jQuery after running jQuery.hide() is the way to go. Otherwise, initially hide it using .js .element CSS that relies on the presence of JS.

@Gavrisimo
Copy link
Author

@necolas That content needs to be hidden. When you load up the page the content is hidden, you click some button and $('hidden content').show() happens.

That is why i was adding that helper class in my projects, so i can just add .hide to any element that i need to be hidden, for which i have some click event on some other element that will show it.

Regarding conditional comment, i was not able to download Vista from that "microsoft students something something website" which means i didn't test this yet. I am downloading it again and hopefully this time it will work and i will update you guys asap.

But anyway, that conditional comment for IE6 is just funny to me, like ok for every IE version lower then 7 add class .ie6 to target only IE6. That means you are going to target lower versions also with .ie6 class.

<!--[if IE 6]> targets only IE6 and .ie6 works only on IE6.

Just saying... :)

@necolas
Copy link
Member

necolas commented Sep 13, 2011

Right, but just because you want something to be visually hidden in your JS-enhanced presentation doesn't necessarily mean that it should be hidden by default when JS is off, or hidden to screenreaders.

@Gavrisimo
Copy link
Author

How about .js .hide? That way it will be hidden only if js is enabled... :)

@Gavrisimo
Copy link
Author

Ok, i tested this conditional comment issue on fresh Vista install and i couldn't reproduce it. Then i tested it again on XP and i couldn't reproduce it there either...

So this issue with conditional comment must have been triggered by something else since everything is working now, both on IE7 on Vista and on IE7 on XP via IECollection.

Regarding .hide, you guys said no and that's fine.

So issue 1 - can't reproduce and issue 2 is a no no -> closing this issue.

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

Successfully merging this pull request may close these issues.

3 participants