Add ActionMenu.IconButton and ActionMenu.Anchor child validation

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": [

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(
+        <ActionMenu>
+          <ActionMenu.IconButton icon={KebabHorizontalIcon} aria-label="Open menu" />
+          <ActionMenu.Overlay>
+            <ActionList>
+              <ActionList.Item>New file</ActionList.Item>
+              <ActionList.Item>Copy link</ActionList.Item>
+            </ActionList>
+          </ActionMenu.Overlay>
+        </ActionMenu>,
+      )
+
+      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(
+        <ActionMenu>
+          <ActionMenu.IconButton icon={KebabHorizontalIcon} aria-label="Open menu" />
+          <ActionMenu.Overlay>
+            <ActionList>
+              <ActionList.Item>New file</ActionList.Item>
+              <ActionList.Item>Copy link</ActionList.Item>
+            </ActionList>
+          </ActionMenu.Overlay>
+        </ActionMenu>,
+      )
+
+      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(
+        <ActionMenu>
+          <ActionMenu.IconButton
+            icon={KebabHorizontalIcon}
+            aria-label="Open menu"
+            onClick={mockOnClick}
+            onKeyDown={mockOnKeyDown}
+          />
+          <ActionMenu.Overlay>
+            <ActionList>
+              <ActionList.Item>New file</ActionList.Item>
+              <ActionList.Item>Copy link</ActionList.Item>
+            </ActionList>
+          </ActionMenu.Overlay>
+        </ActionMenu>,
+      )
+
+      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(
+        <ActionMenu>
+          <ActionMenu.IconButton id={buttonId} icon={KebabHorizontalIcon} aria-label="Open menu" />
+          <ActionMenu.Overlay>
+            <ActionList>
+              <ActionList.Item>New file</ActionList.Item>
+            </ActionList>
+          </ActionMenu.Overlay>
+        </ActionMenu>,
+      )
+      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(
+        <ActionMenu>
+          <ActionMenu.Anchor>
+            <div>Not a button</div>
+          </ActionMenu.Anchor>
+          <ActionMenu.Overlay>
+            <ActionList>
+              <ActionList.Item>New file</ActionList.Item>
+            </ActionList>
+          </ActionMenu.Overlay>
+        </ActionMenu>,
+      )
+
+      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(
+        <ActionMenu>
+          <ActionMenu.Anchor>
+            <Button>Toggle Menu</Button>
+          </ActionMenu.Anchor>
+          <ActionMenu.Overlay>
+            <ActionList>
+              <ActionList.Item>New file</ActionList.Item>
+            </ActionList>
+          </ActionMenu.Overlay>
+        </ActionMenu>,
+      )
+
+      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(
+        <ActionMenu>
+          <ActionMenu.Anchor>
+            <IconButton icon={KebabHorizontalIcon} aria-label="Open menu" />
+          </ActionMenu.Anchor>
+          <ActionMenu.Overlay>
+            <ActionList>
+              <ActionList.Item>New file</ActionList.Item>
+            </ActionList>
+          </ActionMenu.Overlay>
+        </ActionMenu>,
+      )
+
+      expect(consoleSpy).not.toHaveBeenCalled()
+
+      consoleSpy.mockRestore()
+    })
+  })
+
   describe('feature flag: primer_react_action_menu_display_in_viewport_inside_dialog', () => {
     const mockGetAnchoredPosition = vi.mocked(getAnchoredPosition)
 

packages/react/src/ActionMenu/ActionMenu.tsx

@@ -1,14 +1,14 @@
-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'
 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<React.PropsWithChildren<ActionMenuProps>> = ({
   const menuButtonChild = React.Children.toArray(children).find(
     child =>
       React.isValidElement<ActionMenuButtonProps>(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<React.PropsWithChildren<ActionMenuProps>> = ({
     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<React.PropsWithChildren<ActionMenuProps>> = ({
         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<React.PropsWithChildren<ActionMenuProps>> = ({
   )
 }
 
+// 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<any>; id?: string} & React.HTMLAttributes<HTMLElement>
 const Anchor: WithSlotMarker<
@@ -195,6 +229,21 @@ const Anchor: WithSlotMarker<
   >
 > = React.forwardRef<HTMLElement, ActionMenuAnchorProps>(({children: child, ...anchorProps}, anchorRef) => {
   const {onOpen, isSubmenu} = React.useContext(MenuContext)
+  const internalRef = useRef<HTMLElement | null>(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 `<button>` or an `IconButton` as the direct child of `ActionMenu.Anchor`, or use `ActionMenu.Button` or `ActionMenu.IconButton` instead.',
+    )
+  }, [isSubmenu])
 
   const openSubmenuOnRightArrow: React.KeyboardEventHandler<HTMLElement> = useCallback(
     event => {
@@ -233,7 +282,7 @@ const Anchor: WithSlotMarker<
     <ActionListContainerContext.Provider value={thisActionListContext}>
       {React.cloneElement(child, {
         ...anchorProps,
-        ref: anchorRef,
+        ref: mergedRef,
         onClick: onButtonClick,
         onKeyDown: onButtonKeyDown,
       })}
@@ -252,6 +301,17 @@ const MenuButton = React.forwardRef(({...props}, anchorRef) => {
   )
 }) as PolymorphicForwardRefComponent<'button', ActionMenuButtonProps>
 
+/** this component is syntactical sugar 🍭 */
+export type ActionMenuIconButtonProps = IconButtonProps
+
+const MenuIconButton = React.forwardRef(({...props}, anchorRef) => {
+  return (
+    <Anchor ref={anchorRef}>
+      <IconButton type="button" {...props} />
+    </Anchor>
+  )
+}) as PolymorphicForwardRefComponent<'button', ActionMenuIconButtonProps>
+
 const defaultVariant: ResponsiveValue<'anchored', 'anchored' | 'fullscreen'> = {
   regular: 'anchored',
   narrow: 'anchored',
@@ -373,7 +433,14 @@ Menu.displayName = 'ActionMenu'
 
 Menu.__SLOT__ = Symbol('ActionMenu')
 MenuButton.__SLOT__ = Symbol('ActionMenu.Button')
+MenuIconButton.__SLOT__ = Symbol('ActionMenu.IconButton')
 Anchor.__SLOT__ = Symbol('ActionMenu.Anchor')
 Overlay.__SLOT__ = Symbol('ActionMenu.Overlay')
 
-export const ActionMenu = Object.assign(Menu, {Button: MenuButton, Anchor, Overlay, Divider})
+export const ActionMenu = Object.assign(Menu, {
+  Button: MenuButton,
+  IconButton: MenuIconButton,
+  Anchor,
+  Overlay,
+  Divider,
+})

packages/react/src/ActionMenu/ActionMenu.types.test.tsx

@@ -1,4 +1,5 @@
 import {ActionMenu} from '..'
+import {SearchIcon} from '@primer/octicons-react'
 
 export function actionButtonWithoutProps() {
   return <ActionMenu.Button />
@@ -16,3 +17,16 @@ export function actionButtonWithInvalidSize() {
   //@ts-expect-error size must be one of the valid values
   return <ActionMenu.Button size="some-unknownsize">Click me!</ActionMenu.Button>
 }
+
+export function actionIconButtonWithRequiredProps() {
+  return <ActionMenu.IconButton icon={SearchIcon} aria-label="Search" />
+}
+
+export function actionIconButtonWithOptionalProps() {
+  return <ActionMenu.IconButton icon={SearchIcon} aria-label="Search" size="small" />
+}
+
+export function actionIconButtonWithInvalidSize() {
+  //@ts-expect-error size must be one of the valid values
+  return <ActionMenu.IconButton icon={SearchIcon} aria-label="Search" size="some-unknownsize" />
+}

packages/react/src/__tests__/__snapshots__/exports.test.ts.snap

@@ -16,6 +16,7 @@ exports[`@primer/react > should not update exports without a semver change 1`] =
   "ActionMenu",
   "type ActionMenuAnchorProps",
   "type ActionMenuButtonProps",
+  "type ActionMenuIconButtonProps",
   "type ActionMenuProps",
   "AnchoredOverlay",
   "type AnchoredOverlayProps",

packages/react/src/index.ts

@@ -66,7 +66,12 @@ export type {
   ActionListTrailingVisualProps,
 } from './ActionList'
 export {ActionMenu} from './ActionMenu'
-export type {ActionMenuProps, ActionMenuAnchorProps, ActionMenuButtonProps} from './ActionMenu'
+export type {
+  ActionMenuProps,
+  ActionMenuAnchorProps,
+  ActionMenuButtonProps,
+  ActionMenuIconButtonProps,
+} from './ActionMenu'
 export {AnchoredOverlay} from './AnchoredOverlay'
 export type {AnchoredOverlayProps} from './AnchoredOverlay'
 export {default as Autocomplete} from './Autocomplete'