Skip to content

Commit 2f9729d

Browse files
committed
chore: move force update on observe to the manager
1 parent 6e47e12 commit 2f9729d

4 files changed

Lines changed: 59 additions & 28 deletions

File tree

packages/react-components/priority-overflow/src/overflowManager.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,4 +223,40 @@ describe('overflowManager', () => {
223223

224224
expect(manager.getSnapshot().itemVisibility).toEqual({});
225225
});
226+
227+
it('applies a forceUpdate requested before observe once observation starts (deferred first paint)', () => {
228+
const manager = createOverflowManager(createObserveOptions());
229+
const container = createContainer(100);
230+
const itemA = createElementWithSize('button', 60);
231+
const itemB = createElementWithSize('button', 60);
232+
const menu = createElementWithSize('button', 30);
233+
234+
manager.addItem({ element: itemA, id: 'a', priority: 1 });
235+
manager.addItem({ element: itemB, id: 'b', priority: 0 });
236+
manager.addOverflowMenu(menu);
237+
238+
// forceUpdate before observe can't compute anything yet; the manager defers it and observe()
239+
// applies it — so overflow is resolved synchronously without passing the forceUpdate option.
240+
manager.forceUpdate();
241+
manager.observe(container);
242+
243+
expect(getVisibleIds(manager)).toEqual(['a']);
244+
expect(getInvisibleIds(manager)).toEqual(['b']);
245+
});
246+
247+
it('does not apply a deferred forceUpdate when the container is not measured', () => {
248+
const manager = createOverflowManager(createObserveOptions());
249+
const container = createContainer(0);
250+
const itemA = createElementWithSize('button', 60);
251+
const itemB = createElementWithSize('button', 60);
252+
253+
manager.addItem({ element: itemA, id: 'a', priority: 1 });
254+
manager.addItem({ element: itemB, id: 'b', priority: 0 });
255+
256+
// Degenerate 0 size — observe() skips the deferred force so nothing collapses.
257+
manager.forceUpdate();
258+
manager.observe(container);
259+
260+
expect(manager.getSnapshot().itemVisibility).toEqual({});
261+
});
226262
});

