downshift icon indicating copy to clipboard operation
downshift copied to clipboard

Keyboard navigation breaks when the toggle button acquires focus while the menu is open.

Open jmurzy opened this issue 3 years ago • 0 comments

  • downshift version: 6.1.9

Relevant code or config

Unmodified basic usage of useSelect:

https://codesandbox.io/s/github/kentcdodds/downshift-examples?file=/src/hooks/useSelect/basic-usage.js

What you did:

Using the basic useSelect example.

What happened:

Up and down arrows only work when if the menu is focused.

Reproduction repository:

  • Open the basic usage example of useSelect linked above.
  • Click the toggle button to open the menu. Menu will acquire focus. Navigation works as expected.
  • While the menu is open, with a pointer device, press !but not release! the toggle button, then release somewhere outside the toggle button. (Or, move your pointer outside the toggle button while pressed, then hit Esc)
  • The toggle button will acquire focus. However, since the click event was never triggered, the state is still "open".
  • Now you can no longer use the "up" and "down" keys to navigate the menu past the first or the last element.

Problem description:

Since the focus was acquired by the toggle due to the keydown event, keyboard navigation within the menu is no longer possible. Please note that this issue applies to all existing downshift hooks!

Suggested solution:

Currently, the focus is moved to the menu element declaratively in an effect when the state changes. This works in general but fails when the toggle button potentially acquires focus when the menu is open as demonstrated above.

To address the issue, focus should also be moved, perhaps imperatively, to the menu when "up" or "down" are pressed on the toggle button.

This solution would also address issues commonly related to lazy rendering of the menu element by an animation component or a portal node. When the menu is rendered lazily in such cases, menuRef is not available immediately when isOpen changes to true internally. For that reason, focus() is not yet possible due to the lazy nature of the menu element.

jmurzy avatar Aug 31 '22 00:08 jmurzy