Add a '#check_all' option for checkboxes element

Hello,

Forced to rename '#checkall' to '#check_all' due to spell-checking test.
For now code, is only a proof of concept. It miss some documentation, if the implementation is what you think of, i'll doing it.

Fixes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies. Also, 10.2 is in security mode now.

@MaxPah thank you very much! Naming this check_all is totally fine!

What's needed now:

1. Write tests to ensure it works as expected

To be discussed:

1. Should we hide the button until JS is ready, so it's not shown without JS? Is there a core policy for that? Or a helper class maybe?
2. Should we already copy and align the module functionality for the Drupal Field API UI? Or leave this in contrib for now?

PS: @MaxPah would you mind implementing the functionality and test also here in contrib:
✨ Extend core's Checkboxes Element Active

It will need to be changed a bit, but I think it would be helpful to already have this in contrib, until it reaches core?

LRWebks → changed the visibility of the branch 11.x to hidden.

LRWebks → changed the visibility of the branch 3459246-add-a-selectall to hidden.

I have implemented some fixes, as the current behavior did not work in any form, as well as the addition of general tests for the Checkboxes field, since they didn't exist before. This should be fine now!

Made PHPCS happy. This looks really good, thank you @LRWebks!

Regarding

Create follow-up to add a setting to checkboxes field widget to enable / disable this functionality e.g. for entity reference checkboxes, lists, etc.

I think it makes sense to also implement this Field API addition here. If needed, we could sort it out later, but it's based on this issue so implementing it elsewhere will make things a lot harder.

So I think you should go for it here.

Some tests are failing, e.g.

There was 1 error:
    
    1) Drupal\Tests\views_ui\Functional\DisplayFeedTest::testFeedUI
    Error: Call to a member function hasAttribute() on null
    
    /builds/issue/drupal-3459246/core/modules/views_ui/tests/src/Functional/DisplayFeedTest.php:68
    /builds/issue/drupal-3459246/core/modules/views_ui/tests/src/Functional/DisplayFeedTest.php:40
    
    ERRORS!
    Tests: 1, Assertions: 16, Errors: 1.

Furthermore, I can't see a #check_all option being handled anywhere?! Please re-read the issue summary and comments.

MR!8678 by @MaxPah had the required lines of code btw.!

Anybody → changed the visibility of the branch 3459246-add-a-selectall to active.

Anybody → changed the visibility of the branch 3459246-add-a-selectall to hidden.

Anybody → changed the visibility of the branch 11.x to active.

@Anybody @LRWebks thanks for your efforts, this would be a great addition.

The "Check all" and "Uncheck all" buttons working as expected for the "#checboxes" element.

While interacting with it I realized a few things, that are most related to user experience:

1. "Wouldn't it be more convenient if we had a single button that could check and uncheck all the checkboxes? The button label would change based on the state: 'Check all' when none are selected, and 'Uncheck all' when all are selected.
2. Would it make sense to add a property, something like #hide_check_all => TRUE, to hide these buttons when needed, with the default set to FALSE so they are shown by default?

As per the comment https://www.drupal.org/project/drupal/issues/3459246#comment-15705249 ✨ Add a '#selectall' option for checkboxes element Active

There should be option #check_all which I would say should be TRUE by default and display the button to toggle check-all/uncheck-all and FALSE should not render this/these buttons.

