Download & Extend

Add js-show CSS class for hiding elements when JavaScript is disabled

Project:Drupal core
Version:8.x-dev
Component:system.module
Category:task
Priority:minor
Assigned:Dave Reid
Status:needs review
Issue tags:CSS, JavaScript, Quick fix, Rob Loach's Wishlist

Issue Summary

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...

Comments

#1

Status:active» needs review

Patch to remove the no-js style ready for review.

AttachmentSizeStatusTest resultOperations
335185-remove-no-js-css-D7.patch1.67 KBIdleFailed: 10267 passes, 1 fail, 0 exceptionsView details | Re-test

#2

Title:Duplicate system.css classes: no-js and js-hide» Duplicate system.css classes: no-js and js-hide, add general .hide class

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.

AttachmentSizeStatusTest resultOperations
335185-remove-no-js-css-D7.patch1.92 KBIdleFailed: Failed to install HEAD.View details | Re-test

#3

Title:Duplicate system.css classes: no-js and js-hide, add general .hide class» Duplicate system.css classes: no-js and js-hide, add js-show and .hide

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;
}
AttachmentSizeStatusTest resultOperations
335185-remove-no-js-css-D7.patch2.62 KBIdleFailed: Failed to apply patch.View details | Re-test

#4

Status:needs review» needs work

The last submitted patch failed testing.

#5

Status:needs work» needs review

Re-rolled to HEAD.

AttachmentSizeStatusTest resultOperations
335185-remove-no-js-css-D7.patch2.67 KBIdleFailed: 9172 passes, 3 fails, 3 exceptionsView details | Re-test

#6

Status:needs review» reviewed & tested by the community

Works for me. Nice little cleanup. Thanks.

#7

It is not clear why we needed to introduce js-show? Care to explain?

#8

Status:reviewed & tested by the community» needs review

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.

AttachmentSizeStatusTest resultOperations
335185-remove-no-js-css-D7.patch3.03 KBIdleFailed: Failed to apply patch.View details | Re-test

#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

Issue tags:+CSS, +JavaScript

#14

Status:needs review» needs work

The last submitted patch failed testing.

#15

Status:needs work» needs review
Issue tags:+Quick fix

HEAD testing failure. Resetting.

#16

Status:needs review» needs work

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?

AttachmentSizeStatusTest resultOperations
335185-remove-no-js-css-D7_4.patch3.24 KBIdleFailed: Failed to apply patch.View details | Re-test

#18

Status:needs work» needs review

needs review...

#19

Status:needs review» reviewed & tested by the community

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

Status:reviewed & tested by the community» needs work

The last submitted patch failed testing.

#21

Status:needs work» fixed

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;
}

doesn't exist anymore in system.css

#22

Status:fixed» needs work

What about .js-show?

#23

There aren't any js elements left...

/*
** Floating header for tableheader.js
*/
table.sticky-header {
  margin-top: 0;
  background: #fff;
}

is the only js thing you can find

#24

Title:Duplicate system.css classes: no-js and js-hide, add js-show and .hide» Add js-show CSS class for hiding elements when JavaScript is disabled

system.behaviors.css:

/**
* 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;
}

This still needs a .js-show opposite. Clarifying title since part of the issue was taken care of.

#25

Ow fok srry opened wrong file (system.css), I'm so stupid!

Thnx dave... :(

#26

Can't we do something easy as like dimitri said?

html.j .js-show {
display: block
}

#27

That wouldn't work because the point would to be to hide the element if JavaScript is disabled. Also see my #11.

#28

Oh, I've been trying to get this one in for a while.

#29

Version:7.x-dev» 8.x-dev

Bumping to D8.

#30

Issue tags:+Rob Loach's Wishlist

Adding to my ever growing things of things I'd like to eventually look at.

#31

Status:needs work» needs review

There we go!

AttachmentSizeStatusTest resultOperations
jsshow.patch2.27 KBIdlePASSED: [[SimpleTest]]: [MySQL] 29,415 pass(es).View details | Re-test

#32

Looks like system.js is not included everywhere, so this should live in drupal.js instead.

AttachmentSizeStatusTest resultOperations
jsshow.patch2.23 KBIdlePASSED: [[SimpleTest]]: [MySQL] 29,420 pass(es).View details | Re-test

#33

+1

#34

The title of this issue is "Add js-show CSS class for hiding elements when JavaScript is disabled". Yet what this patch does is to allow you to :

* Use the class 'js-hide' to hide an element if JavaScript is enabled.
* Use the class 'js-show' to show an element only if JavaScript is enabled.

What about hiding an element only if JavaScript is disabled, as per the issue title? For that, wouldn't you need a no-js-hide class, something like:

html.no-js .no-js-hide {
   display: none;
}

#35

What about hiding an element only if JavaScript is disabled, as per the issue title?

+/**
+ * Show elements with the 'js-show' class. These elements will not be shown if
+ * JavaScript is disabled.
+ */
+Drupal.behaviors.jsShow = {
+  attach: function(context, settings) {
+    $('.js-show', context).show();
+  }
+};
...
+html.js .js-hide, .js-show {
   display: none;
}

@mrfelton, that's your use case right there. Things with .js-show start out with display: none;, which JavaScript switches to an display: block; style on the element itself, overriding the stylesheet. No script, no unhiding.
nobody click here