fix(usePopover): its trigger ref stays the same function across renders

.changeset/a-dropdownmenu-no-longer-rewrites-its-trigger-s--83je.md

@@ -0,0 +1,6 @@
+---
+'@astryxdesign/core': patch
+---
+
+[fix] `DropdownMenu` no longer rewrites its trigger's style on every render. Its trigger refs, and the trigger ref `usePopover` returns, now stay the same function when the popover's `onShow` or `onHide` changes, so React stops detaching and reattaching the trigger, and a tooltip anchored to the same trigger keeps its place. Triggers that take `usePopover`'s ref directly (ComplexSelector's built-in trigger, DateTimeInput's date field and Breadcrumbs) get the stable ref too.
+@vjeux

packages/core/src/DropdownMenu/DropdownMenu.test.tsx

@@ -30,6 +30,7 @@ import {DropdownMenuGroup} from './DropdownMenuGroup';
 import {warmMenuBottomSheet} from './__tests__/warmMenuBottomSheet';
 import {DropdownMenuSubMenu} from './DropdownMenuSubMenu';
 import {Divider} from '../Divider';
+import {Tooltip} from '../Tooltip/Tooltip';
 import {rtlStyles} from '../utils';
 import {__resetInteractionModalityForTest} from '../utils/interactionModality';
 import {focusOutlineStyles} from '../utils/focusOutline.stylex';
@@ -4379,3 +4380,119 @@ describe('DropdownMenu header and footer', () => {
     ).toBeTruthy();
   });
 });
+
+describe('DropdownMenu trigger refs across renders', () => {
+  // A ref that is a new function on every render makes React call it again on
+  // every commit (with null, then the element). Each call moved the menu's
+  // anchor off the trigger and back, rewriting the trigger's style and
+  // reordering its anchor names whenever a tooltip anchors there too.
+  function watchStyleWrites(element: HTMLElement) {
+    const records: MutationRecord[] = [];
+    const observer = new MutationObserver(found => records.push(...found));
+    observer.observe(element, {attributes: true, attributeFilter: ['style']});
+    return () => {
+      records.push(...observer.takeRecords());
+      observer.disconnect();
+      return records.length;
+    };
+  }
+
+  const anchorNames = (element: HTMLElement) =>
+    element.style.getPropertyValue('anchor-name');
+
+  it('hands renderTrigger the same ref on every render, so a render leaves the trigger and its tooltip anchor alone', () => {
+    const refs: DropdownMenuTriggerProps['ref'][] = [];
+    function Menu({count}: {count: number}) {
+      return (
+        <DropdownMenu
+          renderTrigger={props => {
+            refs.push(props.ref);
+            return (
+              <Tooltip content="More actions">
+                <button type="button" {...props}>
+                  Menu {count}
+                </button>
+              </Tooltip>
+            );
+          }}>
+          <DropdownMenuItem label="Profile" onClick={() => {}} />
+        </DropdownMenu>
+      );
+    }
+    const {rerender} = render(<Menu count={0} />);
+    const trigger = screen.getByRole('button', {name: /^Menu/});
+    const before = anchorNames(trigger);
+    // The menu's anchor and the tooltip's, on one element.
+    expect(before.split(',')).toHaveLength(2);
+
+    const stopWatching = watchStyleWrites(trigger);
+    rerender(<Menu count={1} />);
+
+    expect(stopWatching()).toBe(0);
+    expect(anchorNames(trigger)).toBe(before);
+    expect(refs.length).toBeGreaterThan(1);
+    expect(refs.at(-1)).toBe(refs[0]);
+  });
+
+  it('leaves the default trigger and its tooltip anchor alone on a render', () => {
+    function Menu({count}: {count: number}) {
+      return (
+        <>
+          <output>{count}</output>
+          <DropdownMenu
+            button={{label: 'Actions', tooltip: 'More actions'}}
+            onOpenChange={() => {}}>
+            <DropdownMenuItem label="Profile" onClick={() => {}} />
+          </DropdownMenu>
+        </>
+      );
+    }
+    const {rerender} = render(<Menu count={0} />);
+    const trigger = screen.getByRole('button', {name: 'Actions'});
+    const before = anchorNames(trigger);
+    expect(before.split(',')).toHaveLength(2);
+
+    const stopWatching = watchStyleWrites(trigger);
+    rerender(<Menu count={1} />);
+
+    expect(stopWatching()).toBe(0);
+    expect(anchorNames(trigger)).toBe(before);
+  });
+
+  it('still hands the default trigger to a ref the caller passes', () => {
+    const buttonRef = createRef<HTMLButtonElement>();
+    render(
+      <DropdownMenu button={{label: 'Actions', ref: buttonRef}}>
+        <DropdownMenuItem label="Profile" onClick={() => {}} />
+      </DropdownMenu>,
+    );
+    expect(buttonRef.current).toBe(
+      screen.getByRole('button', {name: 'Actions'}),
+    );
+  });
+
+  it('hands the bottom-sheet trigger the same ref on every render', () => {
+    const refs: DropdownMenuTriggerProps['ref'][] = [];
+    function Menu({count}: {count: number}) {
+      return (
+        <DropdownMenu
+          presentation="bottom-sheet"
+          items={[{label: 'Profile'}]}
+          renderTrigger={props => {
+            refs.push(props.ref);
+            return (
+              <button type="button" {...props}>
+                Menu {count}
+              </button>
+            );
+          }}
+        />
+      );
+    }
+    const {rerender} = render(<Menu count={0} />);
+    rerender(<Menu count={1} />);
+
+    expect(refs.length).toBeGreaterThan(1);
+    expect(refs.at(-1)).toBe(refs[0]);
+  });
+});

packages/core/src/DropdownMenu/DropdownMenu.tsx

@@ -792,13 +792,15 @@ function DropdownMenuBottomSheet({
       {currentTitle}
     </Heading>
   );
+  // A stable ref: an inline one re-runs on every render (null, then the element).
+  const sheetTriggerRef = useCallback((el: HTMLElement | null) => {
+    buttonRef.current = el;
+  }, []);
   return (
     <>
       {renderTrigger != null ? (
         renderTrigger({
-          ref: el => {
-            buttonRef.current = el;
-          },
+          ref: sheetTriggerRef,
           id: triggerId,
           onClick: handleSheetTriggerClick,
           onClickCapture: () => {},
@@ -1591,11 +1593,35 @@ function DropdownMenuPopover({
       </>
     );
 
-  const triggerProps: DropdownMenuTriggerProps = {
-    ref: el => {
+  // Stable trigger refs. An inline ref is a new function every render, so React
+  // re-runs it on every commit (null, then the element), and each run moved the
+  // popover's anchor off the trigger and back: two inline-style writes on the
+  // trigger per render of the menu.
+  const popoverTriggerRef = popover.triggerRef;
+  const triggerRef = useCallback(
+    (el: HTMLElement | null) => {
       buttonRef.current = el;
-      popover.triggerRef(el);
+      popoverTriggerRef(el);
+    },
+    [popoverTriggerRef],
+  );
+  const consumerButtonRef = button.ref;
+  const buttonTriggerRef = useCallback(
+    (el: HTMLButtonElement | null) => {
+      triggerRef(el);
+      if (typeof consumerButtonRef === 'function') {
+        consumerButtonRef(el);
+      } else if (consumerButtonRef) {
+        /* eslint-disable react-compiler/react-compiler -- ref callback: forwarding consumer ref object */
+        consumerButtonRef.current = el;
+        /* eslint-enable react-compiler/react-compiler */
+      }
     },
+    [triggerRef, consumerButtonRef],
+  );
+
+  const triggerProps: DropdownMenuTriggerProps = {
+    ref: triggerRef,
     id: triggerId,
     onClick: handleButtonClick,
     onClickCapture: handleTriggerClickCapture,
@@ -1618,18 +1644,7 @@ function DropdownMenuPopover({
       ) : (
         <Button
           {...button}
-          ref={el => {
-            buttonRef.current = el;
-            popover.triggerRef(el);
-            const consumerRef = button.ref;
-            if (typeof consumerRef === 'function') {
-              consumerRef(el);
-            } else if (consumerRef) {
-              /* eslint-disable react-compiler/react-compiler -- ref callback: forwarding consumer ref object */
-              consumerRef.current = el;
-              /* eslint-enable react-compiler/react-compiler */
-            }
-          }}
+          ref={buttonTriggerRef}
           xstyle={[
             styles.triggerPressable,
             isOpen && styles.triggerOpen,

packages/core/src/Popover/Popover.test.tsx

@@ -28,6 +28,7 @@ import {
   act,
   within,
   cleanup,
+  renderHook,
 } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import * as stylex from '@stylexjs/stylex';
@@ -1462,3 +1463,46 @@ describe('Popover focus region (DEC-9)', () => {
     });
   });
 });
+
+describe('usePopover trigger ref', () => {
+  // The layer object changes whenever onShow or onHide does. A trigger ref
+  // that changed with it made React detach and reattach the trigger on every
+  // render of a caller that passes inline callbacks.
+  it('stays the same function when onShow and onHide change across renders', () => {
+    const {result, rerender} = renderHook(
+      ({onShow, onHide}: {onShow: () => void; onHide: () => void}) =>
+        usePopover({onShow, onHide}),
+      {initialProps: {onShow: () => {}, onHide: () => {}}},
+    );
+    const first = result.current.triggerRef;
+
+    rerender({onShow: () => {}, onHide: () => {}});
+
+    expect(result.current.triggerRef).toBe(first);
+  });
+
+  it('leaves the trigger style alone on a render with new callbacks', () => {
+    function Trigger({count}: {count: number}) {
+      const popover = usePopover({onShow: () => {}, onHide: () => {}});
+      return (
+        <button type="button" ref={popover.triggerRef}>
+          Open {count}
+        </button>
+      );
+    }
+    const {rerender} = render(<Trigger count={0} />);
+    const trigger = screen.getByRole('button', {name: /^Open/});
+    const anchorName = trigger.style.getPropertyValue('anchor-name');
+    expect(anchorName).toMatch(/^--astryx-layer-/);
+    const records: MutationRecord[] = [];
+    const observer = new MutationObserver(found => records.push(...found));
+    observer.observe(trigger, {attributes: true, attributeFilter: ['style']});
+
+    rerender(<Trigger count={1} />);
+    records.push(...observer.takeRecords());
+    observer.disconnect();
+
+    expect(records).toHaveLength(0);
+    expect(trigger.style.getPropertyValue('anchor-name')).toBe(anchorName);
+  });
+});

packages/core/src/Popover/usePopover.tsx

@@ -459,13 +459,20 @@ function usePopoverImplementation(
     }
   }, [layer.isOpen, hasAutoFocus, focusInitialTarget]);
 
-  // Combined ref for trigger element (layer anchor + our ref)
+  // Combined ref for trigger element (layer anchor + our ref). It is keyed on
+  // the layer's own ref, not the whole layer: the layer object changes with
+  // every new onShow or onHide, and a trigger ref that changed with it made
+  // React detach and reattach the trigger on every render. It reads nothing
+  // else from the layer, and layer.ref (useLayer's contextRef) reads only refs
+  // and its anchor name and anchor-clearance option, so nothing in it goes
+  // stale.
+  const layerRef = layer.ref;
   const triggerRef = useCallback(
     (el: HTMLElement | null) => {
       triggerElementRef.current = el;
-      layer.ref(el);
+      layerRef(el);
     },
-    [layer],
+    [layerRef],
   );
 
   // Show function with optional focus preservation