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 0000000000000..9d8cff52e7560 --- /dev/null +++ b/change/@fluentui-priority-overflow-pr3-first-paint.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "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 new file mode 100644 index 0000000000000..6f774e6749592 --- /dev/null +++ b/change/@fluentui-react-overflow-pr3-first-paint.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "feat: produce a correct overflow snapshot before the first paint", + "packageName": "@fluentui/react-overflow", + "email": "bsunderhus@microsoft.com", + "dependentChangeType": "patch" +} 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 a00db82561661..774a84fce5a80 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 31db589584a72..d6d1108e77d2b 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 4f3c28065a516..fc489028228e3 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); @@ -156,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 ceaa2d3bc78dd..d26a3af1f3959 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'] = () => { @@ -330,6 +334,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 +352,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/priority-overflow/src/types.ts b/packages/react-components/priority-overflow/src/types.ts index 961392fcf0c35..d6c3d6de98c97 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 41c5be9ffd4a9..d92921aa21157 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,10 +69,10 @@ 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 { +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 0000000000000..8a313a063af82 --- /dev/null +++ b/packages/react-components/react-overflow/library/src/Overflow.paint-probe.cy.tsx @@ -0,0 +1,287 @@ +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' | 'raf2', + 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()); + // 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]); + + 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}"] [${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' | 'raf2') => { + const text = $probe.find(`[${selectors.probePhase}="${phase}"]`).text(); + return JSON.parse(text) as PaintPhaseSnapshot; + }; + + const raf1 = read('raf1'); + const raf2 = read('raf2'); + const debugSnapshots = `raf1=${JSON.stringify(raf1)} raf2=${JSON.stringify(raf2)}`; + + // 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); + }); +}; + +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( + + {/* 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 + + + + , + ); + + 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 9266ef6a55419..eb06960ba1a39 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,7 +52,7 @@ export const Overflow = React.forwardRef((props: OverflowProps, ref) => { hasHiddenItems, } = props; - const update: OnUpdateOverflow = data => { + const update: OnUpdateOverflow = useEventCallback(() => { const snapshot = getSnapshot(); const state: OverflowState = { hasOverflow: snapshot.invisibleItemCount > 0, @@ -59,17 +60,25 @@ export const Overflow = React.forwardRef((props: OverflowProps, ref) => { groupVisibility: snapshot.groupVisibility, }; 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 +93,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 274e8a2a93199..35aef6771d743 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 fadd260165a5b..ef3e75dbe3191 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 9e24facd3a82e..81aa49d69a29f 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx +++ b/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx @@ -90,7 +90,7 @@ describe('useOverflowContainer', () => { const { getByTestId } = render(); expect(observeMock).toHaveBeenCalledTimes(1); - expect(observeMock).toHaveBeenCalledWith(getByTestId('container')); + 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 9a63bc8f272ed..77166cb4af8ee 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', @@ -37,28 +37,19 @@ export const useOverflowContainer = ( hasHiddenItems = false, } = options; - const onUpdateOverflow = useEventCallback(update); const onUpdateItemVisibilityCallback = useEventCallback(onUpdateItemVisibility); - const observeOptions: Required = React.useMemo( + const observeOptions: Required = React.useMemo( () => ({ overflowAxis, overflowDirection, 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); @@ -71,7 +62,9 @@ export const useOverflowContainer = ( useIsomorphicLayoutEffect(() => { if (managerRef.current && containerRef.current) { - managerRef.current.observe(containerRef.current); + // 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(); } }, []); @@ -126,11 +119,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.test.tsx b/packages/react-components/react-overflow/library/src/useOverflowMenu.test.tsx new file mode 100644 index 0000000000000..9c81111aaf056 --- /dev/null +++ b/packages/react-components/react-overflow/library/src/useOverflowMenu.test.tsx @@ -0,0 +1,74 @@ +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', () => { + // 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( + +
+ + + + + + + + + + +
+ , + ), + ).not.toThrow(); + }); +}); diff --git a/packages/react-components/react-overflow/library/src/useOverflowMenu.ts b/packages/react-components/react-overflow/library/src/useOverflowMenu.ts index e9451c78d9178..6be020a1c1157 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowMenu.ts +++ b/packages/react-components/react-overflow/library/src/useOverflowMenu.ts @@ -11,22 +11,22 @@ 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); + if (isOverflowing) { + forceUpdateOverflow(); + } + return () => { + unregister(); + forceUpdateOverflow(); + }; } - }, [registerOverflowMenu, isOverflowing, elementId]); - - useIsomorphicLayoutEffect(() => { - if (isOverflowing) { - updateOverflow(); - } - }, [isOverflowing, updateOverflow, ref]); + }, [registerOverflowMenu, forceUpdateOverflow, isOverflowing, elementId]); return { ref, overflowCount, isOverflowing }; }