fix(web): matrix rain effect visibility improvements#104
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughAdds a configurable MatrixBackground component (opacity, className, paused, frameInterval), a global modal-open tracker (useAnyModalOpen, incrementModalCount, decrementModalCount), integrates matrix canvases into AppShell, Login, and modal backdrops, and updates CSS for layered bleed-through and z-indexing. Changes
Sequence DiagramsequenceDiagram
participant User
participant Modal
participant Hook as useModalOpen
participant AppShell
participant MatrixApp as MatrixBackground(App)
participant MatrixModal as MatrixBackground(Modal)
User->>Modal: open()
Modal->>Hook: incrementModalCount()
Hook->>AppShell: notify (anyModalOpen = true)
AppShell->>MatrixApp: paused = true
MatrixApp->>MatrixApp: pause loop
Modal->>MatrixModal: render(opacity=0.35, paused=false)
MatrixModal->>MatrixModal: start modal canvas animation
User->>Modal: close()
Modal->>Hook: decrementModalCount()
Hook->>AppShell: notify (anyModalOpen = false)
AppShell->>MatrixApp: paused = false
MatrixApp->>MatrixApp: resume loop
MatrixModal->>MatrixModal: unmount
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.planning/quick/010-matrix-effect-visibility/010-PLAN.md:
- Around line 216-231: The doc has inconsistent references to the main content:
`.app-main` is explicitly marked "DO NOT CHANGE" but other verification and
success criteria refer to "main area" / "main"; update the verification step and
success criteria to consistently reference the `.app-main` selector (or
alternatively change the `.app-main` note to match whichever canonical term you
choose) so all mentions of the main content use the same identifier
(`.app-main`) to avoid confusion for implementers.
In `@apps/web/src/hooks/useModalOpen.ts`:
- Around line 20-28: The forEach callbacks in incrementModalCount and
decrementModalCount implicitly return l() which trips Biome's
useIterableCallbackReturn rule; change the callbacks to use block bodies that
discard the return value (e.g., replace listeners.forEach((l) => l()) with
listeners.forEach((l) => { l(); }) in both incrementModalCount and
decrementModalCount) so the callback does not return anything.
- Around line 30-33: The hook useAnyModalOpen currently calls
useSyncExternalStore(subscribe, getSnapshot) without a server snapshot; update
the call in useAnyModalOpen to pass a third argument getServerSnapshot that
returns false for SSR safety, e.g., add a getServerSnapshot function (or
constant) that returns false and call useSyncExternalStore(subscribe,
getSnapshot, getServerSnapshot) so server renders assume no modals open and
avoid hydration mismatches.
🧹 Nitpick comments (2)
apps/web/src/components/MatrixBackground.tsx (2)
130-130:frameIntervalis captured in stale closure due to empty dependency array.
frameInterval(line 47) is read once when the effect mounts. If the prop ever changes, the animation loop keeps using the old value. Sincepausedalready uses a ref-sync pattern to avoid this, apply the same approach toframeIntervalfor consistency.Not a runtime bug today (all callers pass static values), but it's a latent correctness gap.
Proposed fix: use a ref for frameInterval
const pausedRef = useRef(paused); + const frameIntervalRef = useRef(frameInterval); // Keep pausedRef in sync without restarting the effect useEffect(() => { pausedRef.current = paused; }, [paused]); + useEffect(() => { + frameIntervalRef.current = frameInterval; + }, [frameInterval]); useEffect(() => { ... // Configuration const FONT_SIZE = 14; const COLUMN_WIDTH = 20; - const FRAME_INTERVAL = frameInterval; ... function draw(timestamp: number) { ... - if (timestamp - lastFrameTime < FRAME_INTERVAL) { + if (timestamp - lastFrameTime < frameIntervalRef.current) {
72-76: Paused canvas still schedulesrequestAnimationFrameevery frame — consider the trade-off.When paused, the draw loop continues running at ~60fps doing nothing. This is intentional for instant resume, but on low-power devices or background tabs it means the canvas never fully idles. An alternative is to stop the rAF loop when paused and restart it on unpause via the sync effect, which would be more battery-friendly.
Not blocking — the current approach is reasonable for instant resume, just flagging the trade-off.
Make matrix rain visible through header, sidebar, footer backgrounds and add it to modal backdrop overlays. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…l overlays - Add opacity/className props to MatrixBackground for reuse flexibility - Increase default canvas opacity from 0.25 to 0.5, frame rate to 60fps - Remove solid background from .app-shell so canvas shows through - Reduce header/sidebar/footer backgrounds to 50% opacity - Keep main content area at 90% opacity for readability - Make modal backdrop 50% transparent with minimal blur so matrix shows through - Convert all legacy rgba() to modern rgb() notation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add paused prop to MatrixBackground for animation coordination - Create useModalOpen hook (useSyncExternalStore-based modal counter) - Pause app-level matrix canvas when any modal opens - Render dedicated MatrixBackground inside modal backdrop (opacity 0.35) - Reduce modal backdrop to 40% opacity for visible matrix effect - Remove backdrop-filter blur that was obscuring the matrix - Only one canvas animates at a time for optimal performance Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Quick task completed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Execute small, ad-hoc tasks with GSD guarantees (atomic comm Entire-Checkpoint: 9f7fb5852875
- Add frameInterval prop to MatrixBackground (default 16ms/60fps) - Login page uses opacity=0.3, frameInterval=50ms (~20fps) for subtlety - Add .login-panel with 60% black background behind form content - Prevents matrix rain from interfering with login text readability Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Set frameInterval=50 (~20fps) for both AppShell and Modal matrix backgrounds, matching the login page speed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Bump backdrop from 40% to 70% so background content doesn't show through as much behind the matrix overlay. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
869cf45 to
d0ca89a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/styles/modal.css`:
- Around line 23-26: Rename the offending selector .matrix-canvas--modal to a
kebab-case variant (e.g., .matrix-canvas-modal) or update the stylelint
selector-class-pattern to allow BEM-style double-dash modifiers; locate the CSS
rule for .matrix-canvas--modal in apps/web/src/styles/modal.css and either
rename the class across codebase (HTML/JSX/TSX and other CSS files referencing
.matrix-canvas--modal) to .matrix-canvas-modal, or modify the stylelint config
rule selector-class-pattern to accept the BEM pattern (allowing --) so the
current class name is valid.
🧹 Nitpick comments (2)
apps/web/src/components/MatrixBackground.tsx (2)
130-130:frameIntervalis captured in the effect but missing from the dependency array.The
useEffectcloses overframeInterval(used on line 47 asFRAME_INTERVAL) but has an empty dependency array. IfframeIntervalever changes across renders, the animation loop will use the stale initial value.Since all current call sites pass static literals this won't bite today, but it's a latent bug if props become dynamic.
♻️ Proposed fix
- }, []); + }, [frameInterval]);
72-76: Paused canvas still schedules rAF callbacks (~60 Hz).When
pausedis true, the loop continues callingrequestAnimationFrameevery frame just to check the flag. This is by design for instant resume, but if multiple paused canvases coexist (app shell + login during route transitions, etc.), each burns a rAF slot doing nothing.A lighter alternative: stop the loop on pause and restart it via the
pausedsync effect. This eliminates idle CPU wake-ups entirely.♻️ Sketch: stop/start loop on pause toggle
// Keep pausedRef in sync without restarting the effect useEffect(() => { pausedRef.current = paused; + if (!paused && animationRef.current === 0) { + // Restart the loop after being fully stopped + animationRef.current = requestAnimationFrame(draw); + } }, [paused]);And in the draw function, instead of re-scheduling when paused:
if (pausedRef.current) { - animationRef.current = requestAnimationFrame(draw); + animationRef.current = 0; return; }This requires hoisting
drawor using a ref for it, so it's a larger change — flagging as optional.
| .matrix-canvas--modal { | ||
| position: absolute; | ||
| z-index: 0; | ||
| } |
There was a problem hiding this comment.
Stylelint violation: .matrix-canvas--modal doesn't match kebab-case pattern.
The project's Stylelint config enforces selector-class-pattern (kebab-case). The BEM -- modifier syntax triggers this rule. Either rename the class (e.g., .matrix-canvas-modal) or update the Stylelint config to allow BEM patterns.
🧰 Tools
🪛 Stylelint (17.2.0)
[error] 23-23: Expected class selector ".matrix-canvas--modal" to be kebab-case (selector-class-pattern)
(selector-class-pattern)
🤖 Prompt for AI Agents
In `@apps/web/src/styles/modal.css` around lines 23 - 26, Rename the offending
selector .matrix-canvas--modal to a kebab-case variant (e.g.,
.matrix-canvas-modal) or update the stylelint selector-class-pattern to allow
BEM-style double-dash modifiers; locate the CSS rule for .matrix-canvas--modal
in apps/web/src/styles/modal.css and either rename the class across codebase
(HTML/JSX/TSX and other CSS files referencing .matrix-canvas--modal) to
.matrix-canvas-modal, or modify the stylelint config rule selector-class-pattern
to accept the BEM pattern (allowing --) so the current class name is valid.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/api/src/main.ts`:
- Around line 29-31: The origin list built into allowedOrigins using
process.env.WEB_APP_URL may contain entries with leading/trailing whitespace
after .split(','); update the logic that constructs allowedOrigins to trim each
entry and remove any empty strings (e.g., map each split value with .trim() and
filter(Boolean)) so comparisons against incoming Origin headers succeed; touch
the code that assigns allowedOrigins in main.ts where process.env.WEB_APP_URL is
split.
Add dynamic origin matching for https://cipher-box-pr-<N>.onrender.com so PR preview deployments can access the API. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
418c468 to
550531e
Compare
Public client ID — embedded in frontend bundle, not a secret. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
.app-shellbackgroundTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Style