AnchoredOverlay: Ensure CSS anchor position styles persist

.changeset/poor-carrots-guess.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+AnchoredOverlay: Ensure styles persist on anchors even when re-mounted (behind feature flag)

packages/react/src/ActionMenu/ActionMenu.dev.stories.tsx

@@ -1,7 +1,9 @@
 import type {Meta} from '@storybook/react-vite'
+import {useRef, useState} from 'react'
 import type {ComponentProps} from '../utils/types'
 import {ActionMenu} from './ActionMenu'
 import {ActionList} from '../ActionList'
+import {Button} from '../Button'
 
 export default {
   title: 'Components/ActionMenu/Dev',
@@ -34,3 +36,47 @@ export const WithCss = () => (
     </ActionMenu.Overlay>
   </ActionMenu>
 )
+
+/**
+ * Reproduces a bug where switching the anchor DOM element (via unmount/remount)
+ * causes the CSS anchor positioning to break because `anchor-name` is never
+ * re-applied to the new element.
+ */
+export const AnchorElementReplacement = () => {
+  const anchorRef = useRef<HTMLButtonElement>(null)
+  const [open, setOpen] = useState(false)
+  const [anchorKey, setAnchorKey] = useState(0)
+
+  return (
+    <div style={{padding: 40}}>
+      <p style={{marginBottom: 8, fontSize: 14, color: '#656d76'}}>
+        1. Open the menu below. 2. Click &quot;Switch anchor (remount)&quot; inside the menu. 3. The overlay should
+        remain anchored to the button — not jump to the top-left corner.
+      </p>
+
+      <Button key={anchorKey} ref={anchorRef} onClick={() => setOpen(o => !o)}>
+        Open menu (anchor v{anchorKey})
+      </Button>
+
+      <ActionMenu anchorRef={anchorRef} open={open} onOpenChange={setOpen}>
+        <ActionMenu.Overlay>
+          <ActionList>
+            <ActionList.Item
+              onSelect={event => {
+                // Prevent the menu from closing when clicking this item
+                event.preventDefault()
+                setAnchorKey(k => k + 1)
+              }}
+            >
+              Switch anchor (remount)
+            </ActionList.Item>
+            <ActionList.Divider />
+            <ActionList.Item>Item one</ActionList.Item>
+            <ActionList.Item>Item two</ActionList.Item>
+            <ActionList.Item>Item three</ActionList.Item>
+          </ActionList>
+        </ActionMenu.Overlay>
+      </ActionMenu>
+    </div>
+  )
+}

packages/react/src/AnchoredOverlay/AnchoredOverlay.test.tsx

@@ -391,3 +391,64 @@ describe('AnchoredOverlay feature flag specific behavior', () => {
     })
   })
 })
+
+describe('AnchoredOverlay anchor element replacement', () => {
+  it('should re-apply anchor-name to a new anchor DOM element when the overlay reopens', () => {
+    function TestComponent() {
+      const anchorRef = useRef<HTMLButtonElement>(null)
+      const [open, setOpen] = useState(true)
+      const [anchorKey, setAnchorKey] = useState(0)
+
+      return (
+        <FeatureFlags flags={{primer_react_css_anchor_positioning: true}}>
+          <button type="button" data-testid="switch" onClick={() => setAnchorKey(k => k + 1)}>
+            Switch
+          </button>
+          <button type="button" data-testid="toggle" onClick={() => setOpen(o => !o)}>
+            Toggle
+          </button>
+          <button key={anchorKey} ref={anchorRef} type="button" data-testid="anchor">
+            Anchor
+          </button>
+          <AnchoredOverlay
+            open={open}
+            onOpen={() => setOpen(true)}
+            onClose={() => setOpen(false)}
+            renderAnchor={null}
+            anchorRef={anchorRef}
+          >
+            <div>content</div>
+          </AnchoredOverlay>
+        </FeatureFlags>
+      )
+    }
+
+    const {baseElement} = render(<TestComponent />)
+
+    // Verify anchor-name is set on the initial anchor element
+    const initialAnchor = baseElement.querySelector('[data-testid="anchor"]') as HTMLElement
+    expect(initialAnchor.style.getPropertyValue('anchor-name')).not.toBe('')
+    const anchorName = initialAnchor.style.getPropertyValue('anchor-name')
+
+    // Close the overlay
+    const toggleButton = baseElement.querySelector('[data-testid="toggle"]') as HTMLElement
+    act(() => {
+      toggleButton.click()
+    })
+
+    // Replace the anchor DOM element by changing its key while overlay is closed
+    const switchButton = baseElement.querySelector('[data-testid="switch"]') as HTMLElement
+    act(() => {
+      switchButton.click()
+    })
+
+    // Reopen the overlay — the new anchor should get anchor-name before paint
+    act(() => {
+      toggleButton.click()
+    })
+
+    const newAnchor = baseElement.querySelector('[data-testid="anchor"]') as HTMLElement
+    expect(newAnchor).not.toBe(initialAnchor)
+    expect(newAnchor.style.getPropertyValue('anchor-name')).toBe(anchorName)
+  })
+})

packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx

@@ -1,6 +1,5 @@
 import type React from 'react'
-import {useCallback, useEffect, useRef, type JSX} from 'react'
-import useLayoutEffect from '../utils/useIsomorphicLayoutEffect'
+import {useCallback, useEffect, useState, type JSX} from 'react'
 import type {OverlayProps} from '../Overlay'
 import Overlay from '../Overlay'
 import type {FocusTrapHookSettings} from '../hooks/useFocusTrap'
@@ -170,13 +169,32 @@ export const AnchoredOverlay: React.FC<React.PropsWithChildren<AnchoredOverlayPr
   renderAs = 'portal',
 }) => {
   const cssAnchorPositioningFlag = useFeatureFlag('primer_react_css_anchor_positioning')
-  const supportsNativeCSSAnchorPositioning = useRef(false)
-  // eslint-disable-next-line react-hooks/refs
-  const cssAnchorPositioning = cssAnchorPositioningFlag && supportsNativeCSSAnchorPositioning.current
+  // Lazy initial state so feature detection runs once per mount on the client.
+  // Guarded for SSR where `document` is undefined.
+  const [supportsNativeCSSAnchorPositioning] = useState(
+    () => typeof document !== 'undefined' && 'anchorName' in document.documentElement.style,
+  )
+
+  const cssAnchorPositioning = cssAnchorPositioningFlag && supportsNativeCSSAnchorPositioning
   // Only use Popover API when both CSS anchor positioning is enabled AND renderAs is true
   const shouldRenderAsPopover = cssAnchorPositioning && renderAs === 'popover'
   const anchorRef = useProvidedRefOrCreate(externalAnchorRef)
+  // Track the current anchor DOM element in state so that effects depending on
+  // its identity (e.g. applying `anchor-name` for CSS anchor positioning) re-run
+  // when a consumer remounts/replaces the anchor element while keeping the same
+  // `anchorRef` object. Refs alone don't notify React when `.current` changes.
+  const [anchorElement, setAnchorElement] = useState<HTMLElement | null>(null)
+  // Detect when `anchorRef.current` has been mutated by a consumer (e.g. via
+  // their own `ref={anchorRef}`) without React's ref system notifying us.
+  // Reading the ref during render and conditionally calling setState is the
+  // documented React pattern for syncing external mutable state, but the
+  // react-hooks/refs lint rule is conservative and disallows it.
+  // eslint-disable-next-line react-hooks/refs
+  if (anchorRef.current !== anchorElement) {
+    setAnchorElement(anchorRef.current)
+  }
   const [overlayRef, updateOverlayRef] = useRenderForcingRef<HTMLDivElement>()
+  const [overlayElement, setOverlayElement] = useState<HTMLDivElement | null>(null)
   const anchorId = useId(externalAnchorId)
 
   const onClickOutside = useCallback(() => onClose?.('click-outside'), [onClose])
@@ -230,13 +248,11 @@ export const AnchoredOverlay: React.FC<React.PropsWithChildren<AnchoredOverlayPr
       displayInViewport,
       onPositionChange: positionChange,
     },
