diff --git a/packages/react/src/ActionMenu/ActionMenu.docs.json b/packages/react/src/ActionMenu/ActionMenu.docs.json index 23a8ed07e52..0ebdd44fbe6 100644 --- a/packages/react/src/ActionMenu/ActionMenu.docs.json +++ b/packages/react/src/ActionMenu/ActionMenu.docs.json @@ -71,6 +71,14 @@ "url": "/react/Button" } }, + { + "name": "ActionMenu.IconButton", + "props": [], + "passthrough": { + "element": "IconButton", + "url": "/react/IconButton" + } + }, { "name": "ActionMenu.Anchor", "props": [ 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..296d293f430 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,8 @@ 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' +import {useMergedRefs} from '../internal/hooks/useMergedRefs' export type MenuCloseHandler = ( gesture: 'anchor-click' | 'click-outside' | 'escape' | 'tab' | 'item-select' | 'arrow-left' | 'close', @@ -106,7 +108,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 +129,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 +168,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 +199,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 +229,21 @@ const Anchor: WithSlotMarker< > > = React.forwardRef(({children: child, ...anchorProps}, anchorRef) => { const {onOpen, isSubmenu} = React.useContext(MenuContext) + const internalRef = useRef(null) + const mergedRef = useMergedRefs(internalRef, anchorRef) + + useEffect(() => { + if (!__DEV__) return + // 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) + warning( + !isAnchorInteractive, + 'The `ActionMenu.Anchor` expects a single interactive element that can act as a menu trigger. Consider using a `