fix: stop the outline's first-paint correction animating, and desensitise the FOUC control - #141
fix: stop the outline's first-paint correction animating, and desensitise the FOUC control#141mmcky wants to merge 1 commit into
Conversation
|
🎭 Visual regression resultsDetails
Skipped testsmobile-chrome › theme.spec.ts › QuantEcon theme — visual regression › launch-colab |
There was a problem hiding this comment.
Pull request overview
This PR addresses first-paint (pre-app.css) corrections that were unintentionally animating, and hardens the WebKit FOUC guard test so its control case isn’t racy due to known hydration failure/recovery behavior.
Changes:
- Adds a shared
useMountedhook and uses it to withhold transition classes until after mount inContentsSidebar,SidebarToggle, andBackToTop. - Narrows
SidebarToggle’s transition fromtransition-allto an explicit property list (opacity,transform,color) to avoid unintended first-paint animations. - Updates the WebKit FOUC guard to sample state via an init script at
DOMContentLoaded, and adds explicit precondition assertions for the control strip.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
tests/visual/fouc.spec.ts |
Moves sampling into an init script and adds control precondition probes to reduce flakiness under hydration recovery. |
CHANGELOG.md |
Documents the first-paint transition fixes and the FOUC test hardening. |
app/hooks/useMounted.tsx |
Introduces shared “mounted” gating hook for deferring transitions until after mount. |
app/components/toolbar/SidebarToggle.tsx |
Defers icon transitions until after mount and narrows transition properties to avoid first-paint fade-outs. |
app/components/Outline.tsx |
Defers BackToTop opacity transition until after mount to avoid first-paint fade correction. |
app/components/ContentsSidebar.tsx |
Replaces inline hook with shared useMounted import. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Rescoped following review of #144: the #127 half (toggle icons animating their correction) is made obsolete there, so this PR is reduced to the |
d639383 to
76d1ebd
Compare
… control Two halves of the same first-paint family. the contents panel; the button that drives it has the identical shape and was untouched. Both lucide icons carry `transition-all` with visibility driven by opacity, and on the pre-app.css frame none of `absolute`, `opacity-0` or `opacity-100` exists while lucide still emits real width/height attributes — so both paint in flow at full opacity. When the stylesheet lands `position` snaps, since it is not animatable, but `opacity` 1 -> 0 animates: a close icon fading out of the toolbar on every static-build navigation. "Back to top" in the page margin has the same shape. Both now withhold the transition until after mount, which makes the correction instant and reserves the animation for real state changes. The toggle's `transition-all` is narrowed to opacity, transform and colour — the three that were ever meant to move; colour is kept explicitly so the icons still ease across a dark-mode toggle rather than snapping. `useMounted` is hoisted out of ContentsSidebar into app/hooks/ so all three sites share one definition. (React #418/#423) and the recovery re-render restores the inlined critical <style> around 195ms, after the control has stripped it from the served HTML precisely to prove the guard is meaningful. The measurement ran from the test after domcontentloaded, roughly a 150ms budget, so on a loaded runner all three control assertions flipped at once and a PR- gating job went red for no real reason. It now samples from an init script that fires in-page on DOMContentLoaded, before hydration is even scheduled. This survives a fixed #126 as well: stripping the inline block guarantees a hydration mismatch by construction, so no repair to the hydration failure itself could make a post-hydration sample safe here. Both cases also assert their own preconditions — that the strip actually matched something, and whether the inline block is present as sampled — so a reshaped CRITICAL_CSS reports as a stale strip pattern rather than as three confusing failures about grid layout. Verified the guard still guards: removing the grid rule from CRITICAL_CSS fails the main test with `Expected "grid", Received "block"`, and the control still passes. Settled-state pixels are unchanged — the full visual suite passes against untouched baselines. The hydration failure itself (#126 step 1) stays open. Closes #127 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
76d1ebd to
c18831c
Compare
|
Rebased onto current Worth explaining, because git reported the merge as clean either way. When this branch was last rebased, its entries sat under That would have been invisible and permanent: v2.3.1's release notes were generated at tag time and would never gain these lines, while the next release would ship both fixes with no changelog entry at all. It is the same failure #83 hit against the tagged Both bullets now sit under No code changes in this push. General note for the remaining stacked PRs: now that |
|
Closing in favour of #168 and #169, after re-measuring this against current The The FOUC-control half is still right and is carried onto the current probes ( Also noting for the record: all five files here conflict with |
BackToTop is hidden by opacity-0 and carries transition-opacity, so on any frame where the stylesheet is absent it paints at full opacity and fades out once the sheet lands. That frame is produced by the React #423 hydration recovery (#126), which re-renders the head and briefly drops the stylesheet after the component has mounted — so gating the transition on mount, as #141 tried, cannot prevent it. Measured in WebKit with the stylesheet delayed: 17-18 animating frames without this rule, none with it. The FOUC guard asserts the rule: the main test expects opacity 0, the control expects 1 with the inline block stripped. Supersedes the Outline.tsx change in #141. Relates to #126. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…econditions (#169) * fix: pin "back to top" to opacity 0 in the critical CSS BackToTop is hidden by opacity-0 and carries transition-opacity, so on any frame where the stylesheet is absent it paints at full opacity and fades out once the sheet lands. That frame is produced by the React #423 hydration recovery (#126), which re-renders the head and briefly drops the stylesheet after the component has mounted — so gating the transition on mount, as #141 tried, cannot prevent it. Measured in WebKit with the stylesheet delayed: 17-18 animating frames without this rule, none with it. The FOUC guard asserts the rule: the main test expects opacity 0, the control expects 1 with the inline block stripped. Supersedes the Outline.tsx change in #141. Relates to #126. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(fouc): sample first paint from an init script, assert control preconditions The React #423 hydration recovery restores the inlined critical <style> 150-240ms after DOMContentLoaded (#126). The control strips that block from the served HTML, so a post-domcontentloaded sample can see it restored and every control assertion flips at once. Sample in-page on DOMContentLoaded instead, before hydration is scheduled, and have both cases assert their own preconditions so a reshaped CRITICAL_CSS reports as a stale strip pattern. Carries the test half of #141 onto the post-#144 probes. Relates to #126. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Two pieces of the same first-paint family, shipped together because they share a mechanism.
Important
Rescoped after #144. This PR originally also fixed the contents toggle icons animating their own first-paint correction (#127). #144 rebuilds the drawer on the Popover API and swaps the icons with
displayoff:popover-open, so there is notransition-all, nouseMountedand nothing to correct — that half is obsolete. Once #144 lands this branch will be rebased to drop theSidebarToggle.tsx/ContentsSidebar.tsxchanges. The two remaining pieces below are untouched by #144 and still wanted.Outline/BackToTopanimates its first-paint correctionOutline.BackToToprenders with a transition and an opacity-driven visibility, so on the pre-app.cssframe of a static-build load it paints at full opacity and then fades out once the stylesheet lands. It now withholds the transition until after mount — the correction becomes instant, and the animation is reserved for real state changes.useMountedmoves intoapp/hooks/useMounted.tsx; after the rebaseOutlinewill be its only consumer, which is still worth a shared hook given the drawer used to need it too and future components may.Note
The toolbar is broadly unstyled on that frame anyway (
fixed top-0 h-[50px]is not inCRITICAL_CSS), so the fade was the tail of a larger snap. Whether to extend the critical CSS to cover the toolbar is a bigger, separate question and is not touched here.#126 step 2 — the FOUC control is racy
Hydration currently fails on every page load (React
#418/#423), and the#423recovery re-renders the tree, which puts the inlined critical<style>back — around 195ms in. The control test strips that block from the served HTML precisely to prove the guard is meaningful, so when React restores it, all three control assertions flip at once. The measurement ran from the test afterdomcontentloaded, leaving roughly a 150ms budget; on a loaded CI runner that is a PR-gating job going red for reasons that have nothing to do with the critical CSS.The measurement now happens in an init script that fires in-page on
DOMContentLoaded, before hydration is even scheduled. Chosen over the "bail early" alternative because a control that strips the inline block guarantees a hydration mismatch by construction — so no repair to #126 step 1 could ever make a post-hydration sample safe here. The ordering is empirical rather than guaranteed: Remix v1 emits its entry as an inlinetype="module" asyncscript whose imports must resolve beforeentry.client.tsxeven schedulesrequestIdleCallback/setTimeoutforhydrateRoot, and WebKit dispatchesDOMContentLoadedat end of parse. The earliest restoration observed in #126 is an order of magnitude later than the sample.A cheap version of the bail is folded in anyway: both cases assert their own preconditions. The strip now reports whether it actually matched anything, rather than the test inferring it from the same marker the strip uses — otherwise a reshaped
CRITICAL_CSSwould make the strip a silent no-op and the control would fail with "expected block, received grid", pointing at the critical CSS rather than at the stale pattern actually at fault.Only step 2. Root-causing the hydration mismatch is the real work and #126 stays open for it.
Note that #144 also rewrites
tests/visual/fouc.spec.ts(the drawer assertions become "paints nothing in both cases"), so the init-script change here will need to be re-applied on top of that version of the file.Verification (pre-rescope; to be re-run after the rebase)
The important one: the guard still guards. Removing the grid rule from
CRITICAL_CSSand rebuilding fails the main test withExpected: "grid" / Received: "block", while the control still passes — so the hardening desensitised it to hydration without desensitising it to regressions.Otherwise:
npm run compileclean ·test:fouc2 passed ·test:unit13/13 · full visual suite 19 passed, 0 failed with baselines untouched.Relates to #126 and #127 (the #127 fix itself now lands via #144).
🤖 Generated with Claude Code