ActionMenu: Avoid passive anchor label updates

.changeset/quiet-action-menu-labels.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+ActionMenu: Avoid an extra render when closed menus mount while preserving accessible trigger labeling

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

@@ -1,6 +1,7 @@
 import {describe, expect, it, vi, beforeEach} from 'vitest'
 import {render as HTMLRender, waitFor, act, within} from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
+import {page, userEvent as browserUserEvent} from 'vitest/browser'
 import type React from 'react'
 import {useRef, useState} from 'react'
 import BaseStyles from '../BaseStyles'
@@ -18,6 +19,7 @@ import type {AnchorPosition} from '@primer/behaviors'
 
 import type {JSX} from 'react'
 import {implementsClassName} from '../utils/testing'
+import {createRenderCounter} from '../utils/testing/profiler'
 import {FeatureFlags} from '../FeatureFlags'
 
 // Mock getAnchoredPosition for feature flag tests
@@ -82,6 +84,30 @@ function ExampleWithTooltip(): JSX.Element {
   )
 }
 
+function IconButtonActionMenu({
+  onSelect,
+  onFocus,
+  ...props
+}: {onSelect?: () => void; onFocus?: React.FocusEventHandler<HTMLElement>} & Pick<
+  React.ComponentProps<typeof ActionMenu>,
+  'open' | 'onOpenChange'
+> = {}): JSX.Element {
+  return (
+    <ActionMenu {...props}>
+      <ActionMenu.Anchor>
+        <IconButton icon={SearchIcon} aria-label="More actions" />
+      </ActionMenu.Anchor>
+      <ActionMenu.Overlay width="medium">
+        <ActionList>
+          <ActionList.Item onSelect={onSelect} onFocus={onFocus}>
+            Copy link
+          </ActionList.Item>
+        </ActionList>
+      </ActionMenu.Overlay>
+    </ActionMenu>
+  )
+}
+
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 function ExampleWithTooltipV2(actionMenuTrigger: React.ReactElement<any>): JSX.Element {
   return (
@@ -663,30 +689,113 @@ describe('ActionMenu', () => {
     expect(button.id).toBe(buttonId)
   })
 
-  it('should use the tooltip id to name the menu when the anchor is icon button', async () => {
-    const component = HTMLRender(
-      <BaseStyles>
-        <ActionMenu>
-          <ActionMenu.Anchor>
-            <IconButton icon={SearchIcon} aria-label="More actions" />
-          </ActionMenu.Anchor>
+  describe('anchor labeling and focus behavior', () => {
+    it('does not publish anchor label state after mounting closed', async () => {
+      const [Wrap, counter] = createRenderCounter()
+      HTMLRender(
+        <Wrap>
+          <BaseStyles>
+            <IconButtonActionMenu />
+          </BaseStyles>
+        </Wrap>,
+      )
 
-          <ActionMenu.Overlay width="medium">
-            <ActionList>
-              <ActionList.Item onSelect={() => alert('Copy link clicked')}>
-                Copy link
-                <ActionList.TrailingVisual>⌘C</ActionList.TrailingVisual>
-              </ActionList.Item>
-            </ActionList>
-          </ActionMenu.Overlay>
-        </ActionMenu>
-      </BaseStyles>,
-    )
+      await act(async () => {})
+
+      expect(counter.mountCount).toBe(1)
+      expect(counter.updateCount).toBe(0)
+    })
 
-    const toggleButton = component.getByRole('button', {name: 'More actions'})
-    await userEvent.click(toggleButton)
-    expect(toggleButton).toHaveAttribute('aria-labelledby')
-    expect(component.getByRole('menu')).toHaveAttribute('aria-labelledby', toggleButton.getAttribute('aria-labelledby'))
+    it('labels an initially open controlled menu before item focus', async () => {
+      const onFocus = vi.fn(() => within(document.body).queryByRole('menu', {name: 'More actions'}) !== null)
+      const component = HTMLRender(
+        <FeatureFlags flags={{primer_react_css_anchor_positioning: true}}>
+          <BaseStyles>
+            <IconButtonActionMenu open onOpenChange={vi.fn()} onFocus={onFocus} />
+          </BaseStyles>
+        </FeatureFlags>,
+      )
+
+      await expect.element(page.getByRole('menuitem', {name: 'Copy link', exact: true})).toHaveFocus()
+      expect(onFocus.mock.results[0].value).toBe(true)
+      const menu = component.getByRole('menu', {name: 'More actions'})
+      const anchor = component.getByRole('button', {name: 'More actions'})
+      expect(menu).toHaveAttribute('aria-labelledby', anchor.getAttribute('aria-labelledby'))
+    })
+
+    it('uses the tooltip id to name the menu before first item focus', async () => {
+      const onFocus = vi.fn(() => within(document.body).queryByRole('menu', {name: 'More actions'}) !== null)
+      const component = HTMLRender(
+        <FeatureFlags flags={{primer_react_css_anchor_positioning: true}}>
+          <BaseStyles>
+            <IconButtonActionMenu onFocus={onFocus} />
+          </BaseStyles>
+        </FeatureFlags>,
+      )
+      const toggleButton = component.getByRole('button', {name: 'More actions'})
+
+      await browserUserEvent.click(page.getByRole('button', {name: 'More actions', exact: true}))
+
+      await expect.element(page.getByRole('menuitem', {name: 'Copy link', exact: true})).toHaveFocus()
+      expect(onFocus.mock.results[0].value).toBe(true)
+      expect(toggleButton).toHaveAttribute('aria-labelledby')
+      expect(component.getByRole('menu', {name: 'More actions'})).toHaveAttribute(
+        'aria-labelledby',
+        toggleButton.getAttribute('aria-labelledby'),
+      )
+    })
+
+    it('closes with Escape and returns focus to the anchor', async () => {
+      const component = HTMLRender(
+        <BaseStyles>
+          <IconButtonActionMenu />
+        </BaseStyles>,
+      )
+      const user = userEvent.setup()
+      const toggleButton = component.getByRole('button', {name: 'More actions'})
+      await user.click(toggleButton)
+      component.getByRole('menuitem', {name: 'Copy link'}).focus()
+
+      await user.keyboard('{Escape}')
+
+      expect(component.queryByRole('menu')).not.toBeInTheDocument()
+      expect(toggleButton).toHaveFocus()
+    })
+
+    it('closes with Tab without returning focus to the anchor', async () => {
+      const component = HTMLRender(
+        <BaseStyles>
+          <IconButtonActionMenu />
+        </BaseStyles>,
+      )
+      const user = userEvent.setup()
+      const toggleButton = component.getByRole('button', {name: 'More actions'})
+      await user.click(toggleButton)
+      component.getByRole('menuitem', {name: 'Copy link'}).focus()
+
+      await user.tab()
+
+      expect(component.queryByRole('menu')).not.toBeInTheDocument()
+      expect(toggleButton).not.toHaveFocus()
+    })
+
+    it('closes after selection and returns focus to the anchor', async () => {
+      const onSelect = vi.fn()
+      const component = HTMLRender(
+        <BaseStyles>
+          <IconButtonActionMenu onSelect={onSelect} />
+        </BaseStyles>,
+      )
+      const user = userEvent.setup()
+      const toggleButton = component.getByRole('button', {name: 'More actions'})
+      await user.click(toggleButton)
+
+      await user.click(component.getByRole('menuitem', {name: 'Copy link'}))
+
+      expect(onSelect).toHaveBeenCalledOnce()
+      expect(component.queryByRole('menu')).not.toBeInTheDocument()
+      expect(toggleButton).toHaveFocus()
+    })
   })
 
   describe('submenus', () => {

packages/react/src/ActionMenu/ActionMenu.tsx

@@ -1,4 +1,4 @@
-import React, {useCallback, useContext, useMemo, useEffect, useState} from 'react'
+import React, {useCallback, useContext, useMemo, useState} from 'react'
 import {clsx} from 'clsx'
 import {TriangleDownIcon, ChevronRightIcon} from '@primer/octicons-react'
 import type {AnchoredOverlayProps} from '../AnchoredOverlay'
@@ -19,6 +19,7 @@ import {useResponsiveValue, type ResponsiveValue} from '../hooks/useResponsiveVa
 import {isSlot} from '../utils/is-slot'
 import type {FCWithSlotMarker, WithSlotMarker} from '../utils/types/Slots'
 import {DialogContext} from '../Dialog/DialogContext'
+import useLayoutEffect from '../utils/useIsomorphicLayoutEffect'
 
 export type MenuCloseHandler = (
   gesture: 'anchor-click' | 'click-outside' | 'escape' | 'tab' | 'item-select' | 'arrow-left' | 'close',
@@ -312,22 +313,23 @@ const Overlay: FCWithSlotMarker<React.PropsWithChildren<MenuOverlayProps>> = ({
     [isNarrowFullscreen, onClose],
   )
 
-  useMenuKeyboardNavigation(open, handleClose, containerRef, anchorRef, isSubmenu)
+  // Wait for the anchor label before mounting or focusing the menu.
+  const [anchorLabelReady, setAnchorLabelReady] = useState(false)
+  if (!open && anchorLabelReady) {
+    setAnchorLabelReady(false)
+  }
+  useMenuKeyboardNavigation(open && anchorLabelReady, handleClose, containerRef, anchorRef, isSubmenu)
 
   const responsiveVariant = useResponsiveValue(variant, {regular: 'anchored', narrow: 'anchored'})
 
   // If the menu anchor is an icon button, we need to label the menu by tooltip that also labelled the anchor.
   const [anchorAriaLabelledby, setAnchorAriaLabelledby] = useState<null | string>(null)
-  useEffect(() => {
-    // Necessary for HMR reloads
-    // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
-    if (anchorRef?.current) {
-      const ariaLabelledby = anchorRef.current.getAttribute('aria-labelledby')
-      if (ariaLabelledby) {
-        setAnchorAriaLabelledby(ariaLabelledby)
-      }
+  useLayoutEffect(() => {
+    if (open) {
+      setAnchorAriaLabelledby(anchorRef.current?.getAttribute('aria-labelledby') ?? null)
+      setAnchorLabelReady(true)
     }
-  }, [anchorRef])
+  }, [anchorRef, open])
 
   const afterSelect = useCallback(() => onClose?.('item-select'), [onClose])
 
@@ -350,7 +352,7 @@ const Overlay: FCWithSlotMarker<React.PropsWithChildren<MenuOverlayProps>> = ({
       anchorRef={anchorRef}
       renderAnchor={renderAnchor}
       anchorId={anchorId}
-      open={open}
+      open={open && anchorLabelReady}
       onOpen={onOpen}
       onClose={handleClose}
       align={align}