Listboxes with Rearrangeable Options: Add keyboard commands for multiple selection #1344

smhigley · 2020-02-24T22:21:40Z

Resolves #919

Adds the following functionality:

Shift + arrow keys
Control + Shift + home and end
Control + A
Shift + Click

Includes tests :)

Preview link

Review checklist

Editorial review by Matt (@mcking65)
Functional review by Carolyn (@carmacleod) and Matt (@mcking65)
Code review by Carolyn (@carmacleod) and Simon (zcorpan)
Test review by Simon (@zcorpan)

examples/listbox/js/listbox.js

carmacleod

Nice!

Confirmed that the new "multi-select" keys for Example 2 now work as advertised!
Thanks for adding shift+click as well.

However, Example 1 now selects all options when Ctrl+A is typed. :)
(see inline code suggestion).

Might want to add a new ariaTest for 'key control+A does not select all options in example 1'. ;)
Should the test title explicitly list "example 1"? Not sure what we usually do for test titles for multiple examples, but it might be a bit confusing if we had one test that said:

'key control+A selects all options'

and another that said:

'key control+A does not select all options'

[Edit: Wow - you are way ahead of me! You've already committed a fix! I'll have a look later this evening].

smhigley · 2020-02-25T01:24:08Z

@carmacleod I added a couple more tests for the control + a behavior, including the one you suggested :)

Let me know if that addresses everything, and thanks for the review!

carmacleod

Looks good!
+1

mcking65

Editorial changes and functional changes are good, thank you @smhigley!!

zcorpan · 2020-03-04T14:12:30Z

So on macOS, the usual shortcut for select all is Command+A, not Control+A. How should we handle this? Support both Control+A and Command+A? Detect macOS and change the shortcut from Control+A to Command+A?

zcorpan

Code review and test review done. Looks good and tests pass. There's a typo in a comment elemnt.

We could merge this and follow up with the question about macOS in a new issue.

examples/listbox/js/listbox.js

carmacleod · 2020-03-04T20:01:10Z

examples/listbox/js/listbox.js

@@ -196,6 +218,14 @@ aria.Listbox.prototype.checkKeyPress = function (evt) {
        this.focusItem(nextUnselected);
      }
      break;
