From d970f0cb7784b473b5d5c19a993f9facb39827e0 Mon Sep 17 00:00:00 2001 From: Bernardo Sunderhus Date: Thu, 28 May 2026 12:40:35 +0200 Subject: [PATCH 1/9] fix(react-overflow,priority-overflow): produce a correct overflow snapshot before the first paint Mounting the Overflow container previously produced a one-frame flash where items rendered without accounting for the overflow menu. Force a synchronous overflow computation at the right moments so the first painted frame already reflects the final visible/invisible split. priority-overflow: - addOverflowMenu and removeOverflowMenu now schedule a forced dispatch when observing, so attaching the menu element does not leave the snapshot stale react-overflow: - the observe layout effect in useOverflowContainer calls manager.forceUpdate right after observation starts, before React commits the next paint - useOverflowContainer also returns forceUpdateOverflow, which is plumbed through OverflowContextValue - useOverflowMenu calls forceUpdateOverflow immediately after the menu element is registered, collapsing what used to be two separate layout effects - add an Overflow.paint-probe.cy.tsx test that captures the DOM at the layout effect / passive effect / raf boundaries and asserts the overflow state is already final at the first paint Co-Authored-By: Claude Opus 4.7 (1M context) --- ...tui-priority-overflow-pr3-first-paint.json | 7 + ...uentui-react-overflow-pr3-first-paint.json | 7 + .../src/overflowManager.test.ts | 19 ++ .../priority-overflow/src/overflowManager.ts | 10 + .../library/etc/react-overflow.api.md | 2 +- .../library/src/Overflow.paint-probe.cy.tsx | 269 ++++++++++++++++++ .../library/src/components/Overflow.tsx | 38 ++- .../library/src/overflowContext.ts | 2 + .../react-overflow/library/src/types.ts | 8 +- .../library/src/useOverflowContainer.test.tsx | 18 ++ .../library/src/useOverflowContainer.ts | 8 + .../library/src/useOverflowMenu.ts | 18 +- 12 files changed, 384 insertions(+), 22 deletions(-) create mode 100644 change/@fluentui-priority-overflow-pr3-first-paint.json create mode 100644 change/@fluentui-react-overflow-pr3-first-paint.json create mode 100644 packages/react-components/react-overflow/library/src/Overflow.paint-probe.cy.tsx diff --git a/change/@fluentui-priority-overflow-pr3-first-paint.json b/change/@fluentui-priority-overflow-pr3-first-paint.json new file mode 100644 index 00000000000000..13ba416d1c35ff --- /dev/null +++ b/change/@fluentui-priority-overflow-pr3-first-paint.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: attaching or detaching the overflow menu while observing now eagerly recomputes overflow so the snapshot reflects the menu's footprint before the first paint.", + "packageName": "@fluentui/priority-overflow", + "email": "bsunderhus@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/change/@fluentui-react-overflow-pr3-first-paint.json b/change/@fluentui-react-overflow-pr3-first-paint.json new file mode 100644 index 00000000000000..0f169dfabfad13 --- /dev/null +++ b/change/@fluentui-react-overflow-pr3-first-paint.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "feat: the overflow snapshot is now correct on the first paint. The container forces a synchronous overflow computation inside the same layout effect that starts observation, useOverflowMenu forces another after the menu element is registered, and useOverflowContainer exposes forceUpdateOverflow for advanced consumers.", + "packageName": "@fluentui/react-overflow", + "email": "bsunderhus@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-components/priority-overflow/src/overflowManager.test.ts b/packages/react-components/priority-overflow/src/overflowManager.test.ts index 4f3c28065a516c..1358439f309e2f 100644 --- a/packages/react-components/priority-overflow/src/overflowManager.test.ts +++ b/packages/react-components/priority-overflow/src/overflowManager.test.ts @@ -136,6 +136,25 @@ describe('overflowManager', () => { }); }); + it('should re-dispatch when the overflow menu is attached while observing', () => { + const onUpdateOverflow = jest.fn(); + const manager = createOverflowManager(createObserveOptions({ onUpdateOverflow })); + const container = createContainer(100); + const itemA = createElementWithSize('button', 60); + const itemB = createElementWithSize('button', 60); + + manager.addItem({ element: itemA, id: 'a', priority: 1 }); + manager.addItem({ element: itemB, id: 'b', priority: 0 }); + manager.observe(container); + manager.forceUpdate(); + onUpdateOverflow.mockClear(); + + const menu = createElementWithSize('button', 30); + manager.addOverflowMenu(menu); + + expect(onUpdateOverflow).toHaveBeenCalled(); + }); + it('should remove items through removeItem', () => { const manager = createOverflowManager(createObserveOptions()); const container = createContainer(100); diff --git a/packages/react-components/priority-overflow/src/overflowManager.ts b/packages/react-components/priority-overflow/src/overflowManager.ts index ceaa2d3bc78dd2..f9f8d131fb3b66 100644 --- a/packages/react-components/priority-overflow/src/overflowManager.ts +++ b/packages/react-components/priority-overflow/src/overflowManager.ts @@ -330,6 +330,11 @@ export function createOverflowManager(initialOptions: Partial = const addOverflowMenu: OverflowManager['addOverflowMenu'] = el => { overflowMenu = el; + + if (observing) { + forceDispatch = true; + update(); + } }; const addDivider: OverflowManager['addDivider'] = divider => { @@ -343,6 +348,11 @@ export function createOverflowManager(initialOptions: Partial = const removeOverflowMenu: OverflowManager['removeOverflowMenu'] = () => { overflowMenu = undefined; + + if (observing) { + forceDispatch = true; + update(); + } }; const removeDivider: OverflowManager['removeDivider'] = groupId => { diff --git a/packages/react-components/react-overflow/library/etc/react-overflow.api.md b/packages/react-components/react-overflow/library/etc/react-overflow.api.md index 41c5be9ffd4a9e..3ed40d928e0f4c 100644 --- a/packages/react-components/react-overflow/library/etc/react-overflow.api.md +++ b/packages/react-components/react-overflow/library/etc/react-overflow.api.md @@ -72,7 +72,7 @@ export function useIsOverflowItemVisible(id: string): boolean; export const useOverflowContainer: (update: OnUpdateOverflow, options: Omit) => UseOverflowContainerReturn; // @internal (undocumented) -export interface UseOverflowContainerReturn extends Pick { +export interface UseOverflowContainerReturn extends Pick { containerRef: React_2.RefObject; } diff --git a/packages/react-components/react-overflow/library/src/Overflow.paint-probe.cy.tsx b/packages/react-components/react-overflow/library/src/Overflow.paint-probe.cy.tsx new file mode 100644 index 00000000000000..e809820d4e38ea --- /dev/null +++ b/packages/react-components/react-overflow/library/src/Overflow.paint-probe.cy.tsx @@ -0,0 +1,269 @@ +import * as React from 'react'; +import { mount as mountBase } from '@fluentui/scripts-cypress'; +import { + Overflow, + OverflowItem, + useOverflowMenu, + type OverflowProps, + type OverflowItemProps, +} from '@fluentui/react-overflow'; +import type { DistributiveOmit } from '@fluentui/react-utilities'; + +// Disable StrictMode so the probe measures a single mount/commit path. +const mount = (element: React.ReactElement) => mountBase(element, { strict: false }); + +const selectors = { + container: 'data-test-container', + item: 'data-test-item', + menu: 'data-test-menu', + probe: 'data-test-paint-probe', + probePhase: 'data-test-paint-phase', +}; + +type PaintPhaseSnapshot = { + menuText: string | null; + overflowingItemIds: string[]; +}; + +const readPaintPhaseSnapshot = (): PaintPhaseSnapshot => { + const menu = document.querySelector(`[${selectors.menu}]`); + const overflowingItemIds = Array.from(document.querySelectorAll(`[${selectors.item}]`)) + .filter(item => item.getAttribute('data-overflowing') !== null) + .map(item => item.getAttribute(selectors.item) ?? ''); + + return { + menuText: menu?.textContent ?? null, + overflowingItemIds, + }; +}; + +const writePhaseSnapshot = (name: string, phase: 'layout' | 'effect' | 'raf1', snapshot: PaintPhaseSnapshot) => { + const probeRoot = document.querySelector(`[${selectors.probe}="${name}"]`); + const phaseNode = probeRoot?.querySelector(`[${selectors.probePhase}="${phase}"]`); + + if (phaseNode) { + phaseNode.textContent = JSON.stringify(snapshot); + } +}; + +const Container: React.FC<{ children?: React.ReactNode; size?: number } & Omit> = ({ + children, + size, + ...userProps +}) => { + const selector = { + [selectors.container]: '', + }; + + return ( + +
+ {children} +
+ + ); +}; + +type ItemProps = { children?: React.ReactNode; width?: number | string } & DistributiveOmit< + OverflowItemProps, + 'children' +>; + +const Item = ({ children, width, ...overflowItemProps }: ItemProps) => { + const selector = { + [selectors.item]: overflowItemProps.id, + }; + + return ( + + + + ); +}; + +const Menu = () => { + const { isOverflowing, ref, overflowCount } = useOverflowMenu(); + const selector = { + [selectors.menu]: '', + }; + + if (!isOverflowing) { + return null; + } + + return ( + + ); +}; + +const PaintPhaseProbe: React.FC<{ name: string }> = ({ name }) => { + // The probe deliberately distinguishes the layout phase from the passive effect phase, + // so it must use the non-isomorphic variant. + // eslint-disable-next-line no-restricted-properties + React.useLayoutEffect(() => { + writePhaseSnapshot(name, 'layout', readPaintPhaseSnapshot()); + }, [name]); + + React.useEffect(() => { + writePhaseSnapshot(name, 'effect', readPaintPhaseSnapshot()); + + requestAnimationFrame(() => { + writePhaseSnapshot(name, 'raf1', readPaintPhaseSnapshot()); + }); + }, [name]); + + return null; +}; + +const PaintPhaseProbeHarness: React.FC<{ name: string; children: React.ReactNode }> = ({ name, children }) => { + return ( + <> + {children} +
+ 
+        
+        
+
+ + + ); +}; + +const assertProbeConvergence = (name: string, expected: PaintPhaseSnapshot) => { + cy.get(`[${selectors.probe}="${name}"] [${selectors.probePhase}="raf1"]`).should($node => { + expect($node.text(), 'raf1 snapshot marker').not.to.equal(''); + }); + + cy.get(`[${selectors.probe}="${name}"]`).then($probe => { + const read = (phase: 'layout' | 'effect' | 'raf1') => { + const text = $probe.find(`[${selectors.probePhase}="${phase}"]`).text(); + return JSON.parse(text) as PaintPhaseSnapshot; + }; + + const layout = read('layout'); + const effect = read('effect'); + const raf1 = read('raf1'); + const debugSnapshots = `layout=${JSON.stringify(layout)} effect=${JSON.stringify(effect)} raf1=${JSON.stringify( + raf1, + )}`; + + expect(layout, `missing layout snapshot; ${debugSnapshots}`).to.exist; + expect(effect, `missing effect snapshot; ${debugSnapshots}`).to.exist; + expect(raf1, `missing first-raf snapshot; ${debugSnapshots}`).to.exist; + expect(raf1, `unexpected first-raf snapshot; ${debugSnapshots}`).to.deep.equal(expected); + }); +}; + +describe('Overflow paint probe', () => { + beforeEach(() => { + cy.viewport(700, 700); + }); + + it('is already final by first rAF on initial overflowing mount', { retries: 0 }, () => { + const mapHelper = new Array(10).fill(0).map((_, i) => i); + + mount( + + + {mapHelper.map(i => ( + + {i} + + ))} + + + , + ); + + assertProbeConvergence('initial-overflow', { + menuText: '+5', + overflowingItemIds: ['5', '6', '7', '8', '9'], + }); + }); + + it('is already final by first rAF for a slightly wider initial-overflow case', { retries: 0 }, () => { + const mapHelper = new Array(10).fill(0).map((_, i) => i); + + mount( + + + {mapHelper.map(i => ( + + {i} + + ))} + + + , + ); + + assertProbeConvergence('initial-overflow-wide', { + menuText: '+4', + overflowingItemIds: ['6', '7', '8', '9'], + }); + }); + + it('is already final by first rAF for an uneven-width initial-overflow case', { retries: 0 }, () => { + mount( + + + + Item 0 + + + Item 1 + + + Super Long Item 2 + + + 3 + + Item 4 + Item 5 + + + , + ); + + assertProbeConvergence('initial-overflow-uneven', { + menuText: '+2', + overflowingItemIds: ['4', '5'], + }); + }); + + it('is already final by first rAF when the menu never becomes visible', { retries: 0 }, () => { + const mapHelper = new Array(5).fill(0).map((_, i) => i); + + mount( + + + {mapHelper.map(i => ( + + {i} + + ))} + + + , + ); + + assertProbeConvergence('initial-no-menu', { + menuText: null, + overflowingItemIds: [], + }); + }); +}); diff --git a/packages/react-components/react-overflow/library/src/components/Overflow.tsx b/packages/react-components/react-overflow/library/src/components/Overflow.tsx index 9266ef6a554191..f16c97c0eedc06 100644 --- a/packages/react-components/react-overflow/library/src/components/Overflow.tsx +++ b/packages/react-components/react-overflow/library/src/components/Overflow.tsx @@ -61,15 +61,23 @@ export const Overflow = React.forwardRef((props: OverflowProps, ref) => { onOverflowChange?.(null, state); }; - const { containerRef, getSnapshot, subscribe, registerItem, updateOverflow, registerOverflowMenu, registerDivider } = - useOverflowContainer(update, { - overflowDirection, - overflowAxis, - padding, - minimumVisible, - hasHiddenItems, - onUpdateItemVisibility: updateVisibilityAttribute, - }); + const { + containerRef, + getSnapshot, + subscribe, + registerItem, + updateOverflow, + forceUpdateOverflow, + registerOverflowMenu, + registerDivider, + } = useOverflowContainer(update, { + overflowDirection, + overflowAxis, + padding, + minimumVisible, + hasHiddenItems, + onUpdateItemVisibility: updateVisibilityAttribute, + }); const child = getTriggerChild(children); const clonedChild = applyTriggerPropsToChildren(children, { @@ -84,13 +92,23 @@ export const Overflow = React.forwardRef((props: OverflowProps, ref) => { hasOverflow: false, registerItem, updateOverflow, + forceUpdateOverflow, registerOverflowMenu, registerDivider, containerRef, getSnapshot, subscribe, }), - [getSnapshot, subscribe, registerItem, updateOverflow, registerOverflowMenu, registerDivider, containerRef], + [ + getSnapshot, + subscribe, + registerItem, + updateOverflow, + forceUpdateOverflow, + registerOverflowMenu, + registerDivider, + containerRef, + ], ); return {clonedChild}; diff --git a/packages/react-components/react-overflow/library/src/overflowContext.ts b/packages/react-components/react-overflow/library/src/overflowContext.ts index 274e8a2a931999..35aef6771d7434 100644 --- a/packages/react-components/react-overflow/library/src/overflowContext.ts +++ b/packages/react-components/react-overflow/library/src/overflowContext.ts @@ -29,6 +29,7 @@ export interface OverflowContextValue { registerOverflowMenu: (el: HTMLElement) => () => void; registerDivider: (divider: OverflowDividerEntry) => () => void; updateOverflow: (padding?: number) => void; + forceUpdateOverflow: () => void; containerRef?: React.RefObject; getSnapshot: () => OverflowSnapshot; subscribe: (listener: () => void) => () => void; @@ -48,6 +49,7 @@ const overflowContextDefaultValue: OverflowContextValue = { groupVisibility: {}, registerItem: () => noop, updateOverflow: noop, + forceUpdateOverflow: noop, registerOverflowMenu: () => noop, registerDivider: () => noop, getSnapshot: () => EMPTY_SNAPSHOT, diff --git a/packages/react-components/react-overflow/library/src/types.ts b/packages/react-components/react-overflow/library/src/types.ts index fadd260165a5b1..ef3e75dbe31916 100644 --- a/packages/react-components/react-overflow/library/src/types.ts +++ b/packages/react-components/react-overflow/library/src/types.ts @@ -7,7 +7,13 @@ import type { OverflowContextValue } from './overflowContext'; export interface UseOverflowContainerReturn extends Pick< OverflowContextValue, - 'registerItem' | 'updateOverflow' | 'registerOverflowMenu' | 'registerDivider' | 'getSnapshot' | 'subscribe' + | 'registerItem' + | 'updateOverflow' + | 'forceUpdateOverflow' + | 'registerOverflowMenu' + | 'registerDivider' + | 'getSnapshot' + | 'subscribe' > { /** * Ref to apply to the container that will overflow diff --git a/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx b/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx index 9e24facd3a82ef..4e0810c7241583 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx +++ b/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx @@ -93,6 +93,24 @@ describe('useOverflowContainer', () => { expect(observeMock).toHaveBeenCalledWith(getByTestId('container')); }); + it('should force a synchronous overflow update immediately after observation starts', () => { + const forceUpdateMock = jest.fn(); + const observeMock = jest.fn(); + mockOverflowManager({ observe: observeMock, forceUpdate: forceUpdateMock }); + + const TestComponent: React.FC = () => { + const { containerRef } = useOverflowContainer(() => undefined, { + onUpdateItemVisibility: () => undefined, + }); + return
; + }; + + render(); + expect(forceUpdateMock).toHaveBeenCalledTimes(1); + expect(observeMock).toHaveBeenCalledTimes(1); + expect(observeMock.mock.invocationCallOrder[0]).toBeLessThan(forceUpdateMock.mock.invocationCallOrder[0]); + }); + it('should disconnect on unmount', () => { const disconnectMock = jest.fn(); const observeMock = jest.fn(); diff --git a/packages/react-components/react-overflow/library/src/useOverflowContainer.ts b/packages/react-components/react-overflow/library/src/useOverflowContainer.ts index 9a63bc8f272edd..a5530e44eed1d1 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowContainer.ts +++ b/packages/react-components/react-overflow/library/src/useOverflowContainer.ts @@ -72,6 +72,9 @@ export const useOverflowContainer = ( useIsomorphicLayoutEffect(() => { if (managerRef.current && containerRef.current) { managerRef.current.observe(containerRef.current); + // Force a synchronous overflow computation so the first painted frame already + // reflects the final visible/invisible split instead of flashing all items. + managerRef.current.forceUpdate(); return () => managerRef.current?.disconnect(); } }, []); @@ -126,11 +129,16 @@ export const useOverflowContainer = ( [], ); + const forceUpdateOverflow = React.useCallback(() => { + managerRef.current?.forceUpdate(); + }, []); + return { registerItem, registerDivider, registerOverflowMenu, updateOverflow, + forceUpdateOverflow, containerRef, getSnapshot, subscribe, diff --git a/packages/react-components/react-overflow/library/src/useOverflowMenu.ts b/packages/react-components/react-overflow/library/src/useOverflowMenu.ts index e9451c78d9178b..5cd256e505f1a3 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowMenu.ts +++ b/packages/react-components/react-overflow/library/src/useOverflowMenu.ts @@ -11,22 +11,20 @@ export function useOverflowMenu( ): { ref: React.MutableRefObject; overflowCount: number; isOverflowing: boolean } { const elementId = useId('overflow-menu', id); const overflowCount = useOverflowCount(); - const registerOverflowMenu = useOverflowContext(v => v.registerOverflowMenu); - const updateOverflow = useOverflowContext(v => v.updateOverflow); + const { registerOverflowMenu, forceUpdateOverflow } = useOverflowContext(); const ref = React.useRef(null); const isOverflowing = overflowCount > 0; useIsomorphicLayoutEffect(() => { if (ref.current) { - return registerOverflowMenu(ref.current); + const unregister = registerOverflowMenu(ref.current); + forceUpdateOverflow(); + return () => { + unregister(); + forceUpdateOverflow(); + }; } - }, [registerOverflowMenu, isOverflowing, elementId]); - - useIsomorphicLayoutEffect(() => { - if (isOverflowing) { - updateOverflow(); - } - }, [isOverflowing, updateOverflow, ref]); + }, [registerOverflowMenu, forceUpdateOverflow, isOverflowing, elementId]); return { ref, overflowCount, isOverflowing }; } From 9cc11314717f32e20bd282706b7d5b0fe47596a7 Mon Sep 17 00:00:00 2001 From: Bernardo Sunderhus Date: Thu, 28 May 2026 17:54:58 +0200 Subject: [PATCH 2/9] chore: shorten PR3 change file comments Co-Authored-By: Claude Opus 4.7 (1M context) --- change/@fluentui-priority-overflow-pr3-first-paint.json | 2 +- change/@fluentui-react-overflow-pr3-first-paint.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/change/@fluentui-priority-overflow-pr3-first-paint.json b/change/@fluentui-priority-overflow-pr3-first-paint.json index 13ba416d1c35ff..9d8cff52e75604 100644 --- a/change/@fluentui-priority-overflow-pr3-first-paint.json +++ b/change/@fluentui-priority-overflow-pr3-first-paint.json @@ -1,6 +1,6 @@ { "type": "patch", - "comment": "fix: attaching or detaching the overflow menu while observing now eagerly recomputes overflow so the snapshot reflects the menu's footprint before the first paint.", + "comment": "fix: recompute overflow eagerly when the overflow menu attaches or detaches", "packageName": "@fluentui/priority-overflow", "email": "bsunderhus@microsoft.com", "dependentChangeType": "patch" diff --git a/change/@fluentui-react-overflow-pr3-first-paint.json b/change/@fluentui-react-overflow-pr3-first-paint.json index 0f169dfabfad13..6f774e67495921 100644 --- a/change/@fluentui-react-overflow-pr3-first-paint.json +++ b/change/@fluentui-react-overflow-pr3-first-paint.json @@ -1,6 +1,6 @@ { "type": "minor", - "comment": "feat: the overflow snapshot is now correct on the first paint. The container forces a synchronous overflow computation inside the same layout effect that starts observation, useOverflowMenu forces another after the menu element is registered, and useOverflowContainer exposes forceUpdateOverflow for advanced consumers.", + "comment": "feat: produce a correct overflow snapshot before the first paint", "packageName": "@fluentui/react-overflow", "email": "bsunderhus@microsoft.com", "dependentChangeType": "patch" From 1083def9b5b241d749a1555cbcfa5fb8ad66a133 Mon Sep 17 00:00:00 2001 From: Bernardo Sunderhus Date: Fri, 29 May 2026 16:02:09 +0200 Subject: [PATCH 3/9] fix(react-overflow): guard first-paint overflow against unmeasured container MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The synchronous first-paint forceUpdate (in useOverflowContainer's observe layout effect) must only run once the container has actually been laid out. Before layout (SSR / jsdom / display:none) sizes read as 0, which the manager treats as negative available space and collapses every item — a degenerate first paint and the source of the react-charts snapshot/a11y fallout. Gate that one call on the container being measured. The guard intentionally lives at the call site rather than in the manager: forceUpdate is shared by addItem / addOverflowMenu / setOptions / the ResizeObserver, all pre-existing paths whose behavior (and downstream snapshots) must not change. Only the first-paint call is new, so only it is conditioned here. Moving the guard into the manager is left as a follow-up. Adds Overflow.firstPaint.test.tsx (localized red→green coverage) and splits the container force test into measured / unmeasured cases. Co-Authored-By: Claude Opus 4.8 --- .../components/Overflow.firstPaint.test.tsx | 75 +++++++++++++++++++ .../library/src/useOverflowContainer.test.tsx | 30 +++++++- .../library/src/useOverflowContainer.ts | 19 ++++- 3 files changed, 118 insertions(+), 6 deletions(-) create mode 100644 packages/react-components/react-overflow/library/src/components/Overflow.firstPaint.test.tsx diff --git a/packages/react-components/react-overflow/library/src/components/Overflow.firstPaint.test.tsx b/packages/react-components/react-overflow/library/src/components/Overflow.firstPaint.test.tsx new file mode 100644 index 00000000000000..5946f7663607fd --- /dev/null +++ b/packages/react-components/react-overflow/library/src/components/Overflow.firstPaint.test.tsx @@ -0,0 +1,75 @@ +import * as React from 'react'; +import { render } from '@testing-library/react'; +import { Overflow } from './Overflow'; +import { OverflowItem } from './OverflowItem'; +import { useOverflowMenu } from '../useOverflowMenu'; + +/** + * Regression coverage for first-paint correctness, localized to react-overflow + * rather than relying on downstream consumers (e.g. react-charts Legends) to + * surface it. + * + * A consumer composes Overflow as a set of OverflowItems plus a menu that only + * renders while `isOverflowing`. In jsdom there is no layout and `ResizeObserver` + * is mocked to a no-op, so element sizes report as 0 and — with the default + * padding — the available size is negative. A *completed* overflow pass therefore + * moves items into overflow and renders the menu. + * + * The container must reflect that pass on the first painted frame: the menu is + * present and the overflowing items carry `data-overflowing`. Before the + * synchronous first-paint computation this assertion fails (the items render + * un-processed and the menu never mounts), which is the flicker this guards + * against. + */ +describe('Overflow - first paint', () => { + beforeAll(() => { + // https://github.com/jsdom/jsdom/issues/3368 + global.ResizeObserver = class ResizeObserver { + public observe() { + // do nothing + } + public unobserve() { + // do nothing + } + public disconnect() { + // do nothing + } + } as unknown as typeof ResizeObserver; + }); + + const Menu: React.FC = () => { + const { isOverflowing, ref, overflowCount } = useOverflowMenu(); + + if (!isOverflowing) { + return null; + } + + return ( + + ); + }; + + it('does not collapse items before the container has been measured', () => { + const { container } = render( + +
+ {new Array(5).fill(0).map((_, i) => ( + + + + ))} + +
+ , + ); + + // With no real layout (jsdom reports 0 sizes) the manager has nothing meaningful to + // measure, so it must not prematurely hide items or surface the overflow menu. + // Forcing the overflow pass against unmeasured (zero) sizes collapses everything, + // which is the degenerate first paint we want to avoid. + expect(container.querySelector('[data-test-menu]')).toBeNull(); + expect(container.querySelectorAll('[data-overflowing]').length).toBe(0); + }); +}); diff --git a/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx b/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx index 4e0810c7241583..bf072b3c0e5260 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx +++ b/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx @@ -93,7 +93,30 @@ describe('useOverflowContainer', () => { expect(observeMock).toHaveBeenCalledWith(getByTestId('container')); }); - it('should force a synchronous overflow update immediately after observation starts', () => { + it('should force a synchronous overflow update after observation starts when the container is measured', () => { + const clientWidthSpy = jest.spyOn(HTMLElement.prototype, 'clientWidth', 'get').mockReturnValue(100); + try { + const forceUpdateMock = jest.fn(); + const observeMock = jest.fn(); + mockOverflowManager({ observe: observeMock, forceUpdate: forceUpdateMock }); + + const TestComponent: React.FC = () => { + const { containerRef } = useOverflowContainer(() => undefined, { + onUpdateItemVisibility: () => undefined, + }); + return
; + }; + + render(); + expect(forceUpdateMock).toHaveBeenCalledTimes(1); + expect(observeMock).toHaveBeenCalledTimes(1); + expect(observeMock.mock.invocationCallOrder[0]).toBeLessThan(forceUpdateMock.mock.invocationCallOrder[0]); + } finally { + clientWidthSpy.mockRestore(); + } + }); + + it('should not force an overflow update before the container has been measured', () => { const forceUpdateMock = jest.fn(); const observeMock = jest.fn(); mockOverflowManager({ observe: observeMock, forceUpdate: forceUpdateMock }); @@ -105,10 +128,11 @@ describe('useOverflowContainer', () => { return
; }; + // jsdom reports 0 sizes; the first-paint force is skipped so nothing collapses against a + // degenerate measurement. The ResizeObserver drives the first real pass instead. render(); - expect(forceUpdateMock).toHaveBeenCalledTimes(1); expect(observeMock).toHaveBeenCalledTimes(1); - expect(observeMock.mock.invocationCallOrder[0]).toBeLessThan(forceUpdateMock.mock.invocationCallOrder[0]); + expect(forceUpdateMock).not.toHaveBeenCalled(); }); it('should disconnect on unmount', () => { diff --git a/packages/react-components/react-overflow/library/src/useOverflowContainer.ts b/packages/react-components/react-overflow/library/src/useOverflowContainer.ts index a5530e44eed1d1..f89ca0c894229e 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowContainer.ts +++ b/packages/react-components/react-overflow/library/src/useOverflowContainer.ts @@ -72,9 +72,22 @@ export const useOverflowContainer = ( useIsomorphicLayoutEffect(() => { if (managerRef.current && containerRef.current) { managerRef.current.observe(containerRef.current); - // Force a synchronous overflow computation so the first painted frame already - // reflects the final visible/invisible split instead of flashing all items. - managerRef.current.forceUpdate(); + /** + * FIXME: Ideally this measurement guard would live + * inside the manager (alongside the rest of the overflow logic), + * and that is most likely where it should eventually move. + * It lives here, at the call site, only to preserve previous behavior and + * avoid regressions: `forceUpdate`(and the `update` it backs) is also + * invoked by addItem / addOverflowMenu / setOptions / ResizeObserver — + * all pre-existing paths. + * Gating it inside the manager would change those long-standing behaviors too + * (e.g. it would suppress the collapsed state some downstream snapshots already encode). + * Only this first-paint call is new, + * so for now only this call is conditioned on the container being measured. + */ + if (containerRef.current.clientWidth > 0 || containerRef.current.clientHeight > 0) { + managerRef.current.forceUpdate(); + } return () => managerRef.current?.disconnect(); } }, []); From 339c297e5ee09ed1370b13886cf942b19257ea52 Mon Sep 17 00:00:00 2001 From: Bernardo Sunderhus Date: Fri, 29 May 2026 16:12:31 +0200 Subject: [PATCH 4/9] chore: delete probe test --- .../library/src/Overflow.paint-probe.cy.tsx | 269 ------------------ 1 file changed, 269 deletions(-) delete mode 100644 packages/react-components/react-overflow/library/src/Overflow.paint-probe.cy.tsx diff --git a/packages/react-components/react-overflow/library/src/Overflow.paint-probe.cy.tsx b/packages/react-components/react-overflow/library/src/Overflow.paint-probe.cy.tsx deleted file mode 100644 index e809820d4e38ea..00000000000000 --- a/packages/react-components/react-overflow/library/src/Overflow.paint-probe.cy.tsx +++ /dev/null @@ -1,269 +0,0 @@ -import * as React from 'react'; -import { mount as mountBase } from '@fluentui/scripts-cypress'; -import { - Overflow, - OverflowItem, - useOverflowMenu, - type OverflowProps, - type OverflowItemProps, -} from '@fluentui/react-overflow'; -import type { DistributiveOmit } from '@fluentui/react-utilities'; - -// Disable StrictMode so the probe measures a single mount/commit path. -const mount = (element: React.ReactElement) => mountBase(element, { strict: false }); - -const selectors = { - container: 'data-test-container', - item: 'data-test-item', - menu: 'data-test-menu', - probe: 'data-test-paint-probe', - probePhase: 'data-test-paint-phase', -}; - -type PaintPhaseSnapshot = { - menuText: string | null; - overflowingItemIds: string[]; -}; - -const readPaintPhaseSnapshot = (): PaintPhaseSnapshot => { - const menu = document.querySelector(`[${selectors.menu}]`); - const overflowingItemIds = Array.from(document.querySelectorAll(`[${selectors.item}]`)) - .filter(item => item.getAttribute('data-overflowing') !== null) - .map(item => item.getAttribute(selectors.item) ?? ''); - - return { - menuText: menu?.textContent ?? null, - overflowingItemIds, - }; -}; - -const writePhaseSnapshot = (name: string, phase: 'layout' | 'effect' | 'raf1', snapshot: PaintPhaseSnapshot) => { - const probeRoot = document.querySelector(`[${selectors.probe}="${name}"]`); - const phaseNode = probeRoot?.querySelector(`[${selectors.probePhase}="${phase}"]`); - - if (phaseNode) { - phaseNode.textContent = JSON.stringify(snapshot); - } -}; - -const Container: React.FC<{ children?: React.ReactNode; size?: number } & Omit> = ({ - children, - size, - ...userProps -}) => { - const selector = { - [selectors.container]: '', - }; - - return ( - -
- {children} -
- - ); -}; - -type ItemProps = { children?: React.ReactNode; width?: number | string } & DistributiveOmit< - OverflowItemProps, - 'children' ->; - -const Item = ({ children, width, ...overflowItemProps }: ItemProps) => { - const selector = { - [selectors.item]: overflowItemProps.id, - }; - - return ( - - - - ); -}; - -const Menu = () => { - const { isOverflowing, ref, overflowCount } = useOverflowMenu(); - const selector = { - [selectors.menu]: '', - }; - - if (!isOverflowing) { - return null; - } - - return ( - - ); -}; - -const PaintPhaseProbe: React.FC<{ name: string }> = ({ name }) => { - // The probe deliberately distinguishes the layout phase from the passive effect phase, - // so it must use the non-isomorphic variant. - // eslint-disable-next-line no-restricted-properties - React.useLayoutEffect(() => { - writePhaseSnapshot(name, 'layout', readPaintPhaseSnapshot()); - }, [name]); - - React.useEffect(() => { - writePhaseSnapshot(name, 'effect', readPaintPhaseSnapshot()); - - requestAnimationFrame(() => { - writePhaseSnapshot(name, 'raf1', readPaintPhaseSnapshot()); - }); - }, [name]); - - return null; -}; - -const PaintPhaseProbeHarness: React.FC<{ name: string; children: React.ReactNode }> = ({ name, children }) => { - return ( - <> - {children} -
- 
-        
-        
-
- - - ); -}; - -const assertProbeConvergence = (name: string, expected: PaintPhaseSnapshot) => { - cy.get(`[${selectors.probe}="${name}"] [${selectors.probePhase}="raf1"]`).should($node => { - expect($node.text(), 'raf1 snapshot marker').not.to.equal(''); - }); - - cy.get(`[${selectors.probe}="${name}"]`).then($probe => { - const read = (phase: 'layout' | 'effect' | 'raf1') => { - const text = $probe.find(`[${selectors.probePhase}="${phase}"]`).text(); - return JSON.parse(text) as PaintPhaseSnapshot; - }; - - const layout = read('layout'); - const effect = read('effect'); - const raf1 = read('raf1'); - const debugSnapshots = `layout=${JSON.stringify(layout)} effect=${JSON.stringify(effect)} raf1=${JSON.stringify( - raf1, - )}`; - - expect(layout, `missing layout snapshot; ${debugSnapshots}`).to.exist; - expect(effect, `missing effect snapshot; ${debugSnapshots}`).to.exist; - expect(raf1, `missing first-raf snapshot; ${debugSnapshots}`).to.exist; - expect(raf1, `unexpected first-raf snapshot; ${debugSnapshots}`).to.deep.equal(expected); - }); -}; - -describe('Overflow paint probe', () => { - beforeEach(() => { - cy.viewport(700, 700); - }); - - it('is already final by first rAF on initial overflowing mount', { retries: 0 }, () => { - const mapHelper = new Array(10).fill(0).map((_, i) => i); - - mount( - - - {mapHelper.map(i => ( - - {i} - - ))} - - - , - ); - - assertProbeConvergence('initial-overflow', { - menuText: '+5', - overflowingItemIds: ['5', '6', '7', '8', '9'], - }); - }); - - it('is already final by first rAF for a slightly wider initial-overflow case', { retries: 0 }, () => { - const mapHelper = new Array(10).fill(0).map((_, i) => i); - - mount( - - - {mapHelper.map(i => ( - - {i} - - ))} - - - , - ); - - assertProbeConvergence('initial-overflow-wide', { - menuText: '+4', - overflowingItemIds: ['6', '7', '8', '9'], - }); - }); - - it('is already final by first rAF for an uneven-width initial-overflow case', { retries: 0 }, () => { - mount( - - - - Item 0 - - - Item 1 - - - Super Long Item 2 - - - 3 - - Item 4 - Item 5 - - - , - ); - - assertProbeConvergence('initial-overflow-uneven', { - menuText: '+2', - overflowingItemIds: ['4', '5'], - }); - }); - - it('is already final by first rAF when the menu never becomes visible', { retries: 0 }, () => { - const mapHelper = new Array(5).fill(0).map((_, i) => i); - - mount( - - - {mapHelper.map(i => ( - - {i} - - ))} - - - , - ); - - assertProbeConvergence('initial-no-menu', { - menuText: null, - overflowingItemIds: [], - }); - }); -}); From db73e680153d59ac125d104ba2101e4fc529b14e Mon Sep 17 00:00:00 2001 From: Bernardo Sunderhus Date: Fri, 29 May 2026 19:07:01 +0200 Subject: [PATCH 5/9] test(react-overflow): reproduce overflow menu hydration crash Adds a deterministic jsdom regression test using the real Overflow, OverflowItem and useOverflowMenu. An unconditionally mounted menu (as in the breadcrumb composition) attaches its ref on the first commit, so the menu's child layout effect runs forceUpdateOverflow() before the parent container's onUpdateOverflow useEventCallback assignment effect commits. The manager then dispatches through the not-yet-assigned callback, which throws "Cannot call an event handler while rendering" - the same failure that hangs the breadcrumb SSR hydration test. The test fails on the current branch by design: it documents the unfixed first-paint regression and will pass once the hook no longer dispatches synchronously before the container callback is ready. Co-Authored-By: Claude Opus 4.8 --- .../library/src/useOverflowMenu.test.tsx | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 packages/react-components/react-overflow/library/src/useOverflowMenu.test.tsx diff --git a/packages/react-components/react-overflow/library/src/useOverflowMenu.test.tsx b/packages/react-components/react-overflow/library/src/useOverflowMenu.test.tsx new file mode 100644 index 00000000000000..df75dc2aa83bc9 --- /dev/null +++ b/packages/react-components/react-overflow/library/src/useOverflowMenu.test.tsx @@ -0,0 +1,51 @@ +import * as React from 'react'; +import { render } from '@testing-library/react'; +import { Overflow } from './components/Overflow'; +import { OverflowItem } from './components/OverflowItem/OverflowItem'; +import { useOverflowMenu } from './useOverflowMenu'; + +/** + * Regression coverage for the breadcrumb SSR hydration failure, reproduced with the real + * components (no mocks). + * + * The breadcrumb mounts its overflow menu on the very first commit whenever there are statically + * partitioned overflow items — it is NOT gated on the measured `isOverflowing`. So the menu's `ref` + * is attached before anything has been measured. React commits layout effects child-first, so the + * menu's effect runs `forceUpdateOverflow()` synchronously BEFORE the parent `Overflow` container's + * `onUpdateOverflow` (a `useEventCallback`) assignment effect has committed. The manager then + * dispatches through that not-yet-assigned callback, whose default ref throws + * "Cannot call an event handler while rendering" — aborting hydration so `#root-provider` never + * settles and the SSR test times out. + * + * This reproduces deterministically under jsdom because `canUseDOM()` is true (so the real manager + * is created) and the menu's dispatch is not gated on layout measurement — unlike the container's + * first-paint force, which is skipped when the container reports a 0 size. + */ +const Menu: React.FC = () => { + // Mounted unconditionally, mirroring breadcrumb's OverflowMenu when there are overflow items. + const { ref } = useOverflowMenu(); + return ; +}; + +describe('useOverflowMenu', () => { + it('does not dispatch synchronously from its mount effect before the container callback is ready', () => { + expect(() => + render( + +
+ + + + + + + + + + +
+ , + ), + ).not.toThrow(); + }); +}); From b5b81410c412064371da83c1f40fa8126229b8e4 Mon Sep 17 00:00:00 2001 From: Bernardo Sunderhus Date: Sat, 30 May 2026 04:45:43 +0200 Subject: [PATCH 6/9] fix(react-overflow): resolve overflow menu hydration crash The overflow update callback was wrapped in useEventCallback, whose real implementation is assigned in a layout effect. React commits layout effects child-first, so an unconditionally mounted overflow menu (e.g. breadcrumb) ran its forceUpdateOverflow() on the first commit, before the parent container's useEventCallback assignment effect. The manager then dispatched through the not-yet-assigned callback, whose default ref threw "Cannot call an event handler while rendering", aborting hydration and hanging the breadcrumb SSR test. Pass the (now useCallback-stable) update directly as onUpdateOverflow so it is a real function at manager-creation time, removing the assignment window while keeping the synchronous first-paint force intact. setOptions keeps the manager's callback current when onOverflowChange changes. Also makes the first-paint measurement guard axis-aware so a degenerate zero-size on the overflow axis (e.g. clientHeight === 0 for vertical overflow) is not forced against, and unskips the paint-probe e2e that asserts first-paint correctness directly. Co-Authored-By: Claude Opus 4.8 --- .../library/src/Overflow.paint-probe.cy.tsx | 269 ++++++++++++++++++ .../library/src/components/Overflow.tsx | 24 +- .../library/src/useOverflowContainer.ts | 24 +- .../library/src/useOverflowMenu.test.tsx | 23 ++ 4 files changed, 319 insertions(+), 21 deletions(-) create mode 100644 packages/react-components/react-overflow/library/src/Overflow.paint-probe.cy.tsx diff --git a/packages/react-components/react-overflow/library/src/Overflow.paint-probe.cy.tsx b/packages/react-components/react-overflow/library/src/Overflow.paint-probe.cy.tsx new file mode 100644 index 00000000000000..e809820d4e38ea --- /dev/null +++ b/packages/react-components/react-overflow/library/src/Overflow.paint-probe.cy.tsx @@ -0,0 +1,269 @@ +import * as React from 'react'; +import { mount as mountBase } from '@fluentui/scripts-cypress'; +import { + Overflow, + OverflowItem, + useOverflowMenu, + type OverflowProps, + type OverflowItemProps, +} from '@fluentui/react-overflow'; +import type { DistributiveOmit } from '@fluentui/react-utilities'; + +// Disable StrictMode so the probe measures a single mount/commit path. +const mount = (element: React.ReactElement) => mountBase(element, { strict: false }); + +const selectors = { + container: 'data-test-container', + item: 'data-test-item', + menu: 'data-test-menu', + probe: 'data-test-paint-probe', + probePhase: 'data-test-paint-phase', +}; + +type PaintPhaseSnapshot = { + menuText: string | null; + overflowingItemIds: string[]; +}; + +const readPaintPhaseSnapshot = (): PaintPhaseSnapshot => { + const menu = document.querySelector(`[${selectors.menu}]`); + const overflowingItemIds = Array.from(document.querySelectorAll(`[${selectors.item}]`)) + .filter(item => item.getAttribute('data-overflowing') !== null) + .map(item => item.getAttribute(selectors.item) ?? ''); + + return { + menuText: menu?.textContent ?? null, + overflowingItemIds, + }; +}; + +const writePhaseSnapshot = (name: string, phase: 'layout' | 'effect' | 'raf1', snapshot: PaintPhaseSnapshot) => { + const probeRoot = document.querySelector(`[${selectors.probe}="${name}"]`); + const phaseNode = probeRoot?.querySelector(`[${selectors.probePhase}="${phase}"]`); + + if (phaseNode) { + phaseNode.textContent = JSON.stringify(snapshot); + } +}; + +const Container: React.FC<{ children?: React.ReactNode; size?: number } & Omit> = ({ + children, + size, + ...userProps +}) => { + const selector = { + [selectors.container]: '', + }; + + return ( + +
+ {children} +
+ + ); +}; + +type ItemProps = { children?: React.ReactNode; width?: number | string } & DistributiveOmit< + OverflowItemProps, + 'children' +>; + +const Item = ({ children, width, ...overflowItemProps }: ItemProps) => { + const selector = { + [selectors.item]: overflowItemProps.id, + }; + + return ( + + + + ); +}; + +const Menu = () => { + const { isOverflowing, ref, overflowCount } = useOverflowMenu(); + const selector = { + [selectors.menu]: '', + }; + + if (!isOverflowing) { + return null; + } + + return ( + + ); +}; + +const PaintPhaseProbe: React.FC<{ name: string }> = ({ name }) => { + // The probe deliberately distinguishes the layout phase from the passive effect phase, + // so it must use the non-isomorphic variant. + // eslint-disable-next-line no-restricted-properties + React.useLayoutEffect(() => { + writePhaseSnapshot(name, 'layout', readPaintPhaseSnapshot()); + }, [name]); + + React.useEffect(() => { + writePhaseSnapshot(name, 'effect', readPaintPhaseSnapshot()); + + requestAnimationFrame(() => { + writePhaseSnapshot(name, 'raf1', readPaintPhaseSnapshot()); + }); + }, [name]); + + return null; +}; + +const PaintPhaseProbeHarness: React.FC<{ name: string; children: React.ReactNode }> = ({ name, children }) => { + return ( + <> + {children} +
+ 
+        
+        
+
+ + + ); +}; + +const assertProbeConvergence = (name: string, expected: PaintPhaseSnapshot) => { + cy.get(`[${selectors.probe}="${name}"] [${selectors.probePhase}="raf1"]`).should($node => { + expect($node.text(), 'raf1 snapshot marker').not.to.equal(''); + }); + + cy.get(`[${selectors.probe}="${name}"]`).then($probe => { + const read = (phase: 'layout' | 'effect' | 'raf1') => { + const text = $probe.find(`[${selectors.probePhase}="${phase}"]`).text(); + return JSON.parse(text) as PaintPhaseSnapshot; + }; + + const layout = read('layout'); + const effect = read('effect'); + const raf1 = read('raf1'); + const debugSnapshots = `layout=${JSON.stringify(layout)} effect=${JSON.stringify(effect)} raf1=${JSON.stringify( + raf1, + )}`; + + expect(layout, `missing layout snapshot; ${debugSnapshots}`).to.exist; + expect(effect, `missing effect snapshot; ${debugSnapshots}`).to.exist; + expect(raf1, `missing first-raf snapshot; ${debugSnapshots}`).to.exist; + expect(raf1, `unexpected first-raf snapshot; ${debugSnapshots}`).to.deep.equal(expected); + }); +}; + +describe('Overflow paint probe', () => { + beforeEach(() => { + cy.viewport(700, 700); + }); + + it('is already final by first rAF on initial overflowing mount', { retries: 0 }, () => { + const mapHelper = new Array(10).fill(0).map((_, i) => i); + + mount( + + + {mapHelper.map(i => ( + + {i} + + ))} + + + , + ); + + assertProbeConvergence('initial-overflow', { + menuText: '+5', + overflowingItemIds: ['5', '6', '7', '8', '9'], + }); + }); + + it('is already final by first rAF for a slightly wider initial-overflow case', { retries: 0 }, () => { + const mapHelper = new Array(10).fill(0).map((_, i) => i); + + mount( + + + {mapHelper.map(i => ( + + {i} + + ))} + + + , + ); + + assertProbeConvergence('initial-overflow-wide', { + menuText: '+4', + overflowingItemIds: ['6', '7', '8', '9'], + }); + }); + + it('is already final by first rAF for an uneven-width initial-overflow case', { retries: 0 }, () => { + mount( + + + + Item 0 + + + Item 1 + + + Super Long Item 2 + + + 3 + + Item 4 + Item 5 + + + , + ); + + assertProbeConvergence('initial-overflow-uneven', { + menuText: '+2', + overflowingItemIds: ['4', '5'], + }); + }); + + it('is already final by first rAF when the menu never becomes visible', { retries: 0 }, () => { + const mapHelper = new Array(5).fill(0).map((_, i) => i); + + mount( + + + {mapHelper.map(i => ( + + {i} + + ))} + + + , + ); + + assertProbeConvergence('initial-no-menu', { + menuText: null, + overflowingItemIds: [], + }); + }); +}); diff --git a/packages/react-components/react-overflow/library/src/components/Overflow.tsx b/packages/react-components/react-overflow/library/src/components/Overflow.tsx index f16c97c0eedc06..302e026dcdcf2e 100644 --- a/packages/react-components/react-overflow/library/src/components/Overflow.tsx +++ b/packages/react-components/react-overflow/library/src/components/Overflow.tsx @@ -51,15 +51,21 @@ export const Overflow = React.forwardRef((props: OverflowProps, ref) => { hasHiddenItems, } = props; - const update: OnUpdateOverflow = data => { - const snapshot = getSnapshot(); - const state: OverflowState = { - hasOverflow: snapshot.invisibleItemCount > 0, - itemVisibility: snapshot.itemVisibility, - groupVisibility: snapshot.groupVisibility, - }; - onOverflowChange?.(null, state); - }; + const update: OnUpdateOverflow = React.useCallback( + () => { + const snapshot = getSnapshot(); + const state: OverflowState = { + hasOverflow: snapshot.invisibleItemCount > 0, + itemVisibility: snapshot.itemVisibility, + groupVisibility: snapshot.groupVisibility, + }; + onOverflowChange?.(null, state); + }, + // `getSnapshot` is a stable callback from useOverflowContainer (declared just below); omitted + // from deps to avoid a temporal-dead-zone reference. + // eslint-disable-next-line react-hooks/exhaustive-deps + [onOverflowChange], + ); const { containerRef, diff --git a/packages/react-components/react-overflow/library/src/useOverflowContainer.ts b/packages/react-components/react-overflow/library/src/useOverflowContainer.ts index f89ca0c894229e..1e7ceaf41b83f2 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowContainer.ts +++ b/packages/react-components/react-overflow/library/src/useOverflowContainer.ts @@ -37,7 +37,6 @@ export const useOverflowContainer = ( hasHiddenItems = false, } = options; - const onUpdateOverflow = useEventCallback(update); const onUpdateItemVisibilityCallback = useEventCallback(onUpdateItemVisibility); const observeOptions: Required = React.useMemo( @@ -47,18 +46,10 @@ export const useOverflowContainer = ( padding, minimumVisible, onUpdateItemVisibility: onUpdateItemVisibilityCallback, - onUpdateOverflow, + onUpdateOverflow: update, hasHiddenItems, }), - [ - minimumVisible, - onUpdateItemVisibilityCallback, - overflowAxis, - overflowDirection, - padding, - onUpdateOverflow, - hasHiddenItems, - ], + [minimumVisible, onUpdateItemVisibilityCallback, overflowAxis, overflowDirection, padding, update, hasHiddenItems], ); const containerRef = React.useRef(null); @@ -84,12 +75,21 @@ export const useOverflowContainer = ( * (e.g. it would suppress the collapsed state some downstream snapshots already encode). * Only this first-paint call is new, * so for now only this call is conditioned on the container being measured. + * + * Another problem with moving this to the manager is + * opting out of this eager behavior that ensure first paint correctness. + * TODO: expose a configuration option to disable this eager behavior */ - if (containerRef.current.clientWidth > 0 || containerRef.current.clientHeight > 0) { + const axisClientSize = + overflowAxis === 'horizontal' ? containerRef.current.clientWidth : containerRef.current.clientHeight; + if (axisClientSize > 0) { managerRef.current.forceUpdate(); } return () => managerRef.current?.disconnect(); } + // Mount-only: observe + first-paint force run once. `overflowAxis` is captured at mount; later + // changes are handled by the setOptions effect and ResizeObserver. + // eslint-disable-next-line react-hooks/exhaustive-deps }, []); useIsomorphicLayoutEffect(() => { diff --git a/packages/react-components/react-overflow/library/src/useOverflowMenu.test.tsx b/packages/react-components/react-overflow/library/src/useOverflowMenu.test.tsx index df75dc2aa83bc9..9c81111aaf0567 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowMenu.test.tsx +++ b/packages/react-components/react-overflow/library/src/useOverflowMenu.test.tsx @@ -28,6 +28,29 @@ const Menu: React.FC = () => { }; describe('useOverflowMenu', () => { + // jsdom has no ResizeObserver. The manager's observeResize() falls back to console.error when it + // is missing; once the hook is fixed the container observes and that error would pollute the run. + // Mock it to a no-op (as in Overflow.firstPaint.test.tsx) so the test stays valid post-fix. + // https://github.com/jsdom/jsdom/issues/3368 + let originalResizeObserver: typeof ResizeObserver; + beforeAll(() => { + originalResizeObserver = global.ResizeObserver; + global.ResizeObserver = class ResizeObserver { + public observe() { + // do nothing + } + public unobserve() { + // do nothing + } + public disconnect() { + // do nothing + } + } as unknown as typeof ResizeObserver; + }); + afterAll(() => { + global.ResizeObserver = originalResizeObserver; + }); + it('does not dispatch synchronously from its mount effect before the container callback is ready', () => { expect(() => render( From a03cb7911350dda2274e4b7a67910153d5e43e6c Mon Sep 17 00:00:00 2001 From: Bernardo Sunderhus Date: Sat, 30 May 2026 05:09:34 +0200 Subject: [PATCH 7/9] test(react-overflow): make paint-probe deterministic and assert convergence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The uneven-width paint-probe case used text-content widths, which depend on the host's installed fonts. On CI (fewer fonts) the text rendered narrower, so all items fit at 355px and nothing overflowed — failing the hardcoded expectation that worked locally. Replace the text widths with explicit uneven pixel widths so the overflow boundary is identical across environments. Also add a second-rAF snapshot and assert raf1 === raf2, so each case proves the first-paint value is the settled one (no drift on the next frame) rather than only matching a hardcoded number at a single instant. The layout/effect phases are intentionally not asserted equal to raf1: the overflow menu's own width reservation resolves on a second pass that only settles by the first rAF, so those phases legitimately lag. Co-Authored-By: Claude Opus 4.8 --- .../library/src/Overflow.paint-probe.cy.tsx | 50 +++++++++++++------ 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/packages/react-components/react-overflow/library/src/Overflow.paint-probe.cy.tsx b/packages/react-components/react-overflow/library/src/Overflow.paint-probe.cy.tsx index e809820d4e38ea..8a313a063af827 100644 --- a/packages/react-components/react-overflow/library/src/Overflow.paint-probe.cy.tsx +++ b/packages/react-components/react-overflow/library/src/Overflow.paint-probe.cy.tsx @@ -37,7 +37,11 @@ const readPaintPhaseSnapshot = (): PaintPhaseSnapshot => { }; }; -const writePhaseSnapshot = (name: string, phase: 'layout' | 'effect' | 'raf1', snapshot: PaintPhaseSnapshot) => { +const writePhaseSnapshot = ( + name: string, + phase: 'layout' | 'effect' | 'raf1' | 'raf2', + snapshot: PaintPhaseSnapshot, +) => { const probeRoot = document.querySelector(`[${selectors.probe}="${name}"]`); const phaseNode = probeRoot?.querySelector(`[${selectors.probePhase}="${phase}"]`); @@ -122,6 +126,11 @@ const PaintPhaseProbe: React.FC<{ name: string }> = ({ name }) => { requestAnimationFrame(() => { writePhaseSnapshot(name, 'raf1', readPaintPhaseSnapshot()); + // Second frame: used to assert the first-rAF snapshot is already settled (no drift), + // i.e. it is the converged value rather than a transient mid-convergence reading. + requestAnimationFrame(() => { + writePhaseSnapshot(name, 'raf2', readPaintPhaseSnapshot()); + }); }); }, [name]); @@ -136,6 +145,7 @@ const PaintPhaseProbeHarness: React.FC<{ name: string; children: React.ReactNode 
         
         
+
@@ -147,23 +157,24 @@ const assertProbeConvergence = (name: string, expected: PaintPhaseSnapshot) => { expect($node.text(), 'raf1 snapshot marker').not.to.equal(''); }); + cy.get(`[${selectors.probe}="${name}"] [${selectors.probePhase}="raf2"]`).should($node => { + expect($node.text(), 'raf2 snapshot marker').not.to.equal(''); + }); + cy.get(`[${selectors.probe}="${name}"]`).then($probe => { - const read = (phase: 'layout' | 'effect' | 'raf1') => { + const read = (phase: 'layout' | 'effect' | 'raf1' | 'raf2') => { const text = $probe.find(`[${selectors.probePhase}="${phase}"]`).text(); return JSON.parse(text) as PaintPhaseSnapshot; }; - const layout = read('layout'); - const effect = read('effect'); const raf1 = read('raf1'); - const debugSnapshots = `layout=${JSON.stringify(layout)} effect=${JSON.stringify(effect)} raf1=${JSON.stringify( - raf1, - )}`; + const raf2 = read('raf2'); + const debugSnapshots = `raf1=${JSON.stringify(raf1)} raf2=${JSON.stringify(raf2)}`; - expect(layout, `missing layout snapshot; ${debugSnapshots}`).to.exist; - expect(effect, `missing effect snapshot; ${debugSnapshots}`).to.exist; - expect(raf1, `missing first-raf snapshot; ${debugSnapshots}`).to.exist; + // First-paint correctness: the snapshot is already the expected final value by the first rAF. expect(raf1, `unexpected first-raf snapshot; ${debugSnapshots}`).to.deep.equal(expected); + // Convergence: the first-rAF value is settled, not a transient — it does not drift next frame. + expect(raf2, `first-raf snapshot drifted on the next frame; ${debugSnapshots}`).to.deep.equal(raf1); }); }; @@ -219,21 +230,28 @@ describe('Overflow paint probe', () => { it('is already final by first rAF for an uneven-width initial-overflow case', { retries: 0 }, () => { mount( + {/* Explicit, uneven, font-independent widths. Text-content widths vary with the host's + installed fonts (narrower on CI), shifting the overflow boundary and making the + expected snapshot non-deterministic across environments. */} - + Item 0 - + Item 1 - + Super Long Item 2 - + 3 - Item 4 - Item 5 + + Item 4 + + + Item 5 + , From 3190728b199fb9d7b417545e915257f10d6eb282 Mon Sep 17 00:00:00 2001 From: Bernardo Sunderhus Date: Sat, 30 May 2026 06:14:19 +0200 Subject: [PATCH 8/9] docs(react-overflow): explain the isOverflowing re-run trigger in useOverflowMenu `isOverflowing` is in the effect deps as a stand-in for "ref.current (re)attached" (consumers gate the menu element on it; refs can't be effect deps), not because the effect reads its value. Document this so it is not mistaken for a stray dependency and removed, which would break registering the menu on mount. Co-Authored-By: Claude Opus 4.8 --- .../react-overflow/library/src/useOverflowMenu.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/react-components/react-overflow/library/src/useOverflowMenu.ts b/packages/react-components/react-overflow/library/src/useOverflowMenu.ts index 5cd256e505f1a3..b7152090c6773f 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowMenu.ts +++ b/packages/react-components/react-overflow/library/src/useOverflowMenu.ts @@ -24,6 +24,15 @@ export function useOverflowMenu( forceUpdateOverflow(); }; } + /** + * `isOverflowing` is a re-run trigger. + * Consumers gate the menu element on it, + * so it stands in for "ref.current (re)attached". + * + * MAYBE: A callback ref would express this better, + * but that would change the public ref type and + * move logic earlier in the commit. + */ }, [registerOverflowMenu, forceUpdateOverflow, isOverflowing, elementId]); return { ref, overflowCount, isOverflowing }; From ef140f016aa626f0f7f928459edb5b2fcf64610f Mon Sep 17 00:00:00 2001 From: Bernardo Sunderhus Date: Wed, 3 Jun 2026 18:01:38 +0200 Subject: [PATCH 9/9] refactor(react-overflow): move first-paint force into manager observe Split the priority-overflow option types into `OverflowOptions` (config, used by createOverflowManager/setOptions) and `ObserveOptions` (extends `Partial` with `forceUpdate?`). The first-paint force and its measurement guard now live inside the manager's `observe` method, so `useOverflowContainer` just calls `observe(container, { forceUpdate: true })` instead of guarding and forcing at the call site. The guard reuses the manager's axis-aware `getClientSize` helper so it agrees with the actual overflow measurement on what "measured" means. Co-Authored-By: Claude Opus 4.8 --- .../etc/priority-overflow.api.md | 27 ++++--- .../priority-overflow/src/index.ts | 1 + .../src/overflowManager.test.ts | 48 ++++++++++++ .../priority-overflow/src/overflowManager.ts | 20 +++-- .../priority-overflow/src/types.ts | 12 ++- .../library/etc/react-overflow.api.md | 8 +- .../components/Overflow.firstPaint.test.tsx | 75 ------------------- .../library/src/components/Overflow.tsx | 29 +++---- .../library/src/useOverflowContainer.test.tsx | 44 +---------- .../library/src/useOverflowContainer.ts | 35 ++------- .../library/src/useOverflowMenu.ts | 13 +--- 11 files changed, 113 insertions(+), 199 deletions(-) delete mode 100644 packages/react-components/react-overflow/library/src/components/Overflow.firstPaint.test.tsx diff --git a/packages/react-components/priority-overflow/etc/priority-overflow.api.md b/packages/react-components/priority-overflow/etc/priority-overflow.api.md index a00db825616617..774a84fce5a802 100644 --- a/packages/react-components/priority-overflow/etc/priority-overflow.api.md +++ b/packages/react-components/priority-overflow/etc/priority-overflow.api.md @@ -5,20 +5,14 @@ ```ts // @internal -export function createOverflowManager(initialOptions?: Partial): OverflowManager; +export function createOverflowManager(initialOptions?: Partial): OverflowManager; // @internal export const EMPTY_SNAPSHOT: OverflowSnapshot; -// @public -export interface ObserveOptions { - hasHiddenItems?: boolean; - minimumVisible?: number; - onUpdateItemVisibility: OnUpdateItemVisibility; - onUpdateOverflow: OnUpdateOverflow; - overflowAxis?: OverflowAxis; - overflowDirection?: OverflowDirection; - padding?: number; +// @public (undocumented) +export interface ObserveOptions extends Partial { + forceUpdate?: boolean; } // @public @@ -76,11 +70,22 @@ export interface OverflowManager { removeDivider: (groupId: string) => void; removeItem: (itemId: string) => void; removeOverflowMenu: () => void; - setOptions: (options: Partial) => void; + setOptions: (options: Partial) => void; subscribe: (listener: () => void) => () => void; update: () => void; } +// @public +export interface OverflowOptions { + hasHiddenItems?: boolean; + minimumVisible?: number; + onUpdateItemVisibility: OnUpdateItemVisibility; + onUpdateOverflow: OnUpdateOverflow; + overflowAxis?: OverflowAxis; + overflowDirection?: OverflowDirection; + padding?: number; +} + // @public export interface OverflowSnapshot { groupVisibility: Record; diff --git a/packages/react-components/priority-overflow/src/index.ts b/packages/react-components/priority-overflow/src/index.ts index 31db589584a72d..d6d1108e77d2b6 100644 --- a/packages/react-components/priority-overflow/src/index.ts +++ b/packages/react-components/priority-overflow/src/index.ts @@ -2,6 +2,7 @@ export { createOverflowManager } from './overflowManager'; export { EMPTY_SNAPSHOT } from './consts'; export type { ObserveOptions, + OverflowOptions, OnUpdateItemVisibility, OnUpdateItemVisibilityPayload, OnUpdateOverflow, diff --git a/packages/react-components/priority-overflow/src/overflowManager.test.ts b/packages/react-components/priority-overflow/src/overflowManager.test.ts index 1358439f309e2f..fc489028228e31 100644 --- a/packages/react-components/priority-overflow/src/overflowManager.test.ts +++ b/packages/react-components/priority-overflow/src/overflowManager.test.ts @@ -175,4 +175,52 @@ describe('overflowManager', () => { invisibleItemCount: 0, }); }); + + it('resolves overflow synchronously when observed with forceUpdate and the container is measured', () => { + const manager = createOverflowManager(createObserveOptions()); + const container = createContainer(100); + const itemA = createElementWithSize('button', 60); + const itemB = createElementWithSize('button', 60); + const menu = createElementWithSize('button', 30); + + manager.addItem({ element: itemA, id: 'a', priority: 1 }); + manager.addItem({ element: itemB, id: 'b', priority: 0 }); + manager.addOverflowMenu(menu); + + // No manual forceUpdate(); observing with forceUpdate resolves overflow on its own. + manager.observe(container, { forceUpdate: true }); + + expect(getVisibleIds(manager)).toEqual(['a']); + expect(getInvisibleIds(manager)).toEqual(['b']); + }); + + it('does not resolve overflow on observe when forceUpdate is not requested', () => { + const manager = createOverflowManager(createObserveOptions()); + const container = createContainer(100); + const itemA = createElementWithSize('button', 60); + const itemB = createElementWithSize('button', 60); + + manager.addItem({ element: itemA, id: 'a', priority: 1 }); + manager.addItem({ element: itemB, id: 'b', priority: 0 }); + + manager.observe(container); + + // Nothing has been computed yet (the ResizeObserver is mocked to a noop). + expect(manager.getSnapshot().itemVisibility).toEqual({}); + }); + + it('does not resolve overflow on observe with forceUpdate when the container is not measured', () => { + const manager = createOverflowManager(createObserveOptions()); + const container = createContainer(0); + const itemA = createElementWithSize('button', 60); + const itemB = createElementWithSize('button', 60); + + manager.addItem({ element: itemA, id: 'a', priority: 1 }); + manager.addItem({ element: itemB, id: 'b', priority: 0 }); + + // Degenerate 0 size — the guard skips the force so nothing collapses. + manager.observe(container, { forceUpdate: true }); + + expect(manager.getSnapshot().itemVisibility).toEqual({}); + }); }); diff --git a/packages/react-components/priority-overflow/src/overflowManager.ts b/packages/react-components/priority-overflow/src/overflowManager.ts index f9f8d131fb3b66..d26a3af1f3959a 100644 --- a/packages/react-components/priority-overflow/src/overflowManager.ts +++ b/packages/react-components/priority-overflow/src/overflowManager.ts @@ -7,12 +7,12 @@ import type { OverflowGroupState, OverflowItemEntry, OverflowManager, - ObserveOptions, + OverflowOptions, OverflowDividerEntry, OverflowSnapshot, } from './types'; -const DEFAULT_OPTIONS: Required = { +const DEFAULT_OPTIONS: Required = { overflowAxis: 'horizontal', overflowDirection: 'end', padding: 10, @@ -33,7 +33,7 @@ const DEFAULT_OPTIONS: Required = { * @param initialOptions - Initial observe options. Missing values are filled with defaults. * @returns overflow manager instance */ -export function createOverflowManager(initialOptions: Partial = {}): OverflowManager { +export function createOverflowManager(initialOptions: Partial = {}): OverflowManager { // calls to `offsetWidth or offsetHeight` can happen multiple times in an update // Use a cache to avoid causing too many recalcs and avoid scripting time to meausure sizes const sizeCache = new Map(); @@ -44,7 +44,7 @@ export function createOverflowManager(initialOptions: Partial = // If true, next update will dispatch to onUpdateOverflow even if queue top states don't change // Initially true to force dispatch on first mount let forceDispatch = true; - const options: Required = { ...DEFAULT_OPTIONS, ...initialOptions }; + const options: Required = { ...DEFAULT_OPTIONS, ...initialOptions }; const overflowItems: Record = {}; const overflowDividers: Record = {}; const listeners = new Set<() => void>(); @@ -264,10 +264,10 @@ export function createOverflowManager(initialOptions: Partial = } }; - const observe: OverflowManager['observe'] = (observedContainer, userOptions) => { - if (userOptions) { - Object.assign(options, userOptions); - } + const observe: OverflowManager['observe'] = (observedContainer, observeOptions) => { + const { forceUpdate: shouldForceUpdate, ...userOptions } = observeOptions ?? {}; + Object.assign(options, userOptions); + Object.values(overflowItems).forEach(item => { if (!visibleItemQueue.contains(item.id) && !invisibleItemQueue.contains(item.id)) { visibleItemQueue.enqueue(item.id); @@ -282,6 +282,10 @@ export function createOverflowManager(initialOptions: Partial = } update(); }); + + if (shouldForceUpdate && getClientSize(observedContainer) > 0) { + forceUpdate(); + } }; const disconnect: OverflowManager['disconnect'] = () => { diff --git a/packages/react-components/priority-overflow/src/types.ts b/packages/react-components/priority-overflow/src/types.ts index 961392fcf0c352..d6c3d6de98c974 100644 --- a/packages/react-components/priority-overflow/src/types.ts +++ b/packages/react-components/priority-overflow/src/types.ts @@ -130,7 +130,7 @@ export interface OnUpdateItemVisibilityPayload { /** * Options used to initialize or reconfigure overflow observation. */ -export interface ObserveOptions { +export interface OverflowOptions { /** * Padding in pixels reserved at the end of the container before overflow occurs. * Useful for accounting for extra elements (for example an overflow menu button) @@ -172,6 +172,14 @@ export interface ObserveOptions { hasHiddenItems?: boolean; } +export interface ObserveOptions extends Partial { + /** + * forces update when observation begins, ensuring initial overflow state is correct. This is useful when the container starts with items that should be overflowed, or when the container resizes immediately after mounting. + * @default false + */ + forceUpdate?: boolean; +} + /** * Internal manager contract used to observe and compute priority overflow. * @@ -189,7 +197,7 @@ export interface OverflowManager { /** * Updates engine options without restarting observation. */ - setOptions: (options: Partial) => void; + setOptions: (options: Partial) => void; /** * Add overflow items */ diff --git a/packages/react-components/react-overflow/library/etc/react-overflow.api.md b/packages/react-components/react-overflow/library/etc/react-overflow.api.md index 3ed40d928e0f4c..d92921aa21157e 100644 --- a/packages/react-components/react-overflow/library/etc/react-overflow.api.md +++ b/packages/react-components/react-overflow/library/etc/react-overflow.api.md @@ -4,11 +4,11 @@ ```ts -import type { ObserveOptions } from '@fluentui/priority-overflow'; import type { OnUpdateOverflow } from '@fluentui/priority-overflow'; import type { OverflowDividerEntry } from '@fluentui/priority-overflow'; import type { OverflowGroupState } from '@fluentui/priority-overflow'; import type { OverflowItemEntry } from '@fluentui/priority-overflow'; +import type { OverflowOptions } from '@fluentui/priority-overflow'; import type { OverflowSnapshot } from '@fluentui/priority-overflow'; import * as React_2 from 'react'; @@ -29,7 +29,7 @@ export interface OnOverflowChangeData extends OverflowState { } // @public -export const Overflow: React_2.ForwardRefExoticComponent> & { +export const Overflow: React_2.ForwardRefExoticComponent> & { children: React_2.ReactElement; onOverflowChange?: (ev: null, data: OverflowState) => void; } & React_2.RefAttributes>; @@ -54,7 +54,7 @@ export type OverflowItemProps = { }); // @public -export type OverflowProps = Partial> & { +export type OverflowProps = Partial> & { children: React_2.ReactElement; onOverflowChange?: (ev: null, data: OverflowState) => void; }; @@ -69,7 +69,7 @@ export function useIsOverflowGroupVisible(id: string): OverflowGroupState; export function useIsOverflowItemVisible(id: string): boolean; // @internal (undocumented) -export const useOverflowContainer: (update: OnUpdateOverflow, options: Omit) => UseOverflowContainerReturn; +export const useOverflowContainer: (update: OnUpdateOverflow, options: Omit) => UseOverflowContainerReturn; // @internal (undocumented) export interface UseOverflowContainerReturn extends Pick { diff --git a/packages/react-components/react-overflow/library/src/components/Overflow.firstPaint.test.tsx b/packages/react-components/react-overflow/library/src/components/Overflow.firstPaint.test.tsx deleted file mode 100644 index 5946f7663607fd..00000000000000 --- a/packages/react-components/react-overflow/library/src/components/Overflow.firstPaint.test.tsx +++ /dev/null @@ -1,75 +0,0 @@ -import * as React from 'react'; -import { render } from '@testing-library/react'; -import { Overflow } from './Overflow'; -import { OverflowItem } from './OverflowItem'; -import { useOverflowMenu } from '../useOverflowMenu'; - -/** - * Regression coverage for first-paint correctness, localized to react-overflow - * rather than relying on downstream consumers (e.g. react-charts Legends) to - * surface it. - * - * A consumer composes Overflow as a set of OverflowItems plus a menu that only - * renders while `isOverflowing`. In jsdom there is no layout and `ResizeObserver` - * is mocked to a no-op, so element sizes report as 0 and — with the default - * padding — the available size is negative. A *completed* overflow pass therefore - * moves items into overflow and renders the menu. - * - * The container must reflect that pass on the first painted frame: the menu is - * present and the overflowing items carry `data-overflowing`. Before the - * synchronous first-paint computation this assertion fails (the items render - * un-processed and the menu never mounts), which is the flicker this guards - * against. - */ -describe('Overflow - first paint', () => { - beforeAll(() => { - // https://github.com/jsdom/jsdom/issues/3368 - global.ResizeObserver = class ResizeObserver { - public observe() { - // do nothing - } - public unobserve() { - // do nothing - } - public disconnect() { - // do nothing - } - } as unknown as typeof ResizeObserver; - }); - - const Menu: React.FC = () => { - const { isOverflowing, ref, overflowCount } = useOverflowMenu(); - - if (!isOverflowing) { - return null; - } - - return ( - - ); - }; - - it('does not collapse items before the container has been measured', () => { - const { container } = render( - -
- {new Array(5).fill(0).map((_, i) => ( - - - - ))} - -
- , - ); - - // With no real layout (jsdom reports 0 sizes) the manager has nothing meaningful to - // measure, so it must not prematurely hide items or surface the overflow menu. - // Forcing the overflow pass against unmeasured (zero) sizes collapses everything, - // which is the degenerate first paint we want to avoid. - expect(container.querySelector('[data-test-menu]')).toBeNull(); - expect(container.querySelectorAll('[data-overflowing]').length).toBe(0); - }); -}); diff --git a/packages/react-components/react-overflow/library/src/components/Overflow.tsx b/packages/react-components/react-overflow/library/src/components/Overflow.tsx index 302e026dcdcf2e..eb06960ba1a392 100644 --- a/packages/react-components/react-overflow/library/src/components/Overflow.tsx +++ b/packages/react-components/react-overflow/library/src/components/Overflow.tsx @@ -2,12 +2,13 @@ import * as React from 'react'; import { mergeClasses } from '@griffel/react'; -import type { ObserveOptions, OnUpdateOverflow, OverflowGroupState } from '@fluentui/priority-overflow'; +import type { OverflowOptions, OnUpdateOverflow, OverflowGroupState } from '@fluentui/priority-overflow'; import { applyTriggerPropsToChildren, getTriggerChild, getReactElementRef, useMergedRefs, + useEventCallback, } from '@fluentui/react-utilities'; import { OverflowContext, type OverflowContextValue } from '../overflowContext'; @@ -26,7 +27,7 @@ export interface OnOverflowChangeData extends OverflowState {} * Overflow Props */ export type OverflowProps = Partial< - Pick + Pick > & { children: React.ReactElement; @@ -51,21 +52,15 @@ export const Overflow = React.forwardRef((props: OverflowProps, ref) => { hasHiddenItems, } = props; - const update: OnUpdateOverflow = React.useCallback( - () => { - const snapshot = getSnapshot(); - const state: OverflowState = { - hasOverflow: snapshot.invisibleItemCount > 0, - itemVisibility: snapshot.itemVisibility, - groupVisibility: snapshot.groupVisibility, - }; - onOverflowChange?.(null, state); - }, - // `getSnapshot` is a stable callback from useOverflowContainer (declared just below); omitted - // from deps to avoid a temporal-dead-zone reference. - // eslint-disable-next-line react-hooks/exhaustive-deps - [onOverflowChange], - ); + const update: OnUpdateOverflow = useEventCallback(() => { + const snapshot = getSnapshot(); + const state: OverflowState = { + hasOverflow: snapshot.invisibleItemCount > 0, + itemVisibility: snapshot.itemVisibility, + groupVisibility: snapshot.groupVisibility, + }; + onOverflowChange?.(null, state); + }); const { containerRef, diff --git a/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx b/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx index bf072b3c0e5260..81aa49d69a29f7 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx +++ b/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx @@ -90,49 +90,7 @@ describe('useOverflowContainer', () => { const { getByTestId } = render(); expect(observeMock).toHaveBeenCalledTimes(1); - expect(observeMock).toHaveBeenCalledWith(getByTestId('container')); - }); - - it('should force a synchronous overflow update after observation starts when the container is measured', () => { - const clientWidthSpy = jest.spyOn(HTMLElement.prototype, 'clientWidth', 'get').mockReturnValue(100); - try { - const forceUpdateMock = jest.fn(); - const observeMock = jest.fn(); - mockOverflowManager({ observe: observeMock, forceUpdate: forceUpdateMock }); - - const TestComponent: React.FC = () => { - const { containerRef } = useOverflowContainer(() => undefined, { - onUpdateItemVisibility: () => undefined, - }); - return
; - }; - - render(); - expect(forceUpdateMock).toHaveBeenCalledTimes(1); - expect(observeMock).toHaveBeenCalledTimes(1); - expect(observeMock.mock.invocationCallOrder[0]).toBeLessThan(forceUpdateMock.mock.invocationCallOrder[0]); - } finally { - clientWidthSpy.mockRestore(); - } - }); - - it('should not force an overflow update before the container has been measured', () => { - const forceUpdateMock = jest.fn(); - const observeMock = jest.fn(); - mockOverflowManager({ observe: observeMock, forceUpdate: forceUpdateMock }); - - const TestComponent: React.FC = () => { - const { containerRef } = useOverflowContainer(() => undefined, { - onUpdateItemVisibility: () => undefined, - }); - return
; - }; - - // jsdom reports 0 sizes; the first-paint force is skipped so nothing collapses against a - // degenerate measurement. The ResizeObserver drives the first real pass instead. - render(); - expect(observeMock).toHaveBeenCalledTimes(1); - expect(forceUpdateMock).not.toHaveBeenCalled(); + expect(observeMock.mock.calls[0][0]).toBe(getByTestId('container')); }); it('should disconnect on unmount', () => { diff --git a/packages/react-components/react-overflow/library/src/useOverflowContainer.ts b/packages/react-components/react-overflow/library/src/useOverflowContainer.ts index 1e7ceaf41b83f2..77166cb4af8ee0 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowContainer.ts +++ b/packages/react-components/react-overflow/library/src/useOverflowContainer.ts @@ -12,7 +12,7 @@ import type { OverflowItemEntry, OverflowDividerEntry, OverflowManager, - ObserveOptions, + OverflowOptions, } from '@fluentui/priority-overflow'; import { canUseDOM, useEventCallback, useIsomorphicLayoutEffect } from '@fluentui/react-utilities'; import type { UseOverflowContainerReturn } from './types'; @@ -26,7 +26,7 @@ import { DATA_OVERFLOWING, DATA_OVERFLOW_DIVIDER, DATA_OVERFLOW_ITEM, DATA_OVERF */ export const useOverflowContainer = ( update: OnUpdateOverflow, - options: Omit, + options: Omit, ): UseOverflowContainerReturn => { const { overflowAxis = 'horizontal', @@ -39,7 +39,7 @@ export const useOverflowContainer = ( const onUpdateItemVisibilityCallback = useEventCallback(onUpdateItemVisibility); - const observeOptions: Required = React.useMemo( + const observeOptions: Required = React.useMemo( () => ({ overflowAxis, overflowDirection, @@ -62,34 +62,11 @@ export const useOverflowContainer = ( useIsomorphicLayoutEffect(() => { if (managerRef.current && containerRef.current) { - managerRef.current.observe(containerRef.current); - /** - * FIXME: Ideally this measurement guard would live - * inside the manager (alongside the rest of the overflow logic), - * and that is most likely where it should eventually move. - * It lives here, at the call site, only to preserve previous behavior and - * avoid regressions: `forceUpdate`(and the `update` it backs) is also - * invoked by addItem / addOverflowMenu / setOptions / ResizeObserver — - * all pre-existing paths. - * Gating it inside the manager would change those long-standing behaviors too - * (e.g. it would suppress the collapsed state some downstream snapshots already encode). - * Only this first-paint call is new, - * so for now only this call is conditioned on the container being measured. - * - * Another problem with moving this to the manager is - * opting out of this eager behavior that ensure first paint correctness. - * TODO: expose a configuration option to disable this eager behavior - */ - const axisClientSize = - overflowAxis === 'horizontal' ? containerRef.current.clientWidth : containerRef.current.clientHeight; - if (axisClientSize > 0) { - managerRef.current.forceUpdate(); - } + // forceUpdate resolves overflow synchronously for a correct first paint; the manager guards it + // on the container being measured. + managerRef.current.observe(containerRef.current, { forceUpdate: true }); return () => managerRef.current?.disconnect(); } - // Mount-only: observe + first-paint force run once. `overflowAxis` is captured at mount; later - // changes are handled by the setOptions effect and ResizeObserver. - // eslint-disable-next-line react-hooks/exhaustive-deps }, []); useIsomorphicLayoutEffect(() => { diff --git a/packages/react-components/react-overflow/library/src/useOverflowMenu.ts b/packages/react-components/react-overflow/library/src/useOverflowMenu.ts index b7152090c6773f..6be020a1c11570 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowMenu.ts +++ b/packages/react-components/react-overflow/library/src/useOverflowMenu.ts @@ -18,21 +18,14 @@ export function useOverflowMenu( useIsomorphicLayoutEffect(() => { if (ref.current) { const unregister = registerOverflowMenu(ref.current); - forceUpdateOverflow(); + if (isOverflowing) { + forceUpdateOverflow(); + } return () => { unregister(); forceUpdateOverflow(); }; } - /** - * `isOverflowing` is a re-run trigger. - * Consumers gate the menu element on it, - * so it stands in for "ref.current (re)attached". - * - * MAYBE: A callback ref would express this better, - * but that would change the public ref type and - * move logic earlier in the commit. - */ }, [registerOverflowMenu, forceUpdateOverflow, isOverflowing, elementId]); return { ref, overflowCount, isOverflowing };