Based on what we are implementing (#select_all, #selectall or #check_all or #checkall ) we should change in the issue title as well :).

Regarding #16 and the lines that I had previously removed from @MaxPeh's MR - it seems that there might have been a misunderstanding regarding the functionality on my end.

Please explain in greater detail what the #check_all option should be and where it would reside. Is this supposed to be handled via the Form render tree as a separate attribute like #type, #title, #check_all, #value, etc.? That would make sense to me.

Also, regarding #22, yes, I agree that we should reduce this option to a single button if possible. "Check all" if none are checked, "Uncheck all" if all are checked. The question remains what we want to do if only of the checkboxes are checked - do we use "Check all", "Uncheck all", do we show both buttons then or do we hide the button completely since we cannot really know what the user wants to do?

@LRWebks:
Usage example:

$form['favorites']['colors'] = [
  '#type' => 'checkboxes',
  '#options' => ['blue' => $this->t('Blue'), 'red' => $this->t('Red')],
  '#check_all' => TRUE, // This makes the "Check all" functionality appear!
  '#title' => $this->t('Which colors do you like?'),
  ...
];

(https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Render%21...)

The option must fallback to FALSE to not introduce a breaking change (opt in).

Regarding the single button I'd just use a button with
(Un)check all label.

The logic is:
If *any* checkbox is unchecked, check all. If all checked, uncheck all.

I think that's what would be expected.

I have added the #check_all property, reduced the two buttons to a single one according to @Prashant.c and @Anybody's suggestions and updated the CheckboxesTest.php to the new changes.

Credit definitely goes to @MaxPah for laying out the solid groundwork, which I had definitely misinterpreted before :)

It's good to know that the newly added test also found some cases in Checkboxes.php where we forgot to check if a render array key was set before using it, so I changed these two lines as well (not really unrelated, since we have just now created the tests for the Checkboxes element as a whole).

I'm going to take a look at what's causing the other tests to fail now!

@LRWebks thanks!

Better suggestion for the label: "Check all / none" - I think that's clear and short.

Afterwards please implement the field api checkbox functionality / ui. :)
Could you assign this task to you please, while you're working on it actively?

Do you want me to fully implement the checkboxes field for the Field API or add this function to an existing field? Because as far as I can see there is only a field for a single checkbox and not for multiple. We would need to build this field completely, right?

@LRWebks

You have created a new branch instead you should have pushed your changes to the existing branch https://git.drupalcode.org/issue/drupal-3459246/-/tree/3459246-add-a-sel... itself.

We do not need this branch https://git.drupalcode.org/issue/drupal-3459246/-/tree/11.x, you can hide it.

• Custom for element "Check all/none" not displaying by default which is correct

Adding '#check_all' => TRUEdisplays the button to toggle check all, which is also as expected

• In the Checkboxes field widget the "check all/none" setting is also available and working fine, tested with the content types. Should not be an issue with other entity types as well.
• We need to fix the existing failed tests and write new tests for this.

We need to fix the existing failed tests

Fixed in latest commit!

So the last part is to write additional tests. Any experience here @Prashant.c?

LRWebks → changed the visibility of the branch 11.x to hidden.

@Prashant.c regarding #30: Yes, that branch was accidentally created and contains no changes other than a rebase, it's hidden now and can be disregarded.

I’d assumed the UI would be an extra checkbox at the top of the column, the same way as for tableselect.

@joachim

I’d assumed the UI would be an extra checkbox at the top of the column, the same way as for tableselect.

Do you know about any best practices or widely used libraries solving it that way (or in another way)?

For tables, it's a widely known pattern to have a "Check all / none" checkbox in the heading column. Here we don't have a heading column, but a list of checkboxes and I can hardly imagine how it would look like having "just another" checkbox at the top or bottom to check them all, even if we make the "Check all / none" checkbox text or the whole row styled differently.
Furthermore, it might lead to misunderstandings in meaning, if that checkbox is checked (but others not maybe). We don't run into these cases using a button.

So I'm absolutely not against other better solutions than using the button (which might also get some styling or icon maybe), but I haven't seen a better one yet, so I think there are more good reasons for the button than for another checkbox in this situation.

What do you think how to proceed in that point, is that a blocker for you?

PS: Styling this button less prominent and maybe smaller, perhaps introduce a global class for that button would make sense to me.

Added the test for the field widget and works! Now we still need to discuss #37, I would be fine with both keeping it as is or introducing a new global class (I used the button--small class as that seemed like the most fitting class that already existed in the UI).

