-
Notifications
You must be signed in to change notification settings - Fork 327
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.
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
Menu enhancements #502
Menu enhancements #502
Conversation
|
||
const NAVIGATION_KEYS = [...ARROW_UP_KEYS, ...ARROW_RIGHT_KEYS, ...ARROW_DOWN_KEYS, ...ARROW_LEFT_KEYS]; | ||
|
||
export const useLastNavigationDirection = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be used in the new MenuGridItem
, to detect the direction of menu navigation.
Basically I can use it in more existing places and save some code and complexity - maybe I'll do it when I have some time
@@ -226,6 +232,7 @@ const MenuItem = forwardRef( | |||
aria-current={isActive} | |||
onMouseLeave={onMouseLeave} | |||
onMouseEnter={onMouseEnter} | |||
tabIndex={-1} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are many manipulation of focus/blur, it's much easier when each menu item is focusable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very small suggestion - -1
kinda feels like a "magic" number, maybe assign it to a constant with a meaningful name?
…to feature/omri/menu-enhancements
@@ -226,6 +232,7 @@ const MenuItem = forwardRef( | |||
aria-current={isActive} | |||
onMouseLeave={onMouseLeave} | |||
onMouseEnter={onMouseEnter} | |||
tabIndex={-1} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very small suggestion - -1
kinda feels like a "magic" number, maybe assign it to a constant with a meaningful name?
@sahariko applied your suggestion 😄 |
Task
Small improvements and additions around the
Menu
components. I split it to avoid a larger PR for the incomingMenuGridItem
.