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

Close Menu component when using tab key #1673

Merged
merged 4 commits into from
Jul 14, 2022
Merged

Close Menu component when using tab key #1673

merged 4 commits into from
Jul 14, 2022

Commits on Jul 14, 2022

  1. menu should not trap focus for tab key

    @eriese @RobinMalfait
    eriese authored and RobinMalfait committed Jul 14, 2022
    Configuration menu
    Copy the full SHA
    4e648e0 View commit details
    Browse the repository at this point in the history

  2. introduce focusFrom focus management utility 

    This is internal API, and the actual API is not 100% ideal. I started
refactoring this in a separate branch but it got out of hand and touches
a bit more pieces of the codebase that aren't related to this PR at all.

The idea of this function is just so that we can go Next/Previous but
from the given element not from the document.activeElement. This is
important for this feature. We also bolted this ontop of the existing
code which now means that we have this API:

```js
focusIn([], Focus.Previouw, true, DOMNode)
```

Luckily it's internal API only!
    @RobinMalfait
    RobinMalfait committed Jul 14, 2022
    Configuration menu
    Copy the full SHA
    d519b9b View commit details
    Browse the repository at this point in the history

  3. ensure closing via Tab works as expected 

    Just closing the Menu isn't 100% enough. If we do this, it means that
when the Menu is open, we press shift+tab, then we go to the
Menu.Button because the Menu.Items were the active element.

The other way is also incorrect because it can happen if you have an
`<a>` element as one of the Menu.Item elements then that `<a>` will
receive focus, then the `Menu` will close unmounting the focused `<a>`
and now that element is gone resulting in `document.body` being the
active element.

To fix this, we will make sure that we consider the `Menu` as 1 coherent
component. This means that using `<Tab>` will now go to the next element
after the `<Menu.Button>` once the Menu is closed.

Shift+Tab will go to the element before the `<Menu.Button>` even though
you are currently focused on the `Menu.Items` so depending on the timing
you go to the `Menu.Button` or not.

Considering the Menu as a single component it makes more sense use the
elements before / after the `Menu`
    @RobinMalfait
    RobinMalfait committed Jul 14, 2022
    Configuration menu
    Copy the full SHA
    15665a5 View commit details
    Browse the repository at this point in the history

  4. update changelog

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