-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
List View: Add keyboard shortcut to select all blocks #54899
List View: Add keyboard shortcut to select all blocks #54899
Conversation
Size Change: +88 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
Flaky tests detected in 7295a6d. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6555544512
|
a98f918
to
7295a6d
Compare
packages/block-editor/src/components/list-view/block-select-button.js
Outdated
Show resolved
Hide resolved
Update: now that the list view's performance when selecting all blocks has been resolved (#54900), I've given this PR a rebase and updated it to handle incrementally moving up the block hierarchy with each press of CMD+A / CTRL+A to match the behaviour in the editor canvas. I think it's working pretty well now, though I still need to add some e2e tests to ensure it's covered. I'll be AFK off and on over the next couple of weeks, so I'll look into adding e2e tests once I'm back. |
f4eef32
to
fa93b5f
Compare
Update: I've added some e2e tests in a194cc4 — it's a little verbose but captures the stepping up each level with each subsequent press of the keyboard shortcut. I'll do a little more thinking on it and see if we can simplify the test a bit as I think it might be too verbose in its current form. It might be enough to just select one of the column blocks instead of going all the way down deep in the nested hierarchy 🤔 |
a194cc4
to
39b91b5
Compare
Update, I've tidied up the e2e test as best I can so that it's simpler while still testing the iterative behaviour of "select all" where it begins with all siblings before advancing up the block hierarchy with each press of the keyboard shortcut. @alexstine if you get the chance I'd love any feedback on whether or not they keyboard behaviour in this PR is what you'd expect when attempting to select all blocks in the list view. I've tried to go for consistency with the behaviour in the editor canvas where "select all" means selecting all siblings at the current level before then advancing up the hierarchy, so that the shortcut is a bit more useful than just selecting all blocks in the document. Happy to try out other approaches if the one here isn't any good! |
Just adding the "Needs accessibility feedback" label, as it'd be good to double-check the keyboard behaviour is correct before landing. |
@andrewserong Sorry, been backlogged with life stuff. Found one bug.
Otherwise, this seems to work exactly as you describe. Thanks. |
Thanks for taking it for a spin, Alex! This PR definitely isn't urgent. From a quick test of the number of selected blocks being announced, I think this issue is present in the editor canvas, too. If we select blocks there, it seems the stats or knowledge of how many blocks are selected is based on how many parent or container blocks are selected rather than that number including the number of children, too. It'd be good to fix that up holistically, so that it's fixed in both places. I'm not too sure where we'll need to fix it, but I'm happy to dig in. |
Update: I've opened a separate issue for the problem of accurately describing the number of blocks selected over in #56068. Some of the challenges to resolving it will be that the selection can include template parts or synced patterns, so it might not be straightforward to correctly calculate how many child blocks are really selected. A fix for that will likely be outside of this PR, but I'm happy to let this PR sit for a while if the bug makes the behaviour in this PR feel too confusing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andrewserong Giving this PR a sign off. Let's look into the related issue next. I do believe this is a good improvement over what is currently there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also LGTM. Manually tested and works as described.
Thank you!
Thanks for the reviews, folks! I'll merge this in now, and let's investigate our options for how we calculate the number of selected blocks over in #56068. |
* List View: Try adding keyboard shortcut to select all blocks * Move up each leve of the hierarchy incrementally * Fix bug if focused on a different part of the list view from the block selection * Add e2e tests * Simplify e2e tests * Clarify the group block state a little more in the e2e test
What?
Part of: #49563
Within the list view, add handling for the keyboard shortcut to select all blocks (CMD+A on Mac, CTRL+A on Windows).
Why?
While navigating around the list view via keyboard, it could be handy for users to be able to quickly select all blocks via CMD+A on Mac / CTRL+A on Windows.
The approach in this PR follows that used in the editor canvas, where each press of CMD+A / CTRL+A moves one level up the block hierarchy until all blocks are selected.
How?
Add in similar handling to what's used in the editor canvas for
'core/block-editor/select-all'
and select all blocks when that keyboard shortcut is used.This approach will first select all siblings at the current level of the hierarchy, and will then iteratively move up the block hierarchy with each subsequent press of the keyboard shortcut until eventually all blocks are selected.
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
Screengrab of selecting up the hierarchy:
2023-10-18.13.38.46.mp4