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(menu): include leave intent delay for sub menus #16466

Merged

Conversation

Kilian-Collender
Copy link
Contributor

@Kilian-Collender Kilian-Collender commented May 14, 2024

Closes #16395

Menu component does not protect access to refs in focusItem by adding protection to the accessing of the refs.

Changelog

New

Adds leave intent delay for sub menu as it can accidentally trigger a premature closure of the sub menu in the following cases:

  1. the mouse moves across and down as the user selects a lower sub menu, causing the mouse to leave the menuItem.
  2. the menu has a scroll bar, as the mouse crosses this to enter the sub menu it triggers the closure before the user can access the sub menu.

Including a delay in the closure allows for the users mouse to reenter the submenu and cancel the closure of the menu.

Changed

  • The sub menu does not close immediately when the mouse leave the menu item or menu items children.
  • Add optional menu item aria-label that uses prop or label value

Testing / Reviewing

Unit tests have been added to validate the opening and closing of the sub menu using fake timers.

Manual testing can be easily done by opening the submenu and quickly moving the mouse out and back into the menu item and the menu will close.

The sub menu can accidentally trigger a premature closure of the sub menu
in the following cases:
- the mouse moves across and down as the user selects a lower sub menu,
causing the mouse to leave the menuItem.
- the menu has a scroll bar, as the mouse crosses this to enter the sub menu
 it triggers the closure before the user can access the sub menu.
Including a delay in the closure allows for the users mouse to reenter the
submenu and cancel the closure of the menu.
Copy link
Contributor

github-actions bot commented May 14, 2024

All contributors have signed the DCO.
Posted by the DCO Assistant Lite bot.

Copy link

netlify bot commented May 14, 2024

Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit fb56f16
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-react/deploys/66447c00939471000859645b
😎 Deploy Preview https://deploy-preview-16466--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.

@Kilian-Collender
Copy link
Contributor Author

I have read the DCO document and I hereby sign the DCO.

@Kilian-Collender Kilian-Collender requested a review from a team as a code owner May 14, 2024 12:25
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.

Excellent, thanks for contributing this! 🙏

Copy link
Member

@alisonjoseph alisonjoseph left a comment

Choose a reason for hiding this comment

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

LGTM!

@tay1orjones
Copy link
Member

@Kilian-Collender I can reproduce the avt error in storybook

image

auto-merge was automatically disabled May 15, 2024 09:10

Head branch was pushed to by a user without write access

@tay1orjones tay1orjones added this pull request to the merge queue May 15, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 15, 2024
@tay1orjones tay1orjones added this pull request to the merge queue May 15, 2024
Merged via the queue into carbon-design-system:main with commit 1771cb6 May 15, 2024
20 checks passed
@Kilian-Collender Kilian-Collender deleted the kc-menu-fix-16395 branch May 16, 2024 08:45
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]: Menu component does not protect access to refs in focusItem
3 participants