Fix/load time improved#315
Open
sidhacks wants to merge 3 commits into
Open
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
👋 Thanks for opening a PR, @sidhacks!Your PR has entered the 🚦 PR Review Pipeline.
What happens next
A pipeline status comment will appear below and update automatically as your PR progresses. While you wait
This comment is posted only once. |
WalkthroughThis PR implements device-aware performance optimization and animation refinement across the landing page and UI components. Low-end device detection triggers shader iteration reduction and frame throttling. Touch devices automatically disable expensive glow effects. Animation timings are streamlined, and the commits grid is refactored to precompute cell state, eliminating per-render randomness. ChangesDevice-Aware Rendering Optimization
Animation Refinements & UI Polish
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/ui/animated-shader-background.tsx`:
- Around line 21-24: The component currently starts the WebGL RAF animation
unconditionally; check window.matchMedia('(prefers-reduced-motion:
reduce)').matches early (e.g., inside the AnimatedShaderBackground mount/setup)
and, if true, skip creating the RAF loop and heavy shader setup and instead
render a single static frame or don't mount the shader; apply this gating both
where lowEnd/pixelRatio/numIterations are computed and in the code that starts
requestAnimationFrame (and any RAF start in the 117-135 block), ensuring you
bail out before allocating GL resources and avoid calling requestAnimationFrame
or starting the animation loop when prefers-reduced-motion is set.
- Around line 22-29: The shader's iResolution must reflect backing-buffer pixels
(devicePixelRatio) not CSS pixels: after you call
renderer.setPixelRatio(pixelRatio) and renderer.setSize(...), compute and pass
iResolution as (window.innerWidth * pixelRatio, window.innerHeight * pixelRatio)
to the shader uniform used by the fragment shader (update the code where
iResolution is set/updated), and do the same on window resize—use the pixelRatio
variable (from pixelRatio = lowEnd ? 0.5 : Math.min(window.devicePixelRatio,
1.5)) when constructing the THREE.Vector2 or uniform value so gl_FragCoord and
iResolution are in the same coordinate space.
In `@components/ui/commits-grid.tsx`:
- Around line 84-88: cellData is memoized with React.useMemo calling
buildCellData(highlightedSet, total) but the deps array only includes [total],
causing stale cells when highlightedSet changes; update the React.useMemo call
for cellData to include highlightedSet in the dependency array (e.g.,
[highlightedSet, total]) and remove the eslint-disable-next-line comment so the
hook lint rule correctly enforces dependencies for buildCellData/highlightedSet.
In `@components/ui/glowing-effect.tsx`:
- Around line 7-8: GlowingEffect currently calls isTouchDevice() during render
which returns false on the server and can flip on first client render; change
touch detection to client state: remove the render-time isTouchDevice() call and
instead add a useState e.g. const [isTouchDevice, setIsTouchDevice] =
useState(false) and set it in a useEffect that runs window.matchMedia("(hover:
none)").matches (or use addEventListener for changes); update
effectivelyDisabled to depend on this state (the existing effectivelyDisabled
variable in GlowingEffect) so initial SSR markup is deterministic, and ensure
any class toggles that use effectivelyDisabled (the !block/!hidden logic) use
the state value rather than calling isTouchDevice() directly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4f6ac891-0ac5-4767-9b96-d9d68b8f9f76
📒 Files selected for processing (6)
app/globals.csscomponents/marketing/home-page-client.tsxcomponents/ui/animated-shader-background.tsxcomponents/ui/commits-grid.tsxcomponents/ui/glowing-effect.tsxmodules/dashboard/components/template-selecting-modal.tsx
Comment on lines
+21
to
+24
| const lowEnd = isLowEnd(); | ||
| const pixelRatio = lowEnd ? 0.5 : Math.min(window.devicePixelRatio, 1.5); | ||
| const numIterations = lowEnd ? 18 : 35; | ||
|
|
There was a problem hiding this comment.
Honor prefers-reduced-motion before starting the RAF loop.
This still runs the WebGL animation continuously for users who request reduced motion, so the page keeps its heaviest motion effect alive in the mode that should degrade gracefully. Gate the animation setup on prefers-reduced-motion and render a static frame or skip mounting the shader in that case.
Also applies to: 117-135
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/ui/animated-shader-background.tsx` around lines 21 - 24, The
component currently starts the WebGL RAF animation unconditionally; check
window.matchMedia('(prefers-reduced-motion: reduce)').matches early (e.g.,
inside the AnimatedShaderBackground mount/setup) and, if true, skip creating the
RAF loop and heavy shader setup and instead render a single static frame or
don't mount the shader; apply this gating both where
lowEnd/pixelRatio/numIterations are computed and in the code that starts
requestAnimationFrame (and any RAF start in the 117-135 block), ensuring you
bail out before allocating GL resources and avoid calling requestAnimationFrame
or starting the animation loop when prefers-reduced-motion is set.
Comment on lines
+22
to
29
| const pixelRatio = lowEnd ? 0.5 : Math.min(window.devicePixelRatio, 1.5); | ||
| const numIterations = lowEnd ? 18 : 35; | ||
|
|
||
| const scene = new THREE.Scene(); | ||
| const camera = new THREE.OrthographicCamera(-1, 1, 1, -1, 0, 1); | ||
| const renderer = new THREE.WebGLRenderer({ antialias: true }); | ||
| const renderer = new THREE.WebGLRenderer({ antialias: !lowEnd, powerPreference: "low-power" }); | ||
| renderer.setPixelRatio(pixelRatio); | ||
| renderer.setSize(window.innerWidth, window.innerHeight); |
There was a problem hiding this comment.
Keep iResolution aligned with the actual render buffer size.
After setPixelRatio(...), gl_FragCoord is measured in backing-buffer pixels but iResolution still tracks CSS pixels. That changes the shader coordinate space on low-end and high-DPR devices, so the aurora zoom/stretch varies with DPR instead of just getting cheaper to render.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/ui/animated-shader-background.tsx` around lines 22 - 29, The
shader's iResolution must reflect backing-buffer pixels (devicePixelRatio) not
CSS pixels: after you call renderer.setPixelRatio(pixelRatio) and
renderer.setSize(...), compute and pass iResolution as (window.innerWidth *
pixelRatio, window.innerHeight * pixelRatio) to the shader uniform used by the
fragment shader (update the code where iResolution is set/updated), and do the
same on window resize—use the pixelRatio variable (from pixelRatio = lowEnd ?
0.5 : Math.min(window.devicePixelRatio, 1.5)) when constructing the
THREE.Vector2 or uniform value so gl_FragCoord and iResolution are in the same
coordinate space.
Comment on lines
+84
to
+88
| const cellData = React.useMemo( | ||
| () => buildCellData(highlightedSet, total), | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| [total] | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the memo dependency currently excludes highlightedSet in CommitsGrid
rg -n -C3 'const cellData = React\.useMemo|react-hooks/exhaustive-deps|\[total\]' components/ui/commits-grid.tsxRepository: piyushdotcomm/Editron
Length of output: 415
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show more context around highlightedSet, cellData, and buildCellData usage/definition
sed -n '60,140p' components/ui/commits-grid.tsx
# Find and print buildCellData definition block (function or const) if present
rg -n "buildCellData" components/ui/commits-grid.tsxRepository: piyushdotcomm/Editron
Length of output: 3274
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate exact line for buildCellData
rg -n "buildCellData" components/ui/commits-grid.tsx
# Print buildCellData implementation
line=$(rg -n "function buildCellData" components/ui/commits-grid.tsx | head -n1 | cut -d: -f1)
start=$((line-20))
end=$((line+120))
sed -n "${start},${end}p" components/ui/commits-grid.tsx
# Locate and print generateHighlightedCells to see how width/height (thus total) depends on text
rg -n "generateHighlightedCells" components/ui/commits-grid.tsx
line2=$(rg -n "function generateHighlightedCells|const generateHighlightedCells|generateHighlightedCells\s*=\s*\(" components/ui/commits-grid.tsx | head -n1 | cut -d: -f1 || true)
if [[ -n "${line2}" ]]; then
start2=$((line2-20))
end2=$((line2+140))
sed -n "${start2},${end2}p" components/ui/commits-grid.tsx
fiRepository: piyushdotcomm/Editron
Length of output: 2033
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print the beginning of the file (where buildCellData should live) with line numbers
nl -ba components/ui/commits-grid.tsx | sed -n '1,220p'
# Also show the specific memo section again with a bit more context
nl -ba components/ui/commits-grid.tsx | sed -n '60,120p'Repository: piyushdotcomm/Editron
Length of output: 108
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="components/ui/commits-grid.tsx"
bline=$(rg -n "function buildCellData" "$file" | head -n1 | cut -d: -f1)
gline=$(rg -n "function generateHighlightedCells|const generateHighlightedCells\s*=\s*\(" "$file" | head -n1 | cut -d: -f1 || true)
echo "buildCellData line: $bline"
echo "generateHighlightedCells line: ${gline:-<not found>}"
# Print around buildCellData (clamp start to 1 to avoid sed negative-range errors)
start=$(( bline > 30 ? bline-30 : 1 ))
end=$(( bline + 160 ))
sed -n "${start},${end}p" "$file"
# Print around generateHighlightedCells if found
if [[ -n "${gline:-}" ]]; then
gstart=$(( gline > 30 ? gline-30 : 1 ))
gend=$(( gline + 200 ))
echo "----"
sed -n "${gstart},${gend}p" "$file"
fi
# Finally, ensure we see the highlightedSet and cellData memo block
echo "----"
sed -n '70,110p' "$file"Repository: piyushdotcomm/Editron
Length of output: 14196
Fix cellData memo deps to include highlightedSet
buildCellData(highlightedSet, total) uses highlightedSet.has(index) to compute isHighlighted, but cellData is memoized with deps only [total]. When text changes but yields the same total (e.g., same cleaned length), highlightedSet changes while cellData stays stale.
🔧 Proposed fix
- const cellData = React.useMemo(
- () => buildCellData(highlightedSet, total),
- // eslint-disable-next-line react-hooks/exhaustive-deps
- [total]
- );
+ const cellData = React.useMemo(
+ () => buildCellData(highlightedSet, total),
+ [highlightedSet, total]
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const cellData = React.useMemo( | |
| () => buildCellData(highlightedSet, total), | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| [total] | |
| ); | |
| const cellData = React.useMemo( | |
| () => buildCellData(highlightedSet, total), | |
| [highlightedSet, total] | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/ui/commits-grid.tsx` around lines 84 - 88, cellData is memoized
with React.useMemo calling buildCellData(highlightedSet, total) but the deps
array only includes [total], causing stale cells when highlightedSet changes;
update the React.useMemo call for cellData to include highlightedSet in the
dependency array (e.g., [highlightedSet, total]) and remove the
eslint-disable-next-line comment so the hook lint rule correctly enforces
dependencies for buildCellData/highlightedSet.
Comment on lines
+7
to
+8
| const isTouchDevice = () => | ||
| typeof window !== "undefined" && window.matchMedia("(hover: none)").matches; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
from pathlib import Path
for path in Path(".").rglob("*.tsx"):
try:
text = path.read_text(encoding="utf-8", errors="ignore")
except Exception:
continue
if "<GlowingEffect" not in text and "glowing-effect" not in text:
continue
print(f"--- {path}")
for i, line in enumerate(text.splitlines()[:12], 1):
print(f"{i}: {line}")
for i, line in enumerate(text.splitlines(), 1):
if "<GlowingEffect" in line or "glowing-effect" in line:
print(f"{i}: {line}")
print()
PY
rg -n -C2 'dynamic\(|ssr:\s*false' app components modulesRepository: piyushdotcomm/Editron
Length of output: 1872
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect the relevant portion of glowing-effect.tsx
echo "===== components/ui/glowing-effect.tsx (around isTouchDevice / effectivelyDisabled) ====="
nl -ba components/ui/glowing-effect.tsx | sed -n '1,220p'
# 2) Find all GlowingEffect callsites
echo
echo "===== All GlowingEffect usages ====="
rg -n --hidden --no-ignore-vcs "<GlowingEffect" .
echo
echo "===== All imports of glowing-effect ====="
rg -n --hidden --no-ignore-vcs "glowing-effect" .
# 3) Check if any callsite is protected by dynamic(..., { ssr: false })
echo
echo "===== dynamic(..., { ssr: false }) near GlowingEffect usage files ====="
rg -n -C4 "dynamic\([^)]*ssr:\s*false" modules components app || trueRepository: piyushdotcomm/Editron
Length of output: 216
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== Print components/ui/glowing-effect.tsx (first 260 lines with line numbers via python) ====="
python3 - <<'PY'
from pathlib import Path
p = Path("components/ui/glowing-effect.tsx")
text = p.read_text(encoding="utf-8", errors="ignore").splitlines()
for i, line in enumerate(text[:260], 1):
print(f"{i:4d}: {line}")
PY
echo
echo "===== All GlowingEffect usages ====="
rg -n --hidden --no-ignore-vcs "<GlowingEffect" .
echo
echo "===== All imports of glowing-effect ====="
rg -n --hidden --no-ignore-vcs "glowing-effect" .
echo
echo "===== Any dynamic(..., { ssr: false }) in files that import GlowingEffect ====="
python3 - <<'PY'
from pathlib import Path
import re
# find files that import GlowingEffect
matches = []
for p in Path(".").rglob("*.ts*"):
try:
s = p.read_text(encoding="utf-8", errors="ignore")
except Exception:
continue
if "from \"`@/components/ui/glowing-effect`\"" in s or "from '`@/components/ui/glowing-effect`'" in s:
matches.append(str(p))
print("IMPORTING_FILES:")
for m in matches:
print(m)
print("\nDYNAMIC_SSR_FALSE_NEARBY:")
for m in matches:
# show any dynamic(..., { ssr: false }) occurrences with context
import subprocess, shlex
cmd = ["bash","-lc", f"rg -n -C3 'dynamic\\([^)]*ssr:\\s*false' {shlex.quote(m)} || true"]
out = subprocess.check_output(cmd, stderr=subprocess.STDOUT).decode()
if out.strip():
print(f"\n--- {m}\n{out.rstrip()}")
PY
echo
echo "===== Where effectivelyDisabled is computed/used (quick search) ====="
rg -n "effectivelyDisabled|isTouchDevice" components/ui/glowing-effect.tsxRepository: piyushdotcomm/Editron
Length of output: 11090
Avoid render-time window.matchMedia detection in GlowingEffect.
components/ui/glowing-effect.tsx computes effectivelyDisabled during render via isTouchDevice() (lines 7-8, 40), which returns false on the server and can become true on the first client render, flipping the !block/!hidden classes (lines 150, 189). GlowingEffect is only used from modules/home/features.tsx and isn’t wrapped in a dynamic(..., { ssr: false }) boundary, so this can cause hydration/layout differences on touch devices. Move touch detection into client state from a useEffect (or handle the fallback in CSS) so initial markup is deterministic.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/ui/glowing-effect.tsx` around lines 7 - 8, GlowingEffect currently
calls isTouchDevice() during render which returns false on the server and can
flip on first client render; change touch detection to client state: remove the
render-time isTouchDevice() call and instead add a useState e.g. const
[isTouchDevice, setIsTouchDevice] = useState(false) and set it in a useEffect
that runs window.matchMedia("(hover: none)").matches (or use addEventListener
for changes); update effectivelyDisabled to depend on this state (the existing
effectivelyDisabled variable in GlowingEffect) so initial SSR markup is
deterministic, and ensure any class toggles that use effectivelyDisabled (the
!block/!hidden logic) use the state value rather than calling isTouchDevice()
directly.
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
prefers-reduced-motionsupport across all animations.Type of change
Related issue
Closes #250
Validation
npm run lintnpm testnpm run buildList any additional manual verification you performed:
editron.in— Performance score 88 (baseline before changes deployed)Screenshots or recordings
Checklist
Summary by CodeRabbit
New Features
Improvements