Match focus order to visual order#827
Conversation
|
@t-hamano Thanks for your great work. |
|
Thanks for the review! I should have run the unit tests locally before submitting the PR 😅 The unit test was failing because splitting the resizer increased the number of divs by one: Before
After
Therefore, it seems the test fails because the number of actual The increase in the number of divs was intentional, so I fixed the test itself. For the same reason, the second Please let me know if I've missed anything. |
|
@t-hamano Looks great!! thanks for your contribution :) |
|
6.10.0 published |
Proposed solution
Thank you for providing an amazing library.
I'm a member of the Gutenberg project (part of WordPress) and provide our own
ResizableBoxcomponent that wraps this library.Our own component supports the same props as the library, and additionally renders a focusable button element via the
handleComponentprop to allow the user to focus the resizer and resize it via keyboard events (Example).I noticed that because the
Resizercomponents are always rendered together after itschildren, sometimes the focus order doesn't match the visual order.This PR splits the
renderResizer()into before and after thechildren, which should make the focus order and visual order match.Testing Done
I've created a temporary story to test this PR, and if it makes sense I'd like to delete it after you review it.
The URL for the storybook is http://localhost:6066/?path=/story/handle--multiple.
The video below shows how pressing the tab key moves focus when children contains focusable elements:
Before
before.mp4
After
after.mp4