Add ActionMenu.IconButton and ActionMenu.Anchor child validation#7640
Add ActionMenu.IconButton and ActionMenu.Anchor child validation#7640
Conversation
|
- Add ActionMenu.IconButton as a shortcut companion to ActionMenu.Button that wraps IconButton and wires it correctly as an ActionMenu anchor - Add validation in ActionMenu.Anchor to warn when the child is a non-interactive element (skips validation for submenu anchors) - Add tests for both features - Add type tests for ActionMenu.IconButton - Update exports in index.ts Co-authored-by: siddharthkp <1863771+siddharthkp@users.noreply.github.com>
Co-authored-by: siddharthkp <1863771+siddharthkp@users.noreply.github.com>
|
There was a problem hiding this comment.
Pull request overview
Adds a first-class ActionMenu.IconButton trigger and introduces dev-time validation for ActionMenu.Anchor children to reduce accidentally inaccessible menu triggers in @primer/react.
Changes:
- Add
ActionMenu.IconButtonsubcomponent (andActionMenuIconButtonPropsexport) as a shortcut for usingIconButtonas the menu trigger. - Add dev-time
ActionMenu.Anchorchild validation that warns when the anchor element is not interactive (skipping submenu contexts). - Update docs/exports and add unit + type tests covering the new API and warning behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/index.ts | Exports the new ActionMenuIconButtonProps type from the package entrypoint. |
| packages/react/src/tests/snapshots/exports.test.ts.snap | Updates the public-exports snapshot to include ActionMenuIconButtonProps. |
| packages/react/src/ActionMenu/ActionMenu.types.test.tsx | Adds type-level coverage for ActionMenu.IconButton props (valid/invalid). |
| packages/react/src/ActionMenu/ActionMenu.tsx | Implements ActionMenu.IconButton and adds ActionMenu.Anchor interactive-child validation + merged refs. |
| packages/react/src/ActionMenu/ActionMenu.test.tsx | Adds unit tests for ActionMenu.IconButton behavior and ActionMenu.Anchor warning behavior. |
| packages/react/src/ActionMenu/ActionMenu.docs.json | Documents ActionMenu.IconButton as a passthrough to IconButton. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| ref: (node: HTMLElement | null) => { | ||
| internalRef.current = node | ||
| if (typeof anchorRef === 'function') { | ||
| anchorRef(node) | ||
| } else if (anchorRef) { | ||
| ;(anchorRef as React.MutableRefObject<HTMLElement | null>).current = node | ||
| } |
There was a problem hiding this comment.
The anchor ref wiring is manually re-implementing ref merging and requires unsafe casts. Consider using the existing useMergedRefs helper (packages/react/src/internal/hooks/useMergedRefs.ts) to combine anchorRef and internalRef instead of an inline callback + casts; this also lets you drop the react-hooks/refs eslint disable.
There was a problem hiding this comment.
Replaced manual ref merging with useMergedRefs(internalRef, anchorRef) in d6d9c33. This removes the inline callback, the unsafe MutableRefObject cast, and the eslint-disable comment.
Co-authored-by: siddharthkp <1863771+siddharthkp@users.noreply.github.com>
|
Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days. |
ActionMenu.Anchoraccepts arbitrary children, making it easy to wrap non-interactive elements (e.g.,<div>) and produce inaccessible markup. The most common use case forActionMenu.Anchoris wrappingIconButton, which should have a blessed shortcut.This PR adds two things:
ActionMenu.IconButton— syntactic sugar (likeActionMenu.Button) that correctly wires anIconButtonas the menu anchorActionMenu.Anchorchild validation — dev-only warning when the anchor child is not an interactive element, following the same pattern used inTooltipThe validation warns on non-interactive anchors but skips submenu contexts where
ActionList.Itemis the expected anchor.Changelog
New
ActionMenu.IconButton— shortcut for usingIconButtonas menu trigger, equivalent to wrapping it inActionMenu.AnchorActionMenuIconButtonPropstype exportconsole.warnwhenActionMenu.Anchorreceives a non-interactive child elementChanged
ActionMenu.Anchornow usesuseMergedRefsto combine an internal ref (for child validation) with the forwarded refRemoved
None
Rollout strategy
Testing & Reviewing
ActionMenu.IconButton(open/close, event handlers, id forwarding)<div>) and does not fire forButtonorIconButtonActionList.ItemanchorsActionMenu.IconButtonprop typesMerge checklist
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.