Skip to content

Allow nested sub-menus in the ActionMenu component #3564

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

camertron
Copy link
Collaborator

What are you trying to accomplish?

This PR adds multi-level support to the ActionMenu component. It enables menus to be nested inside other menus just as can be done for the corresponding React component.

Since there was no reason to build it, and because it's not clear how it should work, ActionMenus with sub-menus do not support single-select mode.

Screenshots

Alt: A screen recording initially showing only a button labeled "Edit." When the button is clicked, an ActionMenu appears containing three items: "Cut," "Copy," and "Paste special." The last item's label is followed by a right-facing chevron icon. This last item is clicked, which causes a sub-menu to appear to the right. This sub-menu contains four items, the last of which also features a right-facing chevron icon. When the last item is clicked, a third sub-menu appears containing 3 additional items. The last item is clicked and all three menus disappear.

action_menu.mov

Integration

Multi-level support has been designed to maintain feature parity with the current ActionMenu component so as to be entirely backwards-compatible, at least from an API perspective. The component should generate nearly identical markup with the minor exception of the newly added sub-menu item, which includes a nested <ul> and <anchored-position>.

However, care should be taken when integrating these changes, since they do represent a significant re-architecting of the component's inner workings.

Risk Assessment

  • Low risk the change is small, highly observable, and easily rolled back.
  • Medium risk changes that are isolated, reduced in scope or could impact few users. The change will not impact library availability.
  • High risk changes are those that could impact customers and SLOs, low or no test coverage, low observability, or slow to rollback.

What approach did you choose and why?

This feature was built by introducing a new menu item class called SubMenuItem. This class represents the menu item element and renders a SubMenu component that essentially contains another ActionMenu wrapped in its own <anchored-position>. To share code between the primary menu and sub-menus, the original code from the ActionMenu class was moved into a base class named Menu, from which both PrimaryMenu and SubMenu now inherit.

Accessibility

  • No new axe scan violation - This change does not introduce any new axe scan violations.

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Added/updated previews (Lookbook)
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

Copy link

changeset-bot bot commented Jun 23, 2025

🦋 Changeset detected

Latest commit: 8210923

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/view-components Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@camertron camertron marked this pull request as ready for review June 23, 2025 05:29
@camertron camertron requested a review from a team as a code owner June 23, 2025 05:29
@camertron camertron requested a review from hectahertz June 23, 2025 05:29
@lesliecdubs
Copy link
Member

Thanks for the contribution, @camertron! @hectahertz this one has been open for two weeks, can you please try to prioritize a review this week? Thanks!

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.

3 participants