@LRWebks Great work. In the test, you have covered the widget on the entity but I think we need tests for the custom form element also.

@Prashant.c will you do that please?

I just reviewed and tested the functionality and it works great - as expected.
Setting this RTBC now to probably get final maintainer feedback. I think the time is right for that.

Some points from my side:

• Additional tests for the form element needed? (#39)
• Maybe rename the library & js file from check-all(.js) to checkboxes-check-all(.js) or something like that to make the context more clear?
• Do we need to support disabled JS? That would mean, we should hide the button initially (display: none) and show it on JavaScript init. I wouldn't add the button in JS entirely, as the Form API is much cleaner here.

But yes in general this is excellent and super helpful for many checkboxes!

The proposed resolution here is to "implement a proof of concept for this functionality in contrib". Clearly this is more than than. Adding tag for an issue summary update to clarify exactly what this is implementing.

This is changing the UI so there should be before and after screenshots available from the issue summary. Also, tagging Usability. Speaking of usability has the this had a review of the new text? I don't see one say adding tag for that. For that, I suggest asking in #ux how to get that review.

I read the comments in the MR and left some comments. I did not do a core review or test the change.

@quietone @Anybody

Current Status:

• Corrected the test comment mistakes
• Renamed check-all.js to checkboxes-check-all.js as the previous name was a bit too ambiguous to be in the misc folder (see #40 for the idea)
• Regarding the tests for the Form element (#39, #40), these are already in place, see CheckboxesTest.php
• Updated the issue summary
• Added "before" and "after" screenshots as @quietone requested

Stuff that is left:

• Usability review
• Final discussions

Back to RTBC, all points look addressed! Thank you @LRWebks!

How to get the usability review

Not sure what I think it would be great to do some accessibility review as well?

The issue would need a rebase. on a none case sensitive file system ( i am on a mac) i run into the following error:

$> git checkout -b '3459246-add-a-selectall-11.x' --track drupal-3459246/'3459246-add-a-selectall-11.x'
error: The following untracked working tree files would be overwritten by checkout:
	core/modules/system/tests/modules/driver_test/src/Driver/Database/DrivertestMysql/Connection.php
	core/modules/system/tests/modules/driver_test/src/Driver/Database/DrivertestMysql/Install/Tasks.php
	core/modules/system/tests/modules/driver_test/src/Driver/Database/DrivertestMysqlDeprecatedVersion/Connection.php
	core/modules/system/tests/modules/driver_test/src/Driver/Database/DrivertestMysqlDeprecatedVersion/Install/Tasks.php
	core/modules/system/tests/modules/driver_test/src/Driver/Database/DrivertestPgsql/Connection.php
	core/modules/system/tests/modules/driver_test/src/Driver/Database/DrivertestPgsql/Install/Tasks.php
Please move or remove them before you switch branches.
Aborting

it happens when the issue fork was created or last rebased before this issue went in https://git.drupalcode.org/project/drupal/-/commit/8b368d712d83900765744... on the 13th of august fixing spelling errors. the switch to camel case is the problem where the none case sensite file system in combination with git struggles and leads into an error.

@rkoller I have rebased it, hope it won't show some test failures :).

thanks for the quick rebase @prashant.c (don't wanted to rebase myself in case an conflict turn up on rebase). tests are green, the error is gone, and i was able to checkout the feature branch now.

and in regards of the usability review, thanks to @quietone for adding the need usability review tag. i've added the issue to our shortlist ( #3468437-3: Drupal Usability Meeting 2024-08-23 → ). but just a headsup, not sure if we will be able to get to it this friday. at the moment is vacation time and we already had only three attendees last week and one of them is away this week. so not sure how many attendees will show up on friday and we should have at least three as the bare minimum to have a reasonable discussion with enough different perspectives. but i will have the issue prepared in case enough people show up.

and i've already taken a brief look. I'll add the tag for the WCAG success criterion relevant to this issue: https://www.w3.org/WAI/WCAG22/Understanding/name-role-value.html. Just a brief note based on the discussion happened in #35 and #36. No matter if going with a button- or a checkbox-solution to check and uncheck all checkboxes, the aural interface has to properly communicate the state to people with screenreaders. I've attached a quick video illustrating the current MR with voiceover in safari (button.mp4). in the current state the state of the toggle button is not communicated at all. adrian rosselli elaborated about the topic a few weeks ago: https://adrianroselli.com/2024/03/check-all-expand-all-controls.html . According to him no matter if you go with a button or a checkbox you have to use something that supports tristate. in his example he provided a demo for the checkbox solution: https://adrianroselli.com/demos/do-undo-all/ (see checkbox.mp4). It is definitely a good issue to discuss in an ux meeting. nothing i want to give any recommendation on at this point on my own. just wanted to illustrate the potential issue in regards of screenreaders and state upfront, which is imho one of the tricky aspects for this issue. will have to think about it for a bit.

@rkoller thanks a lot!!

Regarding the options, my favourite would still be to go with a button, perhaps we can dynamically change the button label / title:
Check all (if one unchecked) <> Uncheck all (in all checked)
At least I personally dislike the checkbox tri-state approach for UX

Would be great so see if there are best practices on this field!

On the subject of checkbox vs button and old issue has a 9 year old comment #36 that is still IMO valid: "The functionality is already in Core, it's just about applying this consistently across the UI."

The functionality is in admin/content, nut missing from other similar lists.
I think we should aim to make the functionality available "everywhere" and decide on a unified style of UI for it across the admin UI.

i agree with simo. the only problem, on admin/content the "check all"-checkbox is placed in the table header. so it is not directly applicable to the pattern used on a list field. I've done some research in preparation when we find time to discuss the issue in ux meeting. Hope we get to the issue tomorrow.

Usability review

We discussed this issue at 📌 Drupal Usability Meeting 2024-09-06 Needs work . That issue will have a link to a recording of the meeting. For the record, the attendees at today's usability meeting were @benjifisher, @rkoller, @simohell, and @worldlinemine.

If you want more feedback from the usability team, a good way to reach out is in the #ux channel in Slack.

We have taken a look at the current state of MR8985. Overall we had the consensus that adding a check-all option would be a good thing. First a few details we’ve noticed during our explorations:

• Completely out of scope: While setting up the select field, we’ve realized it is not communicated anywhere in the admin UI how to choose between checkboxes or radio buttons. The option on the form display widget is simply called “Check boxes/radio buttons”, but it is not communicated anywhere that things are controlled via the cardinality/“allowed number of values”-setting on the field settings page. A cardinality limited to one switches to radio buttons, while anything bigger than one or Unlimited switches to checkboxes.
• On a related note relevant to this issue, if you set the allowed number of values to one for the select field, the Show "Check all / none” button checkbox or better the form display widget settings in general is shown anyway, even though it is irrelevant without any purpose in this context.
• The check-all button has a too low color contrast (#d3d4d9 against #ffffff => 1.5:1 -> 3:1 necessary), but there is already an issue for Core: 🐛 Grey button's background color has a too low contrast ratio against page background Needs review .
• The button is no toggle button, so the current as well as the future state is unknown which is potentially confusing for screenreader users (see voiceover.mp4). Even though aria-pressed is supporting tri-state with offering the mixed state (https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attribut...), the support across screenreaders and browsers is subpar: https://a11ysupport.io/tech/aria/aria-pressed_attribute. Plus there is no component to communicate a pressed button state in the Drupal Design System at this point, and then there would also be the need for a “mixed” button state as well. And how to communicate such an intermediate state for a button visually?
• With the button state unknown, the button label adds another level of confusion - it contains both possible future states within the Check all/none label and readability wise it is sort hard to process with the slash separating the two possible states . For sighted users with the visual context, it might make them think for a second, but after using that pattern once it is manageable. Screenreader users instead don’t have the state nor the visual context available, just based on the button label they don’t know if pressing the button will check or uncheck everything and which of the available options is already ticked? Strictly speaking a screenreader user would have to check every available option before going back to the check-all button. The only shortcut might be by pressing the button, check the first option and based on its checkbox state go back to button in case the state is the opposite of the desired state. And switching the button label based on the state as suggested in #50 → has also its downsides for screenreader users as described for the “show and hide password” pattern by Leonie Watson in this webinar by the Smashing Magazine: https://youtu.be/OUDV1gqs9GA?si=b4cf_sUtRiF5DJ1f&t=3220
• Visually, the button and the set of checkboxes underneath feel disconnected, it feels like two separate sets of controls, one button and a set of eight checkboxes that are combined underneath a headline. Despite the spatial proximity it does not seem that the two are one functional unit.

After we’ve gained an initial overview we’ve turned towards the fundamental question, which of the two options should be selected, the button or the checkbox based pattern? In the run-up to the meeting I’ve done some research looking for resources about the check-all button and check-all checkbox pattern in the context of UX related resources. It has to be noted that all links in the following are about checkboxes, I was unable to find any article nor example about the check-all button pattern:

https://ux.stackexchange.com/questions/98089/nested-checkbox-behavior-ch...
https://www.nngroup.com/articles/checkboxes-design-guidelines/
https://app.uxcel.com/courses/ui-components-n-patterns/selection-control...
https://css-tricks.com/indeterminate-checkboxes/
https://uxdesign.cc/selection-controls-ui-component-series-3badc0bdb546
https://blog.logrocket.com/ux-design/checkbox-ui-design-best-practices-e...
https://forum.figma.com/t/select-all-checkbox/48623/5
https://www.setproduct.com/blog/checkbox-ui-design

Then there are few examples of design system that are using the nested checkbox list pattern described in the aforementioned list of resources:

https://carbondesignsystem.com/components/checkbox/usage/
https://m2.material.io/components/checkboxes
https://atlassian.design/components/checkbox/examples

So overall the nested checkbox list pattern seems to be the solution of choice from an UX perspective. This also corresponds to the accessibility perspective reflected in the following resources:

https://adrianroselli.com/2024/03/check-all-expand-all-controls.html
https://adrianroselli.com/demos/do-undo-all/
https://dequeuniversity.com/library/aria/checkbox-tri
https://www.w3.org/WAI/ARIA/apg/patterns/checkbox/examples/checkbox-mixed/
(*the behavior in the w3.org example is odd and confusing since it remembers which checkboxes of the children were ticked for the indeterminate state across checkbox presses)

At the moment, the favorite and preferred option for solving this issue is to go with a check-all button, according to #50 → . But based on the fact that the majority of resources we've found during the research are utilizing checkboxes to check other checkboxes, Drupal already using check-all checkboxes in other contexts within the admin UI, and us trying to keep the admin UI consistent not using different interface components for the same kind of functionality (see #51 → ). Unfortunately all these points are not in line with a check-all button. :( So we would recommend the following points:

1. If the allowed number of fields is set to 1 on the field settings page of a select field, hide the settings for the Check boxes/radio buttons option on the corresponding form display widget.
2. Replace the check-all button with a checkbox. Visually distinguish the check-all checkbox from the checkboxes underneath by either indentation seen in the various link examples above or by adding a separation line/border.
3. Replace Show "Check all / none" button with Select all as the label for the check-all checkbox.
4. Open a follow-up issue that is adding the tri-state checkbox pattern for Core in general. So an intermediate state is added to the already existing checked and unchecked state for check-all checkboxes. Aside select fields on node edit forms we have found the following other places that are using this check-all checkbox pattern (there might be even more): admin/content, admin/content/comment, admin/content/media, admin/content/media-grid, admin/people, and on a content moderation workflow page within the dialog modal for Operations.

For the point mentioned in the beginning which is out of the scope for this issue I will open up a new issue. I'll remove the "needs usability review" tag and set the issue back to "needs work".