Skip to content

Conversation

@heloiselui
Copy link
Contributor

Closes #21159
Closes #20349

This PR fixes cds-menu-button/cds-menu back in line with React so menus close when they should and stay on top of nearby content.

Changelog

New

  • None

Changed

  • All trigger clicks, blurs, and Escape presses now flow through _closeMenu, so the same focus-restoration rules apply for every close path.
  • cds-menu listens for ${prefix}-menu-close-root-request from menu items, resets its placement when open flips, and passes the original trigger event through ${prefix}-menu-closed.
  • Menu items dispatch the new close request when activated, matching React
  • Raised the menu host to Carbon’s modal z-index so the overlay doesn’t hide behind nearby content.
  • Update snap

Removed

  • Dropped the dead @query(${prefix}-menu) since the menu lives in the light DOM slot.

Testing / Reviewing

  • Go to WC Deploy Preview > Menu Button
  • Open and close the menu button should open/close.
  • Press Escape menu should closes and focuses the trigger.
  • Scroll while open and confirm the menu stays aligned in the overview page.

PR Checklist

As the author of this PR, before marking ready for review, confirm you:

  • Reviewed every line of the diff
  • [ ] Updated documentation and storybook examples
  • [ ] Wrote passing tests that cover this change
  • [ ] Addressed any impact on accessibility (a11y)
  • Tested for cross-browser consistency
  • Validated that this code is ready for review and status checks should pass

More details can be found in the pull request guide

@heloiselui heloiselui requested a review from a team as a code owner December 15, 2025 20:48
@netlify
Copy link

netlify bot commented Dec 15, 2025

Deploy Preview for v11-carbon-react ready!

Name Link
🔨 Latest commit c5b74b7
🔍 Latest deploy log https://app.netlify.com/projects/v11-carbon-react/deploys/69416165d2e4a300084885a4
😎 Deploy Preview https://deploy-preview-21184--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 project configuration.

@netlify
Copy link

netlify bot commented Dec 15, 2025

Deploy Preview for v11-carbon-web-components ready!

Name Link
🔨 Latest commit c5b74b7
🔍 Latest deploy log https://app.netlify.com/projects/v11-carbon-web-components/deploys/69416165c593c200087c8529
😎 Deploy Preview https://deploy-preview-21184--v11-carbon-web-components.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 project configuration.

@netlify
Copy link

netlify bot commented Dec 15, 2025

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit c5b74b7
🔍 Latest deploy log https://app.netlify.com/projects/carbon-elements/deploys/69416165c2c5250008395392
😎 Deploy Preview https://deploy-preview-21184--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 project configuration.

@codecov
Copy link

codecov bot commented Dec 15, 2025

Codecov Report

❌ Patch coverage is 71.94245% with 39 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.08%. Comparing base (6c5d4d0) to head (c5b74b7).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...mponents/src/components/menu-button/menu-button.ts 59.30% 34 Missing and 1 partial ⚠️
...es/web-components/src/components/menu/menu-item.ts 87.50% 2 Missing ⚠️
...ackages/web-components/src/components/menu/menu.ts 94.59% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #21184      +/-   ##
==========================================
- Coverage   85.17%   85.08%   -0.10%     
==========================================
  Files         530      530              
  Lines       40889    41015     +126     
  Branches     6280     6350      +70     
==========================================
+ Hits        34829    34898      +69     
- Misses       5900     5955      +55     
- Partials      160      162       +2     
Flag Coverage Δ
main-packages 85.72% <ø> (ø)
web-components 84.72% <71.94%> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@maradwan26 maradwan26 left a comment

Choose a reason for hiding this comment

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

LGTM!

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 Button / Menu does not close . Also moves one scroll. [Bug]: Menu overlay positioning issue - content appears behind other elements

2 participants