feat(react-overflow): allow opting out of first-paint overflow correctness#36281
Draft
bsunderhus wants to merge 6 commits into
Draft
feat(react-overflow): allow opting out of first-paint overflow correctness#36281bsunderhus wants to merge 6 commits into
bsunderhus wants to merge 6 commits into
Conversation
📊 Bundle size report
Unchanged fixtures
|
|
Pull request demo site: URL |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -0,0 +1,7 @@ | |||
| { | |||
There was a problem hiding this comment.
🕵🏾♀️ visual changes to review in the Visual Change Report
vr-tests-react-components/Charts-DonutChart 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Charts-DonutChart.Dynamic.default.chromium.png | 539 | Changed |
vr-tests-react-components/Positioning 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Positioning.Positioning end.updated 2 times.chromium.png | 621 | Changed |
| vr-tests-react-components/Positioning.Positioning end.chromium.png | 50 | Changed |
vr-tests-react-components/ProgressBar converged 3 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/ProgressBar converged.Indeterminate + thickness - Dark Mode.default.chromium.png | 67 | Changed |
| vr-tests-react-components/ProgressBar converged.Indeterminate + thickness - High Contrast.default.chromium.png | 67 | Changed |
| vr-tests-react-components/ProgressBar converged.Indeterminate + thickness.default.chromium.png | 68 | Changed |
vr-tests-react-components/TagPicker 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/TagPicker.disabled - High Contrast.disabled input hover.chromium.png | 1319 | Changed |
There were 3 duplicate changes discarded. Check the build logs for more information.
bsunderhus
force-pushed
the
react-overflow/pr4-opt-out
branch
from
3ced633 to
2f9729d
Compare
First-paint correctness is now *requested* by the default item/menu hooks: useOverflowItem and useOverflowMenu call forceUpdateOverflow on registration, which the container honors by resolving overflow synchronously in its observe layout effect (deferring the request until it observes, via child-before-parent effect ordering). A hot-path consumer that builds a custom item hook on top of useOverflowContext and omits the forceUpdateOverflow call opts the container out of the synchronous first-paint pass entirely — the ResizeObserver then drives the first (async) overflow pass instead. No new Overflow prop or public export; the opt-out is intentionally low-profile. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The cleanup now invokes the unregister function returned by registerItem, so the mocked registerItem must return one too (the real one always does). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Measure only what reaches the screen: a plain requestAnimationFrame loop, decoupled from React's render/commit/effect cycle, records a filmstrip of every painted frame. Covers all four opt-out combinations (item/menu x in/out) and asserts the stable anchors — the first painted frame and the converged final frame — leaving the timing-dependent intermediate (a transient menu-count flicker) unasserted so the test does not flake. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bsunderhus
force-pushed
the
react-overflow/pr4-opt-out
branch
from
6111196 to
9608d91
Compare
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bsunderhus
force-pushed
the
react-overflow/pr4-opt-out
branch
from
9608d91 to
71303ec
Compare
… hooks The paint-probe now drives the real OverflowItem and useOverflowMenu. Each opt-out case is expressed by wrapping the relevant subtree in a provider that overrides forceUpdateOverflow to a no-op, instead of reimplementing the hooks to drop that call. A Context.Provider renders no DOM node, so the flex layout and overflow geometry are unchanged. Same stable filmstrips, now with real component coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR 4 of 4 — DRAFT. Depends on PR 3 (#36264,
react-overflow/pr3-first-paint). Do not merge until PR 3 lands — until then the "Files changed" view shows cumulative diffs. Once PR 3 merges this branch will be rebased onto the latestmasterand the diff will reduce to just the opt-out changes.Builds on PR 3's first-paint correctness by making that synchronous pass opt-in per consumer, so hot-path clients can skip it without any
Overflowprop.Stack
react-overflow/pr1-strict-mode-lifecycle(refactor(react-overflow,priority-overflow): pure manager + strict-mode-safe lifecycle #36262) — strict-mode-safe lifecycle. Merged.react-overflow/pr2-subscribe-model(refactor(react-overflow,priority-overflow): subscribe model removes intermediate state from <Overflow> #36263) — subscribe model. Merged.react-overflow/pr3-first-paint(fix(react-overflow,priority-overflow): correct overflow snapshot on first paint #36264) — first-paint correctness. In review.How it works
First-paint correctness is now requested by the default item/menu hooks rather than forced unconditionally by the container:
useOverflowItemanduseOverflowMenucallforceUpdateOverflow()on registration.useOverflowContainerrecords that request (child effects run before the parent's observe effect) and passes it through asobserve(container, { forceUpdate: <requested> }); after observing,forceUpdateOverflowforces directly. The manager still guards the force on the container being measured.useOverflowContextand omits theforceUpdateOverflow()call opts the container out of the synchronous first-paint pass entirely — theResizeObserverthen drives the first (async) overflow pass instead.No new
Overflowprop and no new public export — the opt-out is intentionally low-profile.Test plan
Overflow.paint-probe.cy.tsxgains an opt-out case asserting the layout-phase paint is unresolved when items/menu opt out, then resolves asynchronously (5/5 passing).react-overflowunit / type-check / lint green;react-charts912 tests / 321 snapshots / 0 axe unaffected.