+    case 65:
+      // handle control + A
+      if (this.multiselectable && evt.ctrlKey) {


To answer @zcorpan 's question:

So on macOS, the usual shortcut for select all is Command+A, not Control+A. How should we handle this?

Detect macOS and change the shortcut from Control+A to Command+A

Something like this should do it:

Suggested change

if (this.multiselectable && evt.ctrlKey) {

var isMac = window.navigator.platform.indexOf("Mac") !== -1;

var mod = isMac ? evt.metaKey : evt.ctrlKey;

if (this.multiselectable && mod) {

I don't think we need to detect Mac. I just added (evt.ctrlKey || evt.metaKey). That should work for Mac, and doesn't cause issues with the Windows key, since Windows + A is eaten by the OS anyway.

@zcorpan let me know if it's working for you with that change

Windows + A is eaten by the OS anyway.

Ha! I did not know that. :)
Seems to open up the notification sidebar.
Whaddaya know.
Thanks for the lesson!

What will this logic end up doing on Linux where I believe we want it to be ctrl-a? There could be other platforms, even some mobile with BT keyboards that will allow these commands to be used. I just pushed a change to the keyboard documentation, but I don't believe what I wrote is entirely accurate using this logic.

Hmmm... now that I'm looking at the logic, I believe it makes both Ctrl+A and Cmd+A work on macOS. Do we want that?

@mcking65
I don't think there's a KeyboardEvent.metaKey on Linux. Hopefully someone can confirm.

The doc says the only other metaKey is the Windows key on Windows keyboards, and @smhigley mentioned that Windows+A is eaten by the OS (i.e. our code would never see it).

The code within the case for the 'A' (65) key being pressed is:
if (this.multiselectable && (evt.ctrlKey || evt.metaKey))

so the logic would work as follows on Linux:
"if the list is multiselectable and the control key was pressed (with the 'A')"

So || evt.metaKey would never be evaluated.

So I think the logic is ok for Linux, but I don't think it's ok for macOS, unless we want both ctrl+A and command-A to select all.

@mcking65
You are correct about the keyboards! I googled a bit and discovered that some older Linux keyboards and even some recent bluetooth android keyboards have a meta key (apparently for the mobile ones, the "search" key maps to metaKey). So even though those keyboards are likely rare, we don't want meta+A to select all. We only want that on Mac.

@smhigley I think we need to go with the code that detects Mac:

var isMac = window.navigator.platform.indexOf("Mac") !== -1; var mod = isMac ? evt.metaKey : evt.ctrlKey; if (this.multiselectable && mod) {

examples/listbox/listbox-rearrangeable.html

css-meeting-bot · 2020-03-17T18:38:47Z

The ARIA Authoring Practices (APG) Task Force just discussed Listboxes with rearrangeable options.

The full IRC log of that discussion

<carmacleod> TOPIC: Listboxes with rearrangeable options
<carmacleod> github: https://github.com//pull/1344
<ZoeBijl> regrets+
<carmacleod> sarah: prefer not to detect platform, in case it's not future-proof
<Jemma> mck: adding this var mod = isMac ? evt.metaKey : evt.ctrlKey;
<carmacleod> sarah: so we can support Ctrl+A select all on Mac as well as Cmd+A for select all
<carmacleod> mck: I'm ok with that - I'll update the documentation table
<carmacleod> mck: will merge after updating table

Listboxes with Rearrangeable Options: Add keyboard commands for multiple selection (pull #1344) Resolves issue #919 by adding the following commands for selecting options: - Shift + arrow keys - Control + Shift + home and end - Control + A, command-a on macOS - Shift + Click Adds regression tests for the commands. Co-authored-by: Matt King <a11yThinker@gmail.com> Co-authored-by: Carolyn MacLeod <Carolyn_MacLeod@ca.ibm.com>

…ple selection (pull w3c#1344) Resolves issue w3c#919 by adding the following commands for selecting options: - Shift + arrow keys - Control + Shift + home and end - Control + A, command-a on macOS - Shift + Click Adds regression tests for the commands. Co-authored-by: Matt King <a11yThinker@gmail.com> Co-authored-by: Carolyn MacLeod <Carolyn_MacLeod@ca.ibm.com>

smhigley requested review from carmacleod and spectranaut February 24, 2020 22:24

smhigley force-pushed the 919-listbox-multiselect-keys branch from 3919615 to 8c00dd1 Compare February 24, 2020 22:30

carmacleod reviewed Feb 25, 2020

View reviewed changes

examples/listbox/js/listbox.js Outdated Show resolved Hide resolved

carmacleod requested changes Feb 25, 2020

View reviewed changes

carmacleod approved these changes Feb 25, 2020

View reviewed changes

mcking65 changed the title ~~Implement multiselect keyboard shorcuts~~ Listboxes with Rearrangeable Options: Add keyboard commands for multiple selection Feb 28, 2020

mcking65 requested review from mcking65 and zcorpan February 28, 2020 09:31

mcking65 approved these changes Feb 28, 2020

View reviewed changes

zcorpan approved these changes Mar 4, 2020

View reviewed changes

examples/listbox/js/listbox.js Outdated Show resolved Hide resolved

carmacleod reviewed Mar 4, 2020

View reviewed changes

mcking65 force-pushed the 919-listbox-multiselect-keys branch from a22e204 to 0931755 Compare March 9, 2020 05:41

carmacleod reviewed Mar 9, 2020

View reviewed changes

examples/listbox/listbox-rearrangeable.html Outdated Show resolved Hide resolved

mcking65 force-pushed the 919-listbox-multiselect-keys branch from ea05c85 to 6b322b5 Compare March 24, 2020 07:58

smhigley and others added 8 commits March 30, 2020 23:51

implement multiselect shortcuts and shift click

f63bf82

Fix fallthrough and ctrl a conditional

a63b1d1

add additional control a tests

ae14c0c

update spelling of cloak

ea36e42

handle mac command + a, fix typo

dedabc2

Add command-A to keyboard documentation

d8862ae

Correct spelling of macOS

2dd19f5

Update control+a documentation per Mar 17 meeting

b85474a

mcking65 force-pushed the 919-listbox-multiselect-keys branch from 6b322b5 to b85474a Compare March 31, 2020 06:51

mcking65 merged commit f84febc into master Mar 31, 2020

mcking65 deleted the 919-listbox-multiselect-keys branch March 31, 2020 07:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Listboxes with Rearrangeable Options: Add keyboard commands for multiple selection #1344

Listboxes with Rearrangeable Options: Add keyboard commands for multiple selection #1344

smhigley commented Feb 24, 2020 •

edited by zcorpan

Loading

carmacleod left a comment •

edited

Loading

smhigley commented Feb 25, 2020

carmacleod left a comment

mcking65 left a comment

zcorpan commented Mar 4, 2020

zcorpan left a comment

carmacleod Mar 4, 2020

smhigley Mar 4, 2020

carmacleod Mar 5, 2020

mcking65 Mar 9, 2020

carmacleod Mar 9, 2020

carmacleod Mar 9, 2020 •

edited

Loading

carmacleod Mar 9, 2020

css-meeting-bot commented Mar 17, 2020

-      if (this.multiselectable && evt.ctrlKey) {
+      var isMac = window.navigator.platform.indexOf("Mac") !== -1;
+      var mod = isMac ? evt.metaKey : evt.ctrlKey;
+      if (this.multiselectable && mod) {

Listboxes with Rearrangeable Options: Add keyboard commands for multiple selection #1344

Listboxes with Rearrangeable Options: Add keyboard commands for multiple selection #1344

Conversation

smhigley commented Feb 24, 2020 • edited by zcorpan Loading

Review checklist

carmacleod left a comment • edited Loading

Choose a reason for hiding this comment

smhigley commented Feb 25, 2020

carmacleod left a comment

Choose a reason for hiding this comment

mcking65 left a comment

Choose a reason for hiding this comment

zcorpan commented Mar 4, 2020

zcorpan left a comment

Choose a reason for hiding this comment

carmacleod Mar 4, 2020

Choose a reason for hiding this comment

smhigley Mar 4, 2020

Choose a reason for hiding this comment

carmacleod Mar 5, 2020

Choose a reason for hiding this comment

mcking65 Mar 9, 2020

Choose a reason for hiding this comment

carmacleod Mar 9, 2020

Choose a reason for hiding this comment

carmacleod Mar 9, 2020 • edited Loading

Choose a reason for hiding this comment

carmacleod Mar 9, 2020

Choose a reason for hiding this comment

css-meeting-bot commented Mar 17, 2020

smhigley commented Feb 24, 2020 •

edited by zcorpan

Loading

carmacleod left a comment •

edited

Loading

carmacleod Mar 9, 2020 •

edited

Loading