fix(GlassPanes): upgrade off flat SSR fallback after mount#26
fix(GlassPanes): upgrade off flat SSR fallback after mount#26yousefh409 wants to merge 1 commit into
Conversation
GlassPanes read reduced-motion and backdrop-filter support during render. Both are static-safe during SSR/first client render (assume reduced motion and no backdrop-filter), so on the client the panes stayed stranded on the flat, non-animated fallback: no frosting and no drift. Gate both behind a mounted flag set in an effect, so the client re-renders once with the real values and upgrades to frosted, animated glass. Also widen the drift (4% -> 13%) so the slide is actually perceptible and reduce the blur range (8-26px -> 4-12px) so panes read as crisp glass, not fog. Claude-Session: https://claude.ai/code/session_01WNP6tiqg78cXb7i23ThmQU
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| widthPct: spacing + MIN_OVERLAP_PCT + frac * OVERLAP_SPAN_PCT, | ||
| blurPx: Math.round(MIN_BLUR_PX + frac * BLUR_SPAN_PX), | ||
| driftPct: `${i % 2 === 0 ? 4 : -4}%`, | ||
| driftPct: `${i % 2 === 0 ? 13 : -13}%`, |
There was a problem hiding this comment.
🔴 Tripled pane drift breaks the full-surface coverage guarantee, exposing gaps at default settings
The pane slide distance is tripled (driftPct changed from ±4% to ±13% at GlassPanes.tsx:136) without increasing the overlap margins that absorb it, so neighboring panes can drift apart far enough to expose the background.
Impact: At the default count of 3, and at count=2, visible gaps flash between the glass panes mid-animation, breaking the documented "every point is always behind at least one pane" guarantee.
Coverage-guarantee math showing the gap
The overlap comment at src/components/GlassPanes.tsx:78 still says "drift amplitude ≈ 4% of pane width" but the drift is now 13%. The overlap constants (MIN_OVERLAP_PCT=12, OVERLAP_SPAN_PCT=8 at lines 80-81) were sized for the old 4% drift and are now too small.
CSS translate(13%, …) moves a pane by 13% of its own width. Two adjacent panes alternate direction (even → +13%, odd → −13%), so at the animation midpoint they move apart by 0.13 × width_i + 0.13 × width_j in container-width terms.
count=3 (default):
- spacing = 116/3 ≈ 38.7%, min pane width ≈ 50.7%
- Worst-case gap opening = 0.13 × 50.7 + 0.13 × 55.6 ≈ 13.8% of container
- Static overlap between adjacent panes ≈ 12%
- Net: −1.8% → a gap of ~1.8% of the container width
count=2:
- spacing = 58%, min pane width ≈ 70%
- Worst-case gap opening ≈ 18.8% of container
- Static overlap ≈ 12%
- Net: −6.8% → a gap of ~6.8% of the container width
The old ±4% drift kept the worst-case gap opening well under the 12% minimum overlap (e.g., count=2: 0.04×70×2 = 5.6% < 12%). The fix is either to reduce the drift back toward the old value, or to increase MIN_OVERLAP_PCT to at least 2 × 0.13 × max_pane_width (roughly 26% for count=2).
Prompt for agents
The drift amplitude at line 136 was changed from ±4% to ±13% of pane width, but the overlap constants MIN_OVERLAP_PCT (line 80) and OVERLAP_SPAN_PCT (line 81) were not increased to compensate. The comment at line 78 also still references the old 4% value.
The coverage guarantee requires that the static overlap between adjacent panes exceeds the maximum drift-induced gap opening. The gap opening equals 0.13 * width_pane_i + 0.13 * width_pane_j (two adjacent panes moving in opposite directions). For count=2, this is about 18.8% of container width, far exceeding the 12% minimum overlap.
Two approaches to fix:
1. Reduce drift back to a value the current overlap can absorb. The old ±4% worked. Something up to about ±5% would be safe with the current 12% minimum overlap.
2. Increase MIN_OVERLAP_PCT to absorb the new ±13% drift. For count=2 (worst case, pane width ~70%), the overlap needs to be at least 2 * 0.13 * 78 ≈ 20.3%, so MIN_OVERLAP_PCT should be at least ~24% (accounting for jitter). This would make panes significantly wider and more overlapping.
Also update the comment at line 78 to reflect whichever drift value is chosen.
Was this helpful? React with 👍 or 👎 to provide feedback.
| widthPct: spacing + MIN_OVERLAP_PCT + frac * OVERLAP_SPAN_PCT, | ||
| blurPx: Math.round(MIN_BLUR_PX + frac * BLUR_SPAN_PX), | ||
| driftPct: `${i % 2 === 0 ? 4 : -4}%`, | ||
| driftPct: `${i % 2 === 0 ? 13 : -13}%`, |
There was a problem hiding this comment.
🟡 Missing changelog entry for user-visible behavior changes
Several user-facing behaviors are changed — pane drift amplitude, blur range, and SSR/hydration rendering — but no CHANGELOG.md entry is added (CONTRIBUTING.md:43-44), so downstream consumers have no record of the visual and rendering changes.
Impact: Users upgrading the library won't know the glass-pane animation and SSR behavior changed.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const [mounted, setMounted] = useState(false); | ||
| useEffect(() => { | ||
| setMounted(true); | ||
| }, []); | ||
|
|
||
| const prefersReducedMotion = mounted ? rawPrefersReducedMotion : true; |
There was a problem hiding this comment.
🟡 Missing test updates for changed animation and SSR behavior
The PR changes drift amplitude, blur depth range, and adds a new SSR-to-client hydration path (the mounted state gate at GlassPanes.tsx:170-175), but no tests are added or updated (CONTRIBUTING.md:41), so the new behavior is untested.
Impact: Regressions in the SSR fallback upgrade path or the new drift/blur values won't be caught by CI.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const MIN_BLUR_PX = 4; | ||
| const BLUR_SPAN_PX = 8; // 4-12px |
There was a problem hiding this comment.
🔍 Blur depth range halved — significantly reduced visual depth differentiation
The blur range was reduced from 8–26px (MIN_BLUR_PX=8, BLUR_SPAN_PX=18) to 4–12px (MIN_BLUR_PX=4, BLUR_SPAN_PX=8 at src/components/GlassPanes.tsx:69-70). The component's core visual premise — documented in the header comment at lines 4-6 — is that each pane gets a DIFFERENT blur strength so overlaps read as physical depth. The old 18px span gave a 3.25× ratio between the lightest and heaviest frost (8px vs 26px); the new 8px span gives only a 3× ratio (4px vs 12px) with much less absolute difference. At 4px minimum blur, the lightest-frosted pane may not read as frosted at all on high-DPI screens. This is a deliberate visual tuning choice but worth confirming it still achieves the intended depth layering effect in practice.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const clampedSpeed = Math.max(speed, MIN_SPEED); | ||
| const clampedCount = Math.min(MAX_COUNT, Math.max(1, Math.round(count))); | ||
| // Guarded pure function (never throws, false in SSR) — same detector the | ||
| // engine's resolveMaterial uses, safe to call during render. | ||
| const frosted = supportsBackdropFilter(); | ||
| // engine's resolveMaterial uses, safe to call during render. Gate on | ||
| // `mounted` so SSR/first render stays on the flat fallback and the client | ||
| // upgrades to frosted glass after mount. | ||
| const frosted = mounted && supportsBackdropFilter(); |
There was a problem hiding this comment.
🔍 Extra render cycle from mounted-state hydration gate
The useState(false) + useEffect(() => setMounted(true), []) pattern at lines 170–173 guarantees a second render on every mount. During the first render, prefersReducedMotion is forced to true and frosted is forced to false, so the initial DOM has animation-name: none and no backdrop-filter. The post-mount render then flips both to their real values, causing a visual flash from flat/static to frosted/animated. This is a standard SSR hydration-safety pattern and is intentional per the comment, but consumers may notice a brief frame of the fallback appearance on client-side-only apps where SSR safety isn't needed. Other components in this repo (e.g., MeshGradient, Silk) do NOT use this pattern — they call supportsBackdropFilter() and usePrefersReducedMotion() directly during render without a mounted gate — so this creates an inconsistency across the ambient background family.
(Refers to lines 170-188)
Was this helpful? React with 👍 or 👎 to provide feedback.
| const panes = useMemo( | ||
| () => layoutPanes(colors, clampedCount, clampedSpeed), | ||
| [colors, clampedCount, clampedSpeed] |
There was a problem hiding this comment.
📝 Info: useMemo dependency on colors array identity may cause unnecessary recalculations after mount
The useMemo at line 189–192 depends on colors by reference identity. Since the mounted-state pattern now forces a second render immediately after mount, if the parent passes a new array literal on each render (e.g., <GlassPanes colors={['red', 'blue']} />), the pane layout will be recomputed on the post-mount render even though the colors haven't semantically changed. This is a pre-existing concern (array identity instability is always a risk with inline literals), but the extra render from the mounted gate makes it fire one more time than before. The computation in layoutPanes is lightweight so the performance impact is negligible, but it's worth noting for consistency with the "byte-identical pane styles" determinism promise in the header comment (line 30–31) — the styles ARE identical, just computed twice.
(Refers to lines 189-192)
Was this helpful? React with 👍 or 👎 to provide feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1014fa9b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| widthPct: spacing + MIN_OVERLAP_PCT + frac * OVERLAP_SPAN_PCT, | ||
| blurPx: Math.round(MIN_BLUR_PX + frac * BLUR_SPAN_PX), | ||
| driftPct: `${i % 2 === 0 ? 4 : -4}%`, | ||
| driftPct: `${i % 2 === 0 ? 13 : -13}%`, |
There was a problem hiding this comment.
When consumers pass count={1}, this new 13% translate moves the only pane by 13% of its 128% width at the 50% keyframe; with the existing layout its interval shifts from [-16%, 112%] to about [0.64%, 128.64%], leaving a visible uncovered strip on the left edge. The component documents and tests that counts are clamped to 1–8 and should tile the full surface, so the widened drift needs matching bleed/width or a one-pane exception.
Useful? React with 👍 / 👎.
1 participant
GlassPanes evaluated reduced-motion and
backdrop-filtersupport during render. Both are static-safe during SSR and the first client render (assume reduced motion + no backdrop-filter), and neither reliably re-evaluated after mount — so in the browser the panes stayed stuck on the flat, non-animated fallback: no frosting, no drift (they looked like a foggy static wash).Fix: gate both behind a
mountedflag set in an effect, so the client re-renders once with the real values and upgrades to frosted, animated glass. Also widened the slide drift (±4% → ±13%) so motion is actually perceptible, and reduced the blur range (8–26px → 4–12px) so panes read as crisp glass rather than fog.Verified in a real browser (Next 16 / React 19): panes now render frosted (
data-fallback=false, backdrop-filter present) and drift visibly.https://claude.ai/code/session_01WNP6tiqg78cXb7i23ThmQU
Summary by cubic
Fixes GlassPanes staying on the flat SSR fallback by re-checking reduced-motion and
backdrop-filterafter mount. Also adjusts drift and blur so panes read as crisp, moving glass.backdrop-filtersupport after mount via amountedflag, upgrading from the SSR-safe flat fallback to frosted, animated panes when supported.Written for commit d1014fa. Summary will update on new commits.