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>
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.