From 3b6b332b2645d06055f5373c366e62829929d4e1 Mon Sep 17 00:00:00 2001 From: Christopher Chedeau Date: Sat, 31 Oct 2026 03:00:02 -0700 Subject: [PATCH 1/2] fix(usePopover): its trigger ref stays the same function across renders usePopover built its trigger ref from the whole layer object, which changes whenever the popover's onShow or onHide does. A caller that passes inline callbacks got a new trigger ref on every render, so React detached and reattached the trigger each time. The layer's anchor came off the trigger and went back, two writes to its inline style per render, and with a tooltip anchored to the same trigger the anchor names changed order. The trigger ref is now keyed on the layer's own ref, which is stable, as useHoverCard's refs were in #6606. DropdownMenu also hands its trigger stable refs on all three trigger paths: a custom trigger, the default button (still forwarding the caller's ref), and the bottom-sheet trigger. Its own inline refs churned the same way. --- ...-no-longer-rewrites-its-trigger-s--83je.md | 6 + .../src/DropdownMenu/DropdownMenu.test.tsx | 117 ++++++++++++++++++ .../core/src/DropdownMenu/DropdownMenu.tsx | 51 +++++--- packages/core/src/Popover/Popover.test.tsx | 44 +++++++ packages/core/src/Popover/usePopover.tsx | 13 +- 5 files changed, 210 insertions(+), 21 deletions(-) create mode 100644 .changeset/a-dropdownmenu-no-longer-rewrites-its-trigger-s--83je.md diff --git a/.changeset/a-dropdownmenu-no-longer-rewrites-its-trigger-s--83je.md b/.changeset/a-dropdownmenu-no-longer-rewrites-its-trigger-s--83je.md new file mode 100644 index 0000000000000..60bc913a22e4b --- /dev/null +++ b/.changeset/a-dropdownmenu-no-longer-rewrites-its-trigger-s--83je.md @@ -0,0 +1,6 @@ +--- +'@astryxdesign/core': patch +--- + +[fix] `usePopover`'s trigger ref now stays the same function when the popover's `onShow` or `onHide` changes, and `DropdownMenu`'s own trigger refs are stable too. A trigger that takes it (DropdownMenu, Breadcrumbs, ComplexSelector, DateInput, DateRangeInput, DateTimeInput, MultiSelector and the trigger menu of `ChatComposerInput`) is no longer detached and reattached on renders that pass new callbacks, so its style is not rewritten each time and a tooltip anchored to the same trigger keeps its place. +@vjeux diff --git a/packages/core/src/DropdownMenu/DropdownMenu.test.tsx b/packages/core/src/DropdownMenu/DropdownMenu.test.tsx index e98af287724f3..2d9f2319dd6b8 100644 --- a/packages/core/src/DropdownMenu/DropdownMenu.test.tsx +++ b/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 ( + { + refs.push(props.ref); + return ( + + + + ); + }}> + {}} /> + + ); + } + const {rerender} = render(); + 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(); + + 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 ( + <> + {count} + {}}> + {}} /> + + + ); + } + const {rerender} = render(); + const trigger = screen.getByRole('button', {name: 'Actions'}); + const before = anchorNames(trigger); + expect(before.split(',')).toHaveLength(2); + + const stopWatching = watchStyleWrites(trigger); + rerender(); + + expect(stopWatching()).toBe(0); + expect(anchorNames(trigger)).toBe(before); + }); + + it('still hands the default trigger to a ref the caller passes', () => { + const buttonRef = createRef(); + render( + + {}} /> + , + ); + 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 ( + { + refs.push(props.ref); + return ( + + ); + }} + /> + ); + } + const {rerender} = render(); + rerender(); + + expect(refs.length).toBeGreaterThan(1); + expect(refs.at(-1)).toBe(refs[0]); + }); +}); diff --git a/packages/core/src/DropdownMenu/DropdownMenu.tsx b/packages/core/src/DropdownMenu/DropdownMenu.tsx index a41b0cc377f76..a1282181e6c74 100644 --- a/packages/core/src/DropdownMenu/DropdownMenu.tsx +++ b/packages/core/src/DropdownMenu/DropdownMenu.tsx @@ -792,13 +792,15 @@ function DropdownMenuBottomSheet({ {currentTitle} ); + // 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({ ) : ( + ); + } + const {rerender} = render(); + 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(); + records.push(...observer.takeRecords()); + observer.disconnect(); + + expect(records).toHaveLength(0); + expect(trigger.style.getPropertyValue('anchor-name')).toBe(anchorName); + }); +}); diff --git a/packages/core/src/Popover/usePopover.tsx b/packages/core/src/Popover/usePopover.tsx index 42c9519d0ec56..bc64eeb26f281 100644 --- a/packages/core/src/Popover/usePopover.tsx +++ b/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 From 08a5dc53a74f92493bc19b5598f1ef9e557e3b1f Mon Sep 17 00:00:00 2001 From: Christopher Chedeau Date: Sat, 31 Oct 2026 04:59:49 -0700 Subject: [PATCH 2/2] fix(usePopover): the changeset names only the triggers the stable ref reaches DateInput, DateRangeInput, MultiSelector and ComplexSelector's custom trigger wrap the trigger ref in an inline function of their own, and the composer's trigger menu calls it directly instead of passing it as a ref, so this change does not reach them. The changeset now names DropdownMenu and the triggers that take usePopover's ref directly. --- .../a-dropdownmenu-no-longer-rewrites-its-trigger-s--83je.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/a-dropdownmenu-no-longer-rewrites-its-trigger-s--83je.md b/.changeset/a-dropdownmenu-no-longer-rewrites-its-trigger-s--83je.md index 60bc913a22e4b..bf2d4867a8654 100644 --- a/.changeset/a-dropdownmenu-no-longer-rewrites-its-trigger-s--83je.md +++ b/.changeset/a-dropdownmenu-no-longer-rewrites-its-trigger-s--83je.md @@ -2,5 +2,5 @@ '@astryxdesign/core': patch --- -[fix] `usePopover`'s trigger ref now stays the same function when the popover's `onShow` or `onHide` changes, and `DropdownMenu`'s own trigger refs are stable too. A trigger that takes it (DropdownMenu, Breadcrumbs, ComplexSelector, DateInput, DateRangeInput, DateTimeInput, MultiSelector and the trigger menu of `ChatComposerInput`) is no longer detached and reattached on renders that pass new callbacks, so its style is not rewritten each time and a tooltip anchored to the same trigger keeps its place. +[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