Duplicate system.css classes: no-js and js-hide, add js-show and .hide
Dave Reid - November 17, 2008 - 02:07
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | system.module |
| Category: | task |
| Priority: | minor |
| Assigned: | Dave Reid |
| Status: | needs work |
| Issue tags: | CSS, JavaScript, Quick fix |
Description
Right now we currently have two different CSS styles in modules/system/system.css to hide elements if JavaScript is enabled: no-js and js-hide:
html.js .no-js {
display: none;
}
/*
** For anything you want to hide on page load when JS is enabled, so
** that you can use the JS to control visibility and avoid flicker.
*/
html.js .js-hide {
display: none;
}I think the .js-hide is a better description and easier to understand, so I propose that we remove the .no-js style. It's also too bad we don't have a way to do a 'js-show' class...
#1
Patch to remove the no-js style ready for review.
#2
There's also no good way just to use display: none on something I want hidden. Quick addition to #1 adds the class .hide in addition to .js-hide.
#3
And a final revision that adds a 'js-show' that will hide items if JavaScript is disabled. Too bad CSS3 isn't widely adopted enough to just use:
html:not(.js) .js-show {display: none;
}
#4
The last submitted patch failed testing.
#5
Re-rolled to HEAD.
#6
Works for me. Nice little cleanup. Thanks.
#7
It is not clear why we needed to introduce js-show? Care to explain?
#8
It's provided to show an element if JavaScript is enabled, as an opposite to js-hide, which hides elements if JavaScript is enabled. I've improved the documentation for it.
#9
For example, the progress bar in upload or the teaser checkbox thing could be emitted with js-show class so that they are only visible if user has js enabled.
#10
Why not do this for js-show
.js-hide { display: none; }html.js .js-show { display: block; }
#11
I'm not sure how that would work with an element that had something like:
<div class="js-show" style="display: inline;">...</div>#12
#13
#14
The last submitted patch failed testing.
#15
HEAD testing failure. Resetting.
#16
1.) Patch doesn't apply to system.js anymore, so a reroll is required.
Additional comments:
2.) The CSS spec tells browsers to ignore selectors it doesn't understand, if I remember correctly. Therefore, even if CSS3 is not widely supported, it may be not be a bad idea to add the html:not(.js) (properly documented, of course) to remove the "flicker" the comment mentions, at least for browsers that do support CSS3.
3.)
<div class="js-show" style="display: inline;">...</div>Shouldn't direct style attributes be avoided in favor of classes anyway?
#17
here is a fast rerole.
not sure about the inline stuff/css3, but what about creating a inline class?
#18
needs review...
#19
Those duplicate asterisks in the comment block are pretty odd. They were already in there though, so let's leave them for another issue to fix.
I'm sceptical as ever about relying on the html.js class to do anything serious (how can you trust the server to accurately tell the client whether the client is executing Javascript at the moment? What if you switch it off or on while browsing?). But as it seems to be done throughout core, we might as well do it here too.
#20
The last submitted patch failed testing.