The Select All checkbox currently has no label, which makes its styling hard. I propose adding an id generator .uniqueId() and an empty (or perhaps not empty?) label right after the checkbox, which would make styling easier.

An example of the proposal is in the attached patch.

Comments

Andrej Galuf created an issue. See original summary.

droplet’s picture

I think Core won't add an empty placeholder for styling but if it's an accessibility issue we have to sort of it.

andrewmacpherson’s picture

Status: Active » Needs review
FileSize
5.3 KB
2.72 KB

Thanks for the patch @Andrej.

There's a small problem where the patch won't apply. It includes a /web/, which I guess comes from your local dev environment.

diff --git a/web/core/misc/tableselect.js b/web/core/misc/tableselect.js
index 243e000..2bc7307 100644
--- a/web/core/misc/tableselect.js
+++ b/web/core/misc/tableselect.js

I've re-rolled it from the Drupal root directory, but with no changes to the logic.

I'll carry out the accessibility review now, this re-roll was necessary first, so that simplytest.me can apply it.

andrewmacpherson’s picture

First a baseline accessibility review, before applying the patch. When using a screen reader, the select-all checkbox is announced as "Select all rows in this table, checkbox".

The select-all checkbox in the table header differs slightly from the checkboxes in the table rows:

  • The select-all checkbox in the table header has no associated <label>, but does have a title attribute. In the absence of a label element, the accessible name is calculated using the title attribute as a fallback. See Using the HTML title attribute – Updated Dec 2012.
  • The checkboxes in the table rows have associated elements, and no title attribute.

After applying the patch, the empty label element follows the checkbox:

<input id="ui-id-1" class="form-checkbox" type="checkbox" title="Select all rows in this table">
<label for="ui-id-1"></label>

In screen reader tests, this was still being announced the same way as it was before the patch, suggesting this patch would not be an a11y regression. I'm still wary of including an empty label element for a couple of reasons, though:

  1. It's plausible that some combinations of browser and assistive tech might use the empty label in preference to the title attribute, resulting in an unlabelled checkbox. (This is speculation though. I'm not aware of any AT where this is actually a problem, but would prefer not to find out the hard way.)
  2. The empty label element may be flagged up as a problem by some accessibility testing tools, even though it's mitigated by the title attribute. Let's not risk confusing developers with a red-herring in test reports.

So, from an accessibility viewpoint things are fine the way they are, and don't need to be changed. That said, I'd be in favour of using a non-empty label element instead of a title attribute for the select-all checkbox, for consistency with the checkboxes in the table rows.

For testing, I used the tables at admin/content and admin/people with the following screen readers:

  • NVDA v.2016.2.1 with Firefox v.47 on Windows 7
  • ChromeVox with Chrome v.51 on Ubuntu 16.04
andrewmacpherson’s picture

I'm following the issue now. Please restore the "needs accessibility review" tag if the issue takes a different direction.

droplet’s picture

Status: Needs review » Needs work

Thanks @andrewmacpherson.

So now we needed an update to LABEL with title content. And the LABEL should be placed before INPUT for consistency.

.uniqueId()

This is a jQuery UI function, we should avoid to use it.

Andrej Galuf’s picture

@andresmacpherson
@droplet

Consistency be damned, here's the core problem: you can't style a checkbox with css across the board if the label is not placed behind the input field like so:

<input type="checkbox" id="foo"><label for="foo">Foo</label>

The reason for this is that the css to style it looks like this:

input[type="checkbox"]:checked + label {
 
}

since pseudoelements (:before and :after) can't target an input field, you need to hide the input and style the label with :before instead.

The problem is not limited to Select All checkbox. Classy does the exact same thing for visually hidden inputs (including checkboxes), which means you can't style them either. The difference is, as that's controlled by a twig template, themers can influence the output directly, overcoming the problem. The Select All checkbox, on the other hand, is added by javascript, meaning you have to add javascript magic to fix it. Changing templates I'm comfortable with - adding javascript to fix a styling - not so much.

droplet’s picture

@Andrej,

It's a global issue should be addressed in other issue thread (Is it table list with this ordering only?). Anyway, I'm not strongly against with it.

andrewmacpherson’s picture

I'm flexible about the order of <input> and <label> elements, as long as they are associated with for and idattributes.

I'm against having an empty <label> element though, which was originally proposed.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.