Expose moveFocus() #282
Expose moveFocus() #282
Conversation
|
It looks like a few steps were missed from the contributing guide. |
jaksenko
force-pushed
the
patch-1
branch
4 times, most recently
from
747ab85
to
25ae89c
Compare
This makes the library ready for menus that open on hover.
This is a great library that handles keyboard users and accessibility perfectly, but only when your submenu opens on click. In our company the menu must open on hover, which can be achieved simply by attaching mouse events to the element:
```
<li className={styles.menuItem} onMouseEnter={() => setIsOpen(true)} onMouseLeave={() => setIsOpen(false)}>
```
But once menu is open, hovering over items won't change focus element. This again can be done by attaching mouse event to submenu item:
```
onMouseEnter={(elem) => elem.target.focus()}
```
However, the internal state of this library won't change so once you switch back to keyboard and hit up/down arrow, an unexpected element is focused.
With this change, one can use `moveFocus` to change also internal state:
```
onMouseEnter={() => moveFocus(index)} // index from `.map(menuItem, index)`
```
so switching between keyboard and mouse is seamless.
Ah right! I hope I didn't miss anything now. |
Codecov Report
@@ Coverage Diff @@
## master #282 +/- ##
=======================================
Coverage 90.72% 90.72%
=======================================
Files 1 1
Lines 97 97
Branches 31 31
=======================================
Hits 88 88
Misses 9 9
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
I've left a handful of comments with suggested changes, most of them have to do with how we'll document the return value differently going forward (#283).
In addition to that, it looks like the "Lint code" job is failing. Here's how to address that: https://github.com/sparksuite/react-accessible-dropdown-menu-hook/blob/master/CONTRIBUTING.md#7-format-the-code.
Finally, it looks like the "Return object" page wasn't updated in the docs, but should be, since this PR introduces a new item in the return object.
Co-authored-by: Wes Cossick <WesCossick@users.noreply.github.com>
|
Thank you for your thorough review. Hopefully I didn't forget anything this time. |
There was a problem hiding this comment.
LGTM and all the CI checks passed. @corymharper do you have any thoughts on this before I merge it?
The implementation is fairly straightforward and I don't see any eye glaring problems with it, probably that the tests for this new function and the verbiage in the documentation could be polished, but that can be done in a separated PR. |
This makes the library ready for menus that open on hover.
This is a great library that handles keyboard users and accessibility perfectly, but only when your submenu opens on click. In our company the menu must open on hover, which can be achieved simply by attaching mouse events to the element:
But once menu is open, hovering over items won't change focus element. This again can be done by attaching mouse event to submenu item:
However, the internal state of this library won't change so once you switch back to keyboard and hit up/down arrow, an unexpected element is focused.
With this change, one can use
moveFocusto change also internal state:so switching between keyboard and mouse is seamless.