packages/react-components/priority-overflow/src/overflowManager.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export function createOverflowManager(initialOptions: Partial<OverflowOptions> =
4444
// If true, next update will dispatch to onUpdateOverflow even if queue top states don't change
4545
// Initially true to force dispatch on first mount
4646
let forceDispatch = true;
47+
let forceUpdateOnObserve = false;
4748
const options: Required<OverflowOptions> = { ...DEFAULT_OPTIONS, ...initialOptions };
4849
const overflowItems: Record<string, OverflowItemEntry> = {};
4950
const overflowDividers: Record<string, OverflowDividerEntry> = {};
@@ -236,6 +237,13 @@ export function createOverflowManager(initialOptions: Partial<OverflowOptions> =
236237
};
237238

238239
const forceUpdate: OverflowManager['forceUpdate'] = () => {
240+
if (!container) {
241+
// Not observing yet — there is nothing to measure. Remember the request and let observe()
242+
// honor it once the container is being observed (first-paint correctness).
243+
forceUpdateOnObserve = true;
244+
return;
245+
}
246+
239247
if (processOverflowItems() || forceDispatch) {
240248
forceDispatch = false;
241249
dispatchOverflowUpdate();
@@ -283,9 +291,10 @@ export function createOverflowManager(initialOptions: Partial<OverflowOptions> =
283291
update();
284292
});
285293

286-
if (shouldForceUpdate && getClientSize(observedContainer) > 0) {
294+
if ((shouldForceUpdate || forceUpdateOnObserve) && getClientSize(observedContainer) > 0) {
287295
forceUpdate();
288296
}
297+
forceUpdateOnObserve = false;
289298
};
290299

291300
const disconnect: OverflowManager['disconnect'] = () => {
@@ -298,6 +307,7 @@ export function createOverflowManager(initialOptions: Partial<OverflowOptions> =
298307
container = undefined;
299308
observing = false;
300309
forceDispatch = true;
310+
forceUpdateOnObserve = false;
301311

302312
// clear all entries
303313
Object.keys(overflowItems).forEach(itemId => removeItem(itemId));

packages/react-components/react-overflow/library/src/useOverflowContainer.ts

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -56,27 +56,17 @@ export const useOverflowContainer = <TElement extends HTMLElement>(
5656

5757
const managerRef = React.useRef<OverflowManager | null>(null);
5858

59-
// Whether the container's observe effect has run. Item/menu hooks request first-paint correctness
60-
// via `forceUpdateOverflow`; before the container observes there is nothing to compute yet, so the
61-
// request is recorded here and honored when the observe effect runs.
62-
const hasObservedRef = React.useRef(false);
63-
// Set when a descendant requests first-paint correctness before the container observes. The default
64-
// item/menu hooks make this request; a hook that omits it opts the container out of the synchronous
65-
// first-paint pass (the hot path), letting the ResizeObserver drive the first overflow pass instead.
66-
const pendingForceUpdateRef = React.useRef(false);
67-
6859
if (managerRef.current === null) {
6960
managerRef.current = canUseDOM() ? createOverflowManager(observeOptions) : null;
7061
}
7162

7263
useIsomorphicLayoutEffect(() => {
7364
if (managerRef.current && containerRef.current) {
74-
// Child item/menu effects already ran (child-before-parent), so `pendingForceUpdateRef`
75-
// reflects whether any descendant requested first-paint correctness. When requested, resolve
76-
// overflow synchronously so the first paint is correct; the manager guards the force on the
77-
// container being measured.
78-
managerRef.current.observe(containerRef.current, { forceUpdate: pendingForceUpdateRef.current });
79-
hasObservedRef.current = true;
65+
// First-paint correctness is requested by descendants (the default item/menu hooks call
66+
// forceUpdateOverflow during their layout effects, which commit before this one). The manager
67+
// honors any such pre-observe request here, guarded on the container being measured. A consumer
68+
// whose item hook omits that request opts out — the ResizeObserver drives the first pass.
69+
managerRef.current.observe(containerRef.current);
8070
return () => managerRef.current?.disconnect();
8171
}
8272
}, []);
@@ -132,14 +122,9 @@ export const useOverflowContainer = <TElement extends HTMLElement>(
132122
);
133123

134124
const forceUpdateOverflow = React.useCallback(() => {
135-
// Before the container observes, a force can't compute anything (it isn't observing yet), so
136-
// record the request and let the observe effect honor it (first-paint correctness). After
137-
// observing, force directly.
138-
if (hasObservedRef.current) {
139-
managerRef.current?.forceUpdate();
140-
} else {
141-
pendingForceUpdateRef.current = true;
142-
}
125+
// The manager handles the before/after-observe distinction: a call before observation is
126+
// recorded and applied once it observes (first-paint correctness); a call after forces directly.
127+
managerRef.current?.forceUpdate();
143128
}, []);
144129

145130
return {

packages/react-components/react-overflow/library/src/useOverflowItem.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,11 @@ export function useOverflowItem<TElement extends HTMLElement>(
4343
groupId,
4444
pinned,
4545
});
46-
// Request first-paint correctness. Before the container observes this defers a synchronous
47-
// force to the container's observe effect; after, it recomputes for the (re)registered item.
48-
// An item hook that omits this call opts the container out of the synchronous first-paint pass.
4946
forceUpdateOverflow();
50-
return unregister;
47+
return () => {
48+
unregister();
49+
forceUpdateOverflow();
50+
};
5151
}
5252
}, [id, priority, registerItem, forceUpdateOverflow, groupId, pinned]);
5353

0 commit comments

Comments
 (0)