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

fix: focus issue in menu #17047

Conversation

preetibansalui
Copy link
Contributor

Closes #16994

Fix the inconsistency in menu where it has more than 2 menu items with submenus.

Changed

The issue was causing because of delay in closing the submenu.

Added floating ui - safePolygon so menu will not close when user hover on submenu.
Added floating ui - FloatingFocusManager to manage the focus.

Copy link

netlify bot commented Jul 26, 2024

Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 601f081
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-react/deploys/66a36483d56372000831cb05
😎 Deploy Preview https://deploy-preview-17047--v11-carbon-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jul 26, 2024

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 601f081
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/66a36483b681260008637ce7
😎 Deploy Preview https://deploy-preview-17047--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jul 26, 2024

Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 7ce1a0c
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-react/deploys/66b65b488d180a0008760114
😎 Deploy Preview https://deploy-preview-17047--v11-carbon-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jul 26, 2024

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 7ce1a0c
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/66b65b48e3de590008e3ec9f
😎 Deploy Preview https://deploy-preview-17047--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@preetibansalui preetibansalui marked this pull request as ready for review July 26, 2024 14:03
@preetibansalui preetibansalui requested a review from a team as a code owner July 26, 2024 14:03
Copy link
Contributor

@guidari guidari left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

Copy link
Member

@tay1orjones tay1orjones left a comment

Choose a reason for hiding this comment

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

This is such an excellent improvement! Just a couple things I noticed

Comment on lines 122 to 124
handleClose: safePolygon({
requireIntent: true,
}),
Copy link
Member

Choose a reason for hiding this comment

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

Here I'm trying to get to Italic, but can't get there even though I'm in the triangle. The Share with item is being invoked when I expect it to be ignored. Maybe the blockPointerEvents option could help?

2024-07-29.at.11.06.45-MAIN-Google.Chrome-converted.mp4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

blockPointerEvents was blocking the entire menu from any event. its happening only when any other pointer comes under the safePolygon. If you try it withShare with It works fine. So I added a delay of 100ms to avoid the issue.

packages/react/src/components/Menu/MenuItem.tsx Outdated Show resolved Hide resolved
@tay1orjones tay1orjones added this pull request to the merge queue Aug 8, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 8, 2024
@preetibansalui preetibansalui added this pull request to the merge queue Aug 9, 2024
@preetibansalui preetibansalui removed this pull request from the merge queue due to a manual request Aug 9, 2024
@kennylam kennylam enabled auto-merge August 9, 2024 17:58
@kennylam kennylam added this pull request to the merge queue Aug 9, 2024
Merged via the queue into carbon-design-system:main with commit 0c456e9 Aug 9, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Inconsistent focus for menu if more than one menu item have nested menu items.
4 participants