List View: Add multi-select support for shift + Home and End keys #39272
Conversation
andrewserong
added
[Type] Enhancement
|
Size Change: +4.04 kB (0%) Total Size: 1.21 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
@andrewserong The change is almost perfect for me. The only issue is if I press Ctrl+End to select all blocks from my current position, it announces the number selected plus the last selected block. This really isn't helpful in this context. Can you ditch the last block name if selection is > than 1 based on the select Home/End keys event?
There was a problem hiding this comment.
LGTM, thanks for working on another good improvement to List View.
From what I can tell, the issue @alexstine mentioned was solved by that last commit, but would be good to get a thumbs up from Alex before merging.
|
Thanks for the quick review @alexstine! I've pushed an update that skips announcing the deselected blocks when the new selection is generated via the Home/End keys and the selection is greater than 1. Is that what you had in mind? It's sounding better to me now using the macOS voice over feature, but I'm happy to keep tweaking if this change didn't quite address the issue 🙂 |
|
@andrewserong Seems that the problem got worse. 👎 Here is what the screen reader announces.
Here all the extra selected blocks? Just the total would be enough.
Hope this helps. Thanks. |
|
I don't have much to add here, but just wanted to thank everyone involved (especially @alexstine — your feedback is always very useful, thank you!) |
|
Thanks @alexstine, that feedback is very helpful! I've been testing with the voice over function in MacOS where the additional announcements of "Image (selected block)" aren't happening. I wonder if it's to do with that component, which is currently focused, being updated when the block selection changes. I'll see if I can install a Windows VM to try out NVDA or another screen reader to see if I can replicate what you've found in testing. I'll follow up! |
|
Alrighty, I'm wrapping up for the day, but I've set up a local environment running Windows 11 and NVDA and can reproduce the problem exactly as Alex described. As a side effect of this, I now have a much better testing environment! I'll do some exploration over the rest of the week to see what we can do to try to fix this up 👍 |
andrewserong
force-pushed
the
update/list-view-to-support-home-and-end-keys-for-multi-select
branch
from
6a53419 to
3d07283
Compare
|
Alrighty, I think I've worked out why there are additional announcements of the block that the user started on, and the destination block (I just don't have a fix for it yet 😅). In I'm not quite sure what the best workaround is for this, since we're relying on state changes for the screen reader to announce these components rather than a call to Thanks for rolling out the changes to |
|
I don't have a screen reader set up yet, but the interactions are working well for me. The only thing I'm not sure about – and it might be just because I'm using a mac keyboard – is that I can't trigger home or end (I assume it's meant to select the entire block?) The first few seconds of the screen case is me trying the documented
Any tips? Non-blocking. It's a helpful interaction to have |
There was a problem hiding this comment.
Thanks for taking this on! The multi-selection behaviour with Shift+Home/End is working fine with the keyboard on all browsers except for Safari. (@ramonjd if you were testing on Safari, that wonky behaviour with Fn + Arrow Left/Right seems to be browser-specific; it's working fine on Firefox and Chrome)
The last selected item being announced by screen readers on multi-selection is also reproducible on trunk when selecting multiple items with Shift+Arrow keys, so I don't think it should be a blocker for this PR. We can investigate it separately.
| ) { | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
In testing on both Windows with NVDA+Firefox and macOS with VoiceOver+Firefox, this change doesn't seem to make any difference. The last selected block of the multi-selection is always announced, whether the multi-selection happens with Home/End keys or just Shift+Arrow Up/Down. And if for @alexstine the change seemed to make things worse, I think it's safe to remove it altogether.
There was a problem hiding this comment.
This change is good when selecting with Arrows but when using Home/End, it should only announce the number of selected/deselected. Why? Let's look at this practically.
- I press Shift+Down Arrow to select a block.
- I hear the total number of blocks selected plus the block I just selected.
- If I press End, it will select say 5 blocks but then I only hear the last block get announced.
See how this can get confusing?
When having these Home/End selections, it should just announce the total as in reality you selected multiple blocks at once vs. one block at a time.
There was a problem hiding this comment.
I see what you mean. When pressing End, the last block in the selection gets focused; that must be why it's being announced. I wonder if we can instead keep focus on the first block in the selection - and if there won't be any negative side-effects to that 😅
There was a problem hiding this comment.
Thanks for taking a closer look here! I was wondering if it's possible for us to conditionally add aria-hidden set to true in the block select button component for just the case of having switched focus while holding shift plus Home / End, to prevent the block from being announced. I might have a play.
There was a problem hiding this comment.
I think focus should always follow the selection. Otherwise when you press up or down again it becomes disorienting.
Looking at how Finder works, it's similar to what Alex describes (though you have to use shift+option+arrow key instead of home or end to select to the start or end). It'll only announce '6 rows selected' and it won't announce the file that is focused (even though focus does move).
Is there any way to suppress the innate announcement that screenreaders make when focus moves?
There was a problem hiding this comment.
I was also wondering if there's any advantage in using roles like aria-multiselectable:
https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-multiselectable
It didn't seem to do anything when I tested adding it, but maybe it needs to be used in combination with aria-selected.
The docs seem to imply that's ok with a tree, but don't say if that extends to a treegrid.
andrewserong
force-pushed
the
update/list-view-to-support-home-and-end-keys-for-multi-select
branch
from
d850880 to
2692f44
Compare
|
I tentatively think I might have come up with a fix for the announcement issue in 2692f44. Thanks again for the feedback and discussion, folks! I've rebased to resolve some conflicts with In that change, I've added some state to the parent list view to keep track of whether or not we should prevent announcing the label of the block selection button. When we shift select with Home or End, we set Then, when we move focus or select any other row, the This appears to be testing pretty well for me so far in NVDA on Windows 11 running in a VM on a Mac. But I'd love a confidence check on the approach if you get a chance to take this for another spin @alexstine. No rush at all, though, as I'm finishing up for the week. Also, very happy to throw out that approach if folks think there's a better way to do it, or if this approach feels too hacky! |
|
Tested this again on Safari+VoiceOver (the home key combo is now working fine on Safari, not sure what happened earlier) and on Firefox+NVDA. In both the currently focused block is read out after "x blocks selected". As Dan mentioned the Not quite sure what else we can try here. |
|
Thanks again for all the feedback and testing @tellthemachines, @alexstine, and @talldan! I did some more investigation, thanks for the link to the alternate PR @tellthemachines, that sparked some ideas 😀 Just a heads-up, there's been a rebase recently, so you may need to remove your local copy of this branch before re-testing, in case that's caused any issues. I think the root of our challenges here is that the set of roles that we're working with in the tree grid is similar to that of working within a grid of tabular data (e.g. the row role or the gridcell role. Rather than wrestle too much with the browser here, I've had a go at reverting the In the latest update in 159b2a2, I've done a few things:
This appears to be testing much better from my perspective using NVDA in Windows 11, where navigating between blocks now only reads that block out once. Shift-selecting all the blocks now no-longer reads out the block you started on a second time (without us having to add the hacky There is the remaining issue that we cannot appear to prevent the destination block from being announced after the selection. This appears to be browser default behaviour of navigating around cell-like elements, so it's a tricky one! I'm wondering if it's enough of an improvement that the block is only announced once instead of twice? With the current state of the PR, the output from NVDA now sounds like the following in my testing: Please let me know if you think this direction seems viable — my hunch is that focusing on having the announcements at the cell or row level will help keep the behaviour contained and focused, but again, happy to throw any of this out if it doesn't seem suitable! Thanks again for all the help and patience as we explore the different options 🙇 |
|
@andrewserong It is better, there is no doubt about it. This is the problem I am seeing now. It might honestly be out of scope for this PR.
In another PR, we've got to fix the label on this. This has confused me so bad, I don't even know what to make of that. We need to use simpler language somehow instead of expecting users to actually know what this means.
Why can I not deselect all blocks? Is it because the paragraph block is selected in the editor but not the selection as far as copy/paste? This confuses me why it says selecting 9 blocks but deselecting 8. Thanks and great work on this! |
|
Thanks for confirming, Alex! Since this approach seems to improve things, I'll proceed now with updating the e2e tests to get them passing (they're currently expecting the
Agreed. At least we've got some more information now on how to change things, so the hard part will be just deciding what we want it to say 🙂
Oh, that's a good point, sorry I meant to follow-up on your comment there yesterday. I think the problem is that in comparable interfaces (such as a spreadsheet or file system) there is always at least one item currently selected, whereas in this interface there is such a thing as a focused but not selected item, which means we've got some more complexity to deal with. It's hard to know if when a user goes back to the original item with the shift key selected, whether or not they would like that item to remain selected. At the moment the logic assumes that the shift key means the original item is always going to be selected. Perhaps it'll be easier in a separate PR to look at the desired behaviour for de-selecting items, and explore which options might make sense? I'd be happy to look into that in a follow-up if it isn't a blocker for this PR. |
|
@andrewserong Follow-up PR is certainly okay. Super excited with the progress here. Let's keep these improvements coming. 👍 Thanks! |
There was a problem hiding this comment.
Thanks for your continued work on this! The latest batch of changes is testing much better than previously; I just left a couple of comments below.
Regarding the inability to fully deselect list items, I think that should be addressed separately because it's an existing issue, not introduced by this PR.
@alexstine, is the Block 2 of 2, Level 2 row 13 column 1 part too verbose? Perhaps we don't need to indicate whereabouts on the tree every time we move focus around. I think that would also be a good candidate for a follow-up though. It'd be good to get this in, as it's already better than what's currently on trunk!
|
@tellthemachines I totally agree.
|
|
Alrighty, I've updated this PR with the following:
Please let me know if I've missed anything, or if it needs further tweaking! |
|
Thanks for reviewing again @alexstine! I've added back in the code to skip announcing the deselect, and removed the I'll merge this in after lunch, once the tests pass! |
|
@andrewserong Sounds good. Then we can move on to more fun with this selection stuff. Maybe it would be a good idea to start a tracking issue if one doesn't already exist. Still a couple weird things to discuss. If you want to draft it, I can start adding some things to check unless you remember my spread out notes. |
|
Sure thing, I'll open up a tracking issue a little later on today, and will ping you on it! |
andrewserong
deleted the
update/list-view-to-support-home-and-end-keys-for-multi-select
branch
|
I've opened up a follow-up issue to track some of the suggestions and ideas raised in the reviews here. Let me know, or add to that issue if I missed anything! |
|
Thank you all for the great work here! |
mtias
added
the
[Focus] Accessibility (a11y)
Description
Following on from #38314, this PR adds in support for shift-selecting multiple blocks within the list view via the Home and End keys. The Tree Grid component already supports navigating to the beginning and end of the block list via these keys — this PR is a fairly simple update that rolls out adding in the
onFocusRowcallback for those key press events, too, and accepting those events as valid key presses for multiple block selection inuseBlockSelection.The end result is that it should now be a little easier to select from a particular block within the list view, to the end or beginning of a post.
Testing Instructions
Check that the added tests pass in CI (or you can use the following to run locally):
Screenshots
In the following screengrab, the Home and End keys allow selection to the beginning or end of the list while the shift key is selected. The up and down arrows then still enable altering that selection.
Types of changes
Enhancement.
Checklist:
*.native.jsfiles for terms that need renaming or removal).