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.

Sign up for GitHub

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

Jump to bottom

Fix FocusTrap escape due to strange tabindex values #2093

Merged
merged 4 commits into from
Dec 14, 2022
Merged

Fix FocusTrap escape due to strange tabindex values #2093

merged 4 commits into from
Dec 14, 2022

Commits on Dec 14, 2022

  1. sort DOM nodes using tabIndex first 

    It will still keep the same DOM order if tabIndex matches, thanks to
stable sorts!
    @RobinMalfait
    RobinMalfait committed Dec 14, 2022
    Configuration menu
    Copy the full SHA
    53837c2 View commit details
    Browse the repository at this point in the history

  2. refactor focusIn API 

    All the arguments resulted in usage like `focusIn(container,
Focus.First, true, null)`, and to make things worse, we need to add
something else to this list in the future.

Instead, let's keep the `container` and the type of `Focus` as known
params, all the other things can sit in an options object.
    @RobinMalfait
    RobinMalfait committed Dec 14, 2022
    Configuration menu
    Copy the full SHA
    c484ba9 View commit details
    Browse the repository at this point in the history

  3. fix FocusTrap escape due to strange tabindex values 

    This code will now ensure that we can't escape the FocusTrap if you use
`<tab>` and you happen to tab to an element outside of the FocusTrap
because the next item in line happens to be outside of the FocusTrap and
we never hit any of the focus guard elements.

How it works is as follows:

1. The `onBlur` is implemented on the `FocusTrap` itself, this will give
   us some information in the event itself.
   - `e.target` is the element that is being blurred (think of it as `from`)
   - `e.currentTarget` is the element with the event listener (the dialog)
   - `e.relatedTarget` is the element we are going to (think of it as `to`)
2. If the blur happened due to a `<tab>` or `<shift>+<tab>`, then we
   will move focus back inside the FocusTrap, and go from the `e.target`
   to the next or previous value.
3. If the blur happened programmatically (so no tab keys are involved,
   aka no direction is known), then the focus is restored to the
   `e.target` value.

Fixes: #1656
    @RobinMalfait
    RobinMalfait committed Dec 14, 2022
    Configuration menu
    Copy the full SHA
    2d920eb View commit details
    Browse the repository at this point in the history

  4. update changelog

    @RobinMalfait
    RobinMalfait committed Dec 14, 2022
    Configuration menu
    Copy the full SHA
    af3def4 View commit details
    Browse the repository at this point in the history