-    // eslint-disable-next-line react-hooks/refs
-    [overlayRef.current],
+
+    [overlayElement],
   )
 
   useEffect(() => {
-    supportsNativeCSSAnchorPositioning.current = 'anchorName' in document.documentElement.style
-
     // ensure overlay ref gets cleared when closed, so position can reset between closing/re-opening
     if (!open && overlayRef.current) {
       updateOverlayRef(null)
@@ -254,36 +270,16 @@ export const AnchoredOverlay: React.FC<React.PropsWithChildren<AnchoredOverlayPr
   const id = popoverId.replaceAll(':', '_') // popoverId can contain colons which are invalid in CSS custom property names, so we replace them with underscores
 
   useEffect(() => {
-    if (!cssAnchorPositioning || !anchorRef.current) return
-
-    const anchor = anchorRef.current
-    const overlay = overlayRef.current
-    anchor.style.setProperty('anchor-name', `--anchored-overlay-anchor-${id}`)
-
-    return () => {
-      anchor.style.removeProperty('anchor-name')
-      if (overlay) {
-        overlay.style.removeProperty('position-anchor')
-      }
-    }
-    // eslint-disable-next-line react-hooks/refs
-  }, [cssAnchorPositioning, anchorRef, overlayRef, id, open])
+    if (!cssAnchorPositioning || !anchorElement) return
 
-  // Track the overlay element so we can re-run the effect when it changes.
-  // The overlay unmounts when closed, so each open creates a new DOM node -
-  // that needs showPopover() called.
-  // eslint-disable-next-line react-hooks/refs
-  const overlayElement = overlayRef.current
-
-  useLayoutEffect(() => {
-    // Read ref inside effect to get the value after child refs are attached
     const currentOverlay = overlayRef.current
 
-    if (!cssAnchorPositioning || !open || !currentOverlay) return
-    currentOverlay.style.setProperty('position-anchor', `--anchored-overlay-anchor-${id}`)
+    // Link the anchor and the overlay (when present) via CSS anchor positioning.
+    anchorElement.style.setProperty('anchor-name', `--anchored-overlay-anchor-${id}`)
+
+    if (open && currentOverlay) {
+      currentOverlay.style.setProperty('position-anchor', `--anchored-overlay-anchor-${id}`)
 
-    const anchorElement = anchorRef.current
-    if (anchorElement) {
       const overlayWidth = width ? parseInt(widthMap[width]) : null
       const result = getDefaultPosition(anchorElement, overlayWidth)
 
@@ -292,20 +288,29 @@ export const AnchoredOverlay: React.FC<React.PropsWithChildren<AnchoredOverlayPr
       // Apply offset only when viewport is too narrow
       const offset = result.horizontal === 'left' ? result.leftOffset : result.rightOffset
       currentOverlay.style.setProperty(`--anchored-overlay-anchor-offset-${result.horizontal}`, `${offset || 0}px`)
-    }
 
-    // Only call showPopover when renderAs is enabled
-    if (shouldRenderAsPopover) {
-      try {
-        if (!currentOverlay.matches(':popover-open')) {
-          currentOverlay.showPopover()
+      // Only call showPopover when shouldRenderAsPopover is enabled
+      if (shouldRenderAsPopover) {
+        try {
+          if (!currentOverlay.matches(':popover-open')) {
+            currentOverlay.showPopover()
+          }
+        } catch {
+          // Ignore if popover is already showing or not supported
         }
-      } catch {
-        // Ignore if popover is already showing or not supported
       }
     }
-    // eslint-disable-next-line react-hooks/refs
-  }, [cssAnchorPositioning, shouldRenderAsPopover, open, overlayElement, id, overlayRef, anchorRef, width])
+
+    return () => {
+      anchorElement.style.removeProperty('anchor-name')
+      // The overlay may no longer be in the DOM at this point, so we need to check for its presence before trying to update it.
+      if (currentOverlay) {
+        currentOverlay.style.removeProperty('position-anchor')
+      }
+    }
+    // overlayRef is a stable ref object; including it in deps is unnecessary.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [cssAnchorPositioning, shouldRenderAsPopover, open, anchorElement, overlayElement, id, width])
 
   const showXIcon = onClose && variant.narrow === 'fullscreen' && displayCloseButton
   const XButtonAriaLabelledBy = closeButtonProps['aria-labelledby']
@@ -316,6 +321,8 @@ export const AnchoredOverlay: React.FC<React.PropsWithChildren<AnchoredOverlayPr
   return (
     <>
       {renderAnchor &&
+        // anchorRef is a ref object passed as a JSX `ref` prop on the rendered
+        // anchor; React writes to it at commit time, it is not read during render.
         // eslint-disable-next-line react-hooks/refs
         renderAnchor({
           ref: anchorRef,
@@ -353,6 +360,7 @@ export const AnchoredOverlay: React.FC<React.PropsWithChildren<AnchoredOverlayPr
               assignRef(overlayProps.ref, node)
             }
             updateOverlayRef(node)
+            setOverlayElement(node)
           }}
           data-anchor-position={cssAnchorPositioning}
           data-side={cssAnchorPositioning ? side : position?.anchorSide}