Writing flow: Fix focus trap on certain input types #41538
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mirka
added
the
[Feature] Writing Flow
|
Size Change: +409 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
|
@mirka Possible for you to add one E2E test showing this behavior? It would be nice so I can follow the instructions in the test on how to recreate the steps here. It seems like some blocks currently do not have this problem such as the image block. Once you focus it, you can move between the buttons with Left/Right Arrow keys I believe. Unrelated to this PR, if you are brave enough to touch the writing flow code, any of this look interesting? I have a lot of good ideas on how to improve A11Y but some of them are above my current React abilities. Thanks! |
alexstine
added
[Focus] Accessibility (a11y)
Here's a branch (
Does that work? I thought a testing branch would be easier than reading an E2E, but let me know if that's not the case 😄
Correct, that's because the buttons in the Image block are |
They're certainly interesting and worthwhile to me, but I have a bit too much on my plate right now to take on a new focus area 😭 |
alexstine
approved these changes
Jun 7, 2022
There was a problem hiding this comment.
@mirka The PR looks good to me and does exactly what you say it should do. I tried to find any accessibility issues and could not.
I think it may be best to get at least one more sign off on this code considering how easy it is to break writing flow. I'd hate to have missed something which would come back to haunt us in the future.
|
This is fine with me. Note that generally editor UI (so most input elements in a block) should go in a placeholder, which will in the future be outside of the writing flow logic. Only input that are previews (and thus editable) should go inside blocks without wrapping it in a placeholder. |
|
Thanks for the reviews everyone!
That makes sense. It's probably a relatively rare use case, hence not being a problem for the vast majority. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #32715
What?
Fixes an issue where keyboard focus could get stuck on certain types of
<input>elements when using the left/right arrow keys to navigate interactive elements within a block.Why?
There is no need to prevent horizontal arrow nav on
inputelement types that do not require horizontal arrow nav internally, such ascheckboxandradio.How?
I picked out an allowlist of input types (from MDN) that are safe to be navigation candidates.
Testing Instructions
Add a
<input type="checkbox" />to the edit component of a core block, such asCode. If necessary, flexbox the container so the buttons are horizontally aligned.Example modification of the Code block:
Move keyboard focus to the first
<input>, and hit the right arrow key.Screenshots or screencast
CleanShot.2022-06-03.at.23.01.11.mp4