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.

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

[MenuItem] submenu tabbing support! #2669

Closed
wants to merge 6 commits into from
Closed

[MenuItem] submenu tabbing support! #2669

wants to merge 6 commits into from

Conversation

giladgray
Copy link
Contributor

@giladgray giladgray commented Jul 10, 2018

Changes proposed in this pull request:

  • add MenuItem tabIndex prop, so all menu items (not just those with hrefs) can be tabbed to!
  • ⚠️ disable portals for popovers so all submenu items are nicely ordered in a container for tabbing
    • this is slightly dangerous as folks may rely on the portal behavior, but it's oh so much better.
  • move Popover onBlur handler to the wrapper so it captures focus leaving the entire thing
    • this include content with usePortal={false} so a submenu will close when tabbing past its last item!
  • fix submenu styles

Gilad Gray added 5 commits July 10, 2018 15:19
inline popover makes for nice easy tab ordering.
move Popover onBlur so it captures focus leaving wrapper
@blueprint-bot
Copy link

support dark portal menus

Preview: documentation | landing | table

disabled={disabled}
enforceFocus={false}
hoverCloseDelay={0}
interactionKind={PopoverInteractionKind.HOVER}
modifiers={SUBMENU_POPOVER_MODIFIERS}
position={Position.RIGHT_TOP}
usePortal={false}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a pretty significant difference. You can still do this if you adequately document the change.

@giladgray
Copy link
Contributor Author

closing in favor of #2676 to just fix the bug. will revisit later.

@giladgray giladgray closed this Jul 11, 2018
@giladgray giladgray deleted the gg/submeus branch October 14, 2018 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants