diff --git a/packages/react/src/navigation-menu/content/NavigationMenuContent.tsx b/packages/react/src/navigation-menu/content/NavigationMenuContent.tsx index 46438b8d98d..0b347e5cc9f 100644 --- a/packages/react/src/navigation-menu/content/NavigationMenuContent.tsx +++ b/packages/react/src/navigation-menu/content/NavigationMenuContent.tsx @@ -99,11 +99,13 @@ export const NavigationMenuContent = React.forwardRef(function NavigationMenuCon const handleCurrentContentRef = React.useCallback( (node: HTMLDivElement | null) => { - if (node) { + // Inactive `keepMounted` content also mounts in the viewport; only the + // active content can own the shared sizing observer target. + if (node && open) { currentContentRef.current = node; } }, - [currentContentRef], + [currentContentRef, open], ); const commonProps: HTMLProps = { diff --git a/packages/react/src/navigation-menu/root/NavigationMenuRoot.test.tsx b/packages/react/src/navigation-menu/root/NavigationMenuRoot.test.tsx index 97bba0ac6e4..72b5f0bead6 100644 --- a/packages/react/src/navigation-menu/root/NavigationMenuRoot.test.tsx +++ b/packages/react/src/navigation-menu/root/NavigationMenuRoot.test.tsx @@ -252,8 +252,13 @@ function TestNavigationMenuOrientationAttributes() { ); } -function TestInlineNestedNavigationMenu(props: { nestedDefaultValue?: string | null } = {}) { - const { nestedDefaultValue = 'nested-item-1' } = props; +function TestInlineNestedNavigationMenu( + props: { + nestedDefaultValue?: string | null; + keepMountedContent?: boolean; + } = {}, +) { + const { nestedDefaultValue = 'nested-item-1', keepMountedContent = false } = props; const nestedRootProps = nestedDefaultValue == null ? undefined : { defaultValue: nestedDefaultValue }; @@ -263,7 +268,7 @@ function TestInlineNestedNavigationMenu(props: { nestedDefaultValue?: string | n Item 1 - + Link 1 @@ -271,7 +276,10 @@ function TestInlineNestedNavigationMenu(props: { nestedDefaultValue?: string | n Nested Item 1 - + Nested Link 1 @@ -279,7 +287,10 @@ function TestInlineNestedNavigationMenu(props: { nestedDefaultValue?: string | n Nested Item 2 - + Nested Link 2 @@ -292,7 +303,7 @@ function TestInlineNestedNavigationMenu(props: { nestedDefaultValue?: string | n Item 2 - + Link 3 @@ -687,6 +698,18 @@ function mockResizeObserver() { }; } +function primeOpenPopupSize( + popupRoot: HTMLElement, + positioner: HTMLElement, + width: number, + height: number, +) { + popupRoot.style.setProperty('--popup-width', 'auto'); + popupRoot.style.setProperty('--popup-height', 'auto'); + positioner.style.setProperty('--positioner-width', `${width}px`); + positioner.style.setProperty('--positioner-height', `${height}px`); +} + function TestDeeplyNestedNavigationMenu() { return ( @@ -2353,7 +2376,7 @@ describe('', () => { const positioner = screen.getByTestId('positioner'); const animations = mockAnimations(popupRoot); - let popupWidth = 250; + const popupWidth = 250; let popupHeight = 120; Object.defineProperty(popupRoot, 'offsetWidth', { @@ -2365,7 +2388,6 @@ describe('', () => { get: () => popupHeight, }); - popupWidth = 250; popupHeight = 220; animations.start(); fireEvent.click(screen.getByTestId('insert-content')); @@ -2379,7 +2401,7 @@ describe('', () => { }); await act(async () => { - animations.finish(); + await animations.finish(); await flushMicrotasks(); }); @@ -2410,22 +2432,65 @@ describe('', () => { const popupRoot = screen.getByTestId('popup-root'); const positioner = screen.getByTestId('positioner'); - const popupWidthValues = [250, 250]; const popupHeightValues = [120, 220]; - let popupWidth = 250; + const popupWidth = 250; let popupHeight = 220; Object.defineProperty(popupRoot, 'offsetWidth', { + configurable: true, + get: () => popupWidth, + }); + Object.defineProperty(popupRoot, 'offsetHeight', { configurable: true, get: () => { - const nextWidth = popupWidthValues.shift(); - if (nextWidth != null) { - popupWidth = nextWidth; + const nextHeight = popupHeightValues.shift(); + if (nextHeight != null) { + popupHeight = nextHeight; } - return popupWidth; + return popupHeight; }, }); + + await flushMicrotasks(); + + expect(screen.getByTestId('nested-popup-1')).not.toBe(null); + await waitFor(() => { + expect(popupRoot.style.getPropertyValue('--popup-width')).toBe('auto'); + expect(popupRoot.style.getPropertyValue('--popup-height')).toBe('auto'); + expect(positioner.style.getPropertyValue('--positioner-width')).toBe('250px'); + expect(positioner.style.getPropertyValue('--positioner-height')).toBe('220px'); + }); + } finally { + globalThis.BASE_UI_ANIMATIONS_DISABLED = previousAnimationsDisabled; + } + }); + + it('does not animate popup sizing when kept nested default content first moves into the portal', async () => { + const previousAnimationsDisabled = globalThis.BASE_UI_ANIMATIONS_DISABLED; + globalThis.BASE_UI_ANIMATIONS_DISABLED = false; + + let setPopupPropertySpy: ReturnType | undefined; + + try { + await render(); + const trigger1 = screen.getByTestId('trigger-1'); + + fireEvent.click(trigger1); + + const popupRoot = screen.getByTestId('popup-root'); + const positioner = screen.getByTestId('positioner'); + + setPopupPropertySpy = vi.spyOn(popupRoot.style, 'setProperty'); + + const popupHeightValues = [120, 220]; + const popupWidth = 250; + let popupHeight = 220; + + Object.defineProperty(popupRoot, 'offsetWidth', { + configurable: true, + get: () => popupWidth, + }); Object.defineProperty(popupRoot, 'offsetHeight', { configurable: true, get: () => { @@ -2440,15 +2505,137 @@ describe('', () => { await flushMicrotasks(); - expect(screen.getByTestId('nested-popup-1')).not.toBe(null); + expect(screen.getByTestId('nested-popup-1')).not.toHaveAttribute('hidden'); await waitFor(() => { expect(popupRoot.style.getPropertyValue('--popup-width')).toBe('auto'); expect(popupRoot.style.getPropertyValue('--popup-height')).toBe('auto'); expect(positioner.style.getPropertyValue('--positioner-width')).toBe('250px'); expect(positioner.style.getPropertyValue('--positioner-height')).toBe('220px'); }); + + const popupSetPropertyCalls = setPopupPropertySpy.mock.calls as Array< + [property: string, value: string, priority?: string] + >; + const fixedPopupHeightCalls = popupSetPropertyCalls + .filter((call) => call[0] === '--popup-height') + .map((call) => call[1]) + .filter((value) => value !== 'auto' && value !== '0px'); + + expect(fixedPopupHeightCalls.length).toBeGreaterThan(0); + expect(fixedPopupHeightCalls.every((value) => value === '220px')).toBe(true); + } finally { + setPopupPropertySpy?.mockRestore(); + globalThis.BASE_UI_ANIMATIONS_DISABLED = previousAnimationsDisabled; + } + }); + + it('updates popup sizing when switching kept inline nested content', async () => { + const restoreResizeObserver = mockResizeObserver(); + const previousAnimationsDisabled = globalThis.BASE_UI_ANIMATIONS_DISABLED; + globalThis.BASE_UI_ANIMATIONS_DISABLED = false; + + try { + await render(); + const trigger1 = screen.getByTestId('trigger-1'); + + fireEvent.click(trigger1); + await flushMicrotasks(); + + const popupRoot = screen.getByTestId('popup-root'); + const positioner = screen.getByTestId('positioner'); + const animations = mockAnimations(popupRoot); + + const popupWidth = 250; + let popupHeight = 220; + + Object.defineProperty(popupRoot, 'offsetWidth', { + configurable: true, + get: () => popupWidth, + }); + Object.defineProperty(popupRoot, 'offsetHeight', { + configurable: true, + get: () => popupHeight, + }); + + primeOpenPopupSize(popupRoot, positioner, 250, 220); + + popupHeight = 300; + animations.start(); + fireEvent.click(screen.getByTestId('nested-trigger-2')); + await flushMicrotasks(); + + expect(screen.getByTestId('nested-popup-2')).not.toHaveAttribute('hidden'); + await waitFor(() => { + expect(screen.getByTestId('nested-popup-1')).toHaveAttribute('hidden'); + }); + await waitFor(() => { + expect( + parseInt(getComputedStyle(positioner).getPropertyValue('--positioner-height'), 10), + ).toBe(300); + }); + + await act(async () => { + await animations.finish(); + await flushMicrotasks(); + }); + + await waitFor(() => { + expect(popupRoot.style.getPropertyValue('--popup-width')).toBe('auto'); + expect(popupRoot.style.getPropertyValue('--popup-height')).toBe('auto'); + expect(positioner.style.getPropertyValue('--positioner-width')).toBe('250px'); + expect(positioner.style.getPropertyValue('--positioner-height')).toBe('300px'); + }); + } finally { + globalThis.BASE_UI_ANIMATIONS_DISABLED = previousAnimationsDisabled; + restoreResizeObserver(); + } + }); + + it('updates popup sizing when a kept nested content hidden attribute changes', async () => { + const restoreResizeObserver = mockResizeObserver(); + const previousAnimationsDisabled = globalThis.BASE_UI_ANIMATIONS_DISABLED; + globalThis.BASE_UI_ANIMATIONS_DISABLED = false; + + try { + await render(); + const trigger1 = screen.getByTestId('trigger-1'); + + fireEvent.click(trigger1); + await flushMicrotasks(); + + const popupRoot = screen.getByTestId('popup-root'); + const positioner = screen.getByTestId('positioner'); + + const popupWidth = 250; + let popupHeight = 220; + + Object.defineProperty(popupRoot, 'offsetWidth', { + configurable: true, + get: () => popupWidth, + }); + Object.defineProperty(popupRoot, 'offsetHeight', { + configurable: true, + get: () => popupHeight, + }); + + primeOpenPopupSize(popupRoot, positioner, 250, 220); + + popupHeight = 300; + + await act(async () => { + // Contract-level check for the MutationObserver path; the sibling + // switch test above covers the React-driven `hidden` toggle. + screen.getByTestId('nested-popup-1').setAttribute('hidden', ''); + await flushMicrotasks(); + }); + + await waitFor(() => { + expect(positioner.style.getPropertyValue('--positioner-width')).toBe('250px'); + expect(positioner.style.getPropertyValue('--positioner-height')).toBe('300px'); + }); } finally { globalThis.BASE_UI_ANIMATIONS_DISABLED = previousAnimationsDisabled; + restoreResizeObserver(); } }); diff --git a/packages/react/src/navigation-menu/trigger/NavigationMenuTrigger.tsx b/packages/react/src/navigation-menu/trigger/NavigationMenuTrigger.tsx index a4c7323fb09..cb2e00b3f5c 100644 --- a/packages/react/src/navigation-menu/trigger/NavigationMenuTrigger.tsx +++ b/packages/react/src/navigation-menu/trigger/NavigationMenuTrigger.tsx @@ -458,6 +458,10 @@ export const NavigationMenuTrigger = React.forwardRef(function NavigationMenuTri childList: true, subtree: true, characterData: true, + // `keepMounted` submenu switches update dimensions by toggling hidden + // content rather than inserting or removing content nodes. + attributes: true, + attributeFilter: ['hidden'], }); return () => { @@ -489,15 +493,16 @@ export const NavigationMenuTrigger = React.forwardRef(function NavigationMenuTri useIsoLayoutEffect(() => { if (isActiveItemRef.current && open && popupElement) { - if (transitionStatus === 'starting') { - const hasNestedMenu = currentContentRef.current?.querySelector('[data-nested]') != null; - - if (hasNestedMenu) { - sizeFrame.request(syncCurrentSize); - return () => { - sizeFrame.cancel(); - }; - } + const hasNestedMenu = currentContentRef.current?.querySelector('[data-nested]') != null; + + if (transitionStatus === 'starting' && hasNestedMenu) { + // Inline nested menus can reveal their default content after the + // top-level content enters the viewport. Defer once so the opening + // size is measured from the final nested content, not the shell. + sizeFrame.request(syncCurrentSize); + return () => { + sizeFrame.cancel(); + }; } if (skipAutoSizeSyncRef.current) {