Skip to content
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

Writing flow: Fix focus trap on non-text input types #32714

Merged
merged 2 commits into from
Sep 16, 2021
Merged

Writing flow: Fix focus trap on non-text input types #32714

merged 2 commits into from
Sep 16, 2021

Conversation

mirka
Copy link
Member

@mirka mirka commented Jun 15, 2021

Fixes #32712

Description

Some existing logic to handle keydown events on input elements does not completely account for the fact that a lot of input types do not support selection ranges, which means we can't always assume they are "text fields".

This PR addresses the most serious problem caused by this naive handling, which is a focus trap on non-text inputs.

The downside to the simple fix in this PR is that up/down arrow keydowns will no longer trigger the native behavior for input types like number. I think this is a reasonable trade-off though, since a keyboard user has alternative ways to modify a number input but no way out of the focus trap. Smarter handling of various input types will require a more complex system, since they each react to arrow keys in different ways.

The rest of the user-facing issues caused by naive input handling seem to be less serious, and merely inconveniences, e.g. #32713 and #32715.

How has this been tested?

  • Unit tests pass.
  • The repro steps in the original issues are fixed (using non-text input types like color and file).

Screenshots

Down arrow navigates through a checkbox

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@mirka mirka requested a review from ellatrix as a code owner June 15, 2021 20:29
@youknowriad
Copy link
Contributor

This makes sense to me but I'd appreciate a confirmation from @ellatrix

@annezazu annezazu added [a11y] Keyboard & Focus [Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... [Type] Bug An existing feature does not function as intended labels Aug 25, 2021
@mirka
Copy link
Member Author

mirka commented Sep 16, 2021

Sorry for the re-ping @ellatrix — would you have any objections to this? I'd really love get this focus trap fixed 🙂

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Let's get this in. We can always follow-up if anything comes up.

@youknowriad youknowriad merged commit 760f4d2 into WordPress:trunk Sep 16, 2021
@github-actions github-actions bot added this to the Gutenberg 11.6 milestone Sep 16, 2021
@mirka mirka deleted the fix/input-focus-trap branch September 16, 2021 10:17
@priethor priethor added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Writing flow: Arrow key navigation gets stuck on non-text inputs
4 participants