From c732051b328c9837b1b76dcb50a87aa8d713f9bf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 9 Mar 2026 10:53:36 +0000 Subject: [PATCH 1/6] Initial plan From af9631546b66f824593276996ff60a8130ca6f5a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 9 Mar 2026 11:09:21 +0000 Subject: [PATCH 2/6] Add ActionMenu.IconButton and ActionMenu.Anchor validation - 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> --- .../react/src/ActionMenu/ActionMenu.test.tsx | 166 ++++++++++++++++++ packages/react/src/ActionMenu/ActionMenu.tsx | 95 +++++++++- .../src/ActionMenu/ActionMenu.types.test.tsx | 14 ++ packages/react/src/index.ts | 7 +- 4 files changed, 273 insertions(+), 9 deletions(-) diff --git a/packages/react/src/ActionMenu/ActionMenu.test.tsx b/packages/react/src/ActionMenu/ActionMenu.test.tsx index f7e74c3db77..7a5c6c2a7b9 100644 --- a/packages/react/src/ActionMenu/ActionMenu.test.tsx +++ b/packages/react/src/ActionMenu/ActionMenu.test.tsx @@ -837,6 +837,172 @@ describe('ActionMenu', () => { }) }) + describe('ActionMenu.IconButton', () => { + it('should open Menu on ActionMenu.IconButton click', async () => { + const component = HTMLRender( + + + + + New file + Copy link + + + , + ) + + const user = userEvent.setup() + const button = component.getByRole('button', {name: 'Open menu'}) + await user.click(button) + + expect(component.getByRole('menu')).toBeInTheDocument() + }) + + it('should close Menu on selecting an action when using ActionMenu.IconButton', async () => { + const component = HTMLRender( + + + + + New file + Copy link + + + , + ) + + const user = userEvent.setup() + const button = component.getByRole('button', {name: 'Open menu'}) + await user.click(button) + + const menuItems = component.getAllByRole('menuitem') + await user.click(menuItems[0]) + + expect(component.queryByRole('menu')).toBeNull() + }) + + it('should call onClick and onKeyDown passed to ActionMenu.IconButton', async () => { + const mockOnClick = vi.fn() + const mockOnKeyDown = vi.fn() + + const component = HTMLRender( + + + + + New file + Copy link + + + , + ) + + const user = userEvent.setup() + const button = component.getByRole('button') + await user.click(button) + + expect(component.getByRole('menu')).toBeInTheDocument() + expect(mockOnClick).toHaveBeenCalledTimes(1) + + // select and close menu + const menuItems = component.getAllByRole('menuitem') + await user.click(menuItems[0]) + expect(component.queryByRole('menu')).toBeNull() + + expect(button).toEqual(document.activeElement) + await user.keyboard('{Enter}') + expect(component.queryByRole('menu')).toBeInTheDocument() + expect(mockOnKeyDown).toHaveBeenCalledTimes(1) + }) + + it('should pass the "id" prop from ActionMenu.IconButton to the HTML button', async () => { + const buttonId = 'icon-button-custom-id' + const component = HTMLRender( + + + + + New file + + + , + ) + const button = component.getByRole('button') + + expect(button.id).toBe(buttonId) + }) + }) + + describe('ActionMenu.Anchor validation', () => { + it('should warn when ActionMenu.Anchor receives a non-interactive child', async () => { + const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + HTMLRender( + + +
Not a button
+ + + + New file + + + , + ) + + expect(consoleSpy).toHaveBeenCalledWith('Warning:', expect.stringContaining('ActionMenu.Anchor')) + + consoleSpy.mockRestore() + }) + + it('should not warn when ActionMenu.Anchor receives a Button child', async () => { + const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + HTMLRender( + + + + + + + New file + + + , + ) + + expect(consoleSpy).not.toHaveBeenCalled() + + consoleSpy.mockRestore() + }) + + it('should not warn when ActionMenu.Anchor receives an IconButton child', async () => { + const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + HTMLRender( + + + + + + + New file + + + , + ) + + expect(consoleSpy).not.toHaveBeenCalled() + + consoleSpy.mockRestore() + }) + }) + describe('feature flag: primer_react_action_menu_display_in_viewport_inside_dialog', () => { const mockGetAnchoredPosition = vi.mocked(getAnchoredPosition) diff --git a/packages/react/src/ActionMenu/ActionMenu.tsx b/packages/react/src/ActionMenu/ActionMenu.tsx index e501b117377..4f1544f9dd2 100644 --- a/packages/react/src/ActionMenu/ActionMenu.tsx +++ b/packages/react/src/ActionMenu/ActionMenu.tsx @@ -1,4 +1,4 @@ -import React, {useCallback, useContext, useMemo, useEffect, useState} from 'react' +import React, {useCallback, useContext, useMemo, useEffect, useRef, useState} from 'react' import {TriangleDownIcon, ChevronRightIcon} from '@primer/octicons-react' import type {AnchoredOverlayProps} from '../AnchoredOverlay' import {AnchoredOverlay} from '../AnchoredOverlay' @@ -6,9 +6,9 @@ import type {OverlayProps} from '../Overlay' import {useProvidedRefOrCreate, useProvidedStateOrCreate, useMenuKeyboardNavigation} from '../hooks' import {Divider} from '../ActionList/Divider' import {ActionListContainerContext} from '../ActionList/ActionListContainerContext' -import type {ButtonProps} from '../Button' +import type {ButtonProps, IconButtonProps} from '../Button' import type {AnchorPosition} from '@primer/behaviors' -import {Button} from '../Button' +import {Button, IconButton} from '../Button' import {useId} from '../hooks/useId' import type {MandateProps} from '../utils/types' import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' @@ -19,6 +19,7 @@ import {isSlot} from '../utils/is-slot' import type {FCWithSlotMarker, WithSlotMarker} from '../utils/types/Slots' import {useFeatureFlag} from '../FeatureFlags' import {DialogContext} from '../Dialog/Dialog' +import {warning} from '../utils/warning' export type MenuCloseHandler = ( gesture: 'anchor-click' | 'click-outside' | 'escape' | 'tab' | 'item-select' | 'arrow-left' | 'close', @@ -106,7 +107,12 @@ const Menu: FCWithSlotMarker> = ({ const menuButtonChild = React.Children.toArray(children).find( child => React.isValidElement(child) && - (child.type === MenuButton || child.type === Anchor || isSlot(child, Anchor) || isSlot(child, MenuButton)), + (child.type === MenuButton || + child.type === MenuIconButton || + child.type === Anchor || + isSlot(child, Anchor) || + isSlot(child, MenuButton) || + isSlot(child, MenuIconButton)), ) const menuButtonChildId = React.isValidElement(menuButtonChild) ? menuButtonChild.props.id : undefined @@ -122,7 +128,12 @@ const Menu: FCWithSlotMarker> = ({ if (child.type === Tooltip || isSlot(child, Tooltip)) { // tooltip trigger const anchorChildren = child.props.children - if (anchorChildren.type === MenuButton || isSlot(anchorChildren, MenuButton)) { + if ( + anchorChildren.type === MenuButton || + isSlot(anchorChildren, MenuButton) || + anchorChildren.type === MenuIconButton || + isSlot(anchorChildren, MenuIconButton) + ) { // eslint-disable-next-line react-hooks/immutability renderAnchor = anchorProps => { // We need to attach the anchor props to the tooltip trigger (ActionMenu.Button's grandchild) not the tooltip itself. @@ -156,7 +167,12 @@ const Menu: FCWithSlotMarker> = ({ renderAnchor = anchorProps => React.cloneElement(child, anchorProps) } return null - } else if (child.type === MenuButton || isSlot(child, MenuButton)) { + } else if ( + child.type === MenuButton || + isSlot(child, MenuButton) || + child.type === MenuIconButton || + isSlot(child, MenuIconButton) + ) { renderAnchor = anchorProps => React.cloneElement(child, mergeAnchorHandlers(anchorProps, child.props)) return null } else { @@ -182,6 +198,23 @@ const Menu: FCWithSlotMarker> = ({ ) } +// The list is from GitHub's custom-axe-rules https://github.com/github/github/blob/master/app/assets/modules/github/axe-custom-rules.ts#L3 +const interactiveElements = [ + 'a[href]', + 'button:not([disabled])', + 'summary', + 'select', + 'input:not([type=hidden])', + 'textarea', +] + +const isInteractive = (element: HTMLElement) => { + return ( + interactiveElements.some(selector => element.matches(selector)) || + (element.hasAttribute('role') && element.getAttribute('role') === 'button') + ) +} + // eslint-disable-next-line @typescript-eslint/no-explicit-any export type ActionMenuAnchorProps = {children: React.ReactElement; id?: string} & React.HTMLAttributes const Anchor: WithSlotMarker< @@ -195,6 +228,26 @@ const Anchor: WithSlotMarker< > > = React.forwardRef(({children: child, ...anchorProps}, anchorRef) => { const {onOpen, isSubmenu} = React.useContext(MenuContext) + const internalRef = useRef(null) as React.MutableRefObject + + useEffect(() => { + // Skip validation for submenu anchors, where ActionList.Item is a valid anchor + if (isSubmenu) return + if (!internalRef.current) return + const anchorElement = internalRef.current + const isAnchorInteractive = isInteractive(anchorElement) + const anchorChildren = anchorElement.childNodes + const hasInteractiveDescendant = Array.from(anchorChildren).some(child => { + return ( + (child instanceof HTMLElement && isInteractive(child)) || + Array.from(child.childNodes).some(grandChild => grandChild instanceof HTMLElement && isInteractive(grandChild)) + ) + }) + warning( + !isAnchorInteractive && !hasInteractiveDescendant, + 'The `ActionMenu.Anchor` expects a single interactive element that can act as a menu trigger. Consider using a `