-
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: Allow Escape key to deselect blocks if blocks are selected #48708
List View: Allow Escape key to deselect blocks if blocks are selected #48708
Conversation
@alexstine just for visibility, here's the draft I'm working on to try out deselecting blocks within the list view via using the Escape key. I wasn't sure, but for this one, will we need to add a |
Size Change: +1.22 kB (0%) Total Size: 1.51 MB
ℹ️ View Unchanged
|
Flaky tests detected in b68fcce. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4412306267
|
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 This mostly looks good but yeah, we'll need a speak( msg, 'assertive' );
on this. Maybe model it after the announcement in the current selection hook in the list-view.
speak( 'All blocks deselected', 'assertive' );
Is it okay to access blockEditorStore
outside of block-editor package?
Thanks for the feedback @alexstine! I've added the assertive
Yes, in the case of the edit-post and edit-site packages, the block editor store appears to be a safe dependency, and it's used quite a bit for block selection actions and selectors. |
Just including @WordPress/gutenberg-design as a ping since I know a few designers have helped out with UX flows of the List View in the past, so might have feedback on the approach here, too 🙂 |
@andrewserong I am working on changing that in my ARIA refactor, #48461. I will give this PR a test shortly. Thanks. |
Ah, yes, of course I forgot about that. Thank you for the reminder! |
Hey, thanks for taking a stab at this, this is really important to get right. This issue seems to be working for the list view, but I wonder, can we find a more basic solution that works for blocks in the canvas too? In #47172 I had an idea to allow esc twice to work for this. Would that thread the needle? |
@jasmussen I think pressing escape twice is a little excessive for this. Besides, most dialogs/pop overs in Gutenberg close with a single press of escape which can sometimes be a very destructive action. |
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.
I realize there's some conversation about direction, just noting that I tested this as it currently stands and it all worked as expected.
Thanks for the feedback @jasmussen! Totally agree that this is an important one to get right, and to try think about the implications holistically. In the case of the list view, the behaviour in this PR only applies to when focus is within the list items, where a user is interacting with and adjusting selection of the blocks. For the Escape key to be able to trigger a deselection, we'll probably always need to have some code to handle it here, so that we can skip closing the list view, as Alex mentions.
For the editor in general, what if we thought about it the other way, and had logic that says that if there is a multiple block selection in place within the editor canvas, then the first Escape press clears the selection altogether, and then the second press switches to the Select mode? This is a sample size of one, but personally I'd expect deselection to happen first before a mode switch. Whichever direction we go with, though, I agree that the behaviour in the List View should be consistent with expected behaviour elsewhere in the editor. I think this PR will likely still be a good shape for it within the List View, but I'm very happy for us to slow down a little on this PR to think it through — also happy to try other ideas if there are others in mind! |
Thanks for diving in. I tend to agree, and I suggested it mainly because I so wanted a way to deselect all blocks in the canvas using Esc, that I was willing to entertain the double-esc idea. I quite like Andrew's suggestion above, which is slightly simplified in this comment. Essentially that if you have blocks selected, it deselects them. Select mode you only enter if you don't have any blocks selected. (Or have 1 block selected? — I guess that's the main inconsistency to figure out.) What do you think about that behavior? |
@jasmussen Agreed. We should not be allowing the user to enter select mode if blocks are selected or closing the list view if blocks are selected. I think the double escape will confuse users if nothing happens on the first press though. Besides, what if something like this happens?
|
I realised that I forgot to include the widgets editor, too, which also has its own sidebar logic. In 1d4c96f I've included that one, too. Separately, I've had a go over in #48904 of trying out allowing Escape in the editor canvas to clear the block selection. At the very least, it highlighted that if we want to allow block deselection within the list view, this PR will still be required, as we need the logic to fire before the Escape key handlers in the list views that are responsible for closing the list view. So, whichever direction we think will work best for the editor canvas, I think that means this PR should still be viable. Or at the very least, is in a good place for review now 🙂 Let me know if anyone would like me to try out any other ideas not already covered in this PR and over in #48904 (that one just deals with the editor canvas, this one just with the list views). Thanks again for all the discussion here! |
… if focus is on the close or tab buttons, Escape closes the list view instead of clearing
…o blocks are selected
ab1613b
to
b93392d
Compare
Now that we're in the 6.4 cycle, I've given this a rebase and added a small addition to the existing keyboard shortcuts test for the list view. I think it's mostly working pretty well, but I haven't yet found a good way to always return explicit focus to the list view button when the list view is closed via the Escape key without any blocks selected. In some ways, that might be behaviour for us to look into separately, because it's possible to have focus within the list view without a block selected and then press Escape on |
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 Just gave this another test. While the selection announcement is working great, pressing Escape no longer gives any indication that blocks have been deselected. Was that an oversight on my part or is there a bug?
Thank you for re-testing @alexstine! It turns out it was a bug, I had forgotten that I needed to add extra handling to the I've retested using VoiceOver on a Mac and the deselection announcements are now happening for me when I press the Escape key. Please let me know if I've missed anything, though, and I'll dig in. |
@andrewserong This is looking good. If you promise to look into the loss of focus in a follow-up, I'm good to let this get merged now. To replicate:
Focus loss is a big problem but one I think for this situation, can be addressed in another PR. Thanks. |
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.
👍
Thanks for reviewing and approving @alexstine! Yes, absolutely, I'm happy to continue looking into the focus issue. I've opened up a separate issue over in #54165 to capture the discussion so far, and will have a deeper look. Thanks again for all your help with this one! I'll merge this in now. |
This is such a good improvement. Thank you all for working on this. |
What?
Fixes #48462
Related issues and discussions:
This is an attempt to solve the issue of users being unable to deselect blocks from within the list view. The idea proposed is to allow the Escape key to deselect all blocks if there is a block selection, and skip closing the list view. If there are no blocks selected, then the Escape key will close the list view.
Why?
As reported in #48462, it would be good for users to be able to deselect blocks from within the list view via keyboard, and it is not currently possible to do so. Because folks can use the list view keyboard shortcut to open and close it, the idea in this PR is that it would be okay for Escape to be prioritised for deselecting blocks rather than closing the list view.
For reference, the keyboard shortcut for closing the list view is alt+shift+o on Windows, and control+option+o on Mac.
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
In the below screengrab, the user shift-clicks multiple blocks within the list view, then presses Escape to clear the selection. When there are no blocks selected and the user presses Escape, the list view is closed.
escape-to-clear-selection.mp4