Fix Projects page background particles #93
Conversation
WalkthroughAdded explicit loading and error handling and centralized render helpers to the Projects page and Members tab, introduced skeleton loaders and a 500ms delayed loading teardown, and changed Particles positioning from absolute to fixed. Error states are set on fetch failures and reset at fetch start. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (3)
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 |
Deploying call-of-code with
|
| Latest commit: |
9d6893b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://76f16c7a.call-of-code.pages.dev |
| Branch Preview URL: | https://animation-fixes.call-of-code.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/projects/page.tsx (1)
40-64: Error handling flow is correct but consider abort handling.The fetch logic properly resets states and handles errors. However, unlike
Members-Text.tsxwhich uses anAbortControllerto cancel in-flight requests on unmount, this component lacks abort handling. If the component unmounts before the fetch completes, state updates on an unmounted component could occur.🔎 Suggested improvement with AbortController
useEffect(() => { + const abortController = new AbortController(); setLoading(true); setError(false); const fetchProjects = async () => { try { - const res = await fetch("/api/projects-with-members"); + const res = await fetch("/api/projects-with-members", { + signal: abortController.signal, + }); const data = await res.json(); if (data.success) { setProjects(data.data as Project[]); } else { console.error("Error loading project data"); throw new Error("Network response was not ok"); } } catch (err) { + if (err instanceof Error && err.name === "AbortError") { + return; + } console.error("Error fetching projects:", err); setError(true); } finally{ setTimeout(() => { setLoading(false); }, 500); } }; fetchProjects(); + return () => { + abortController.abort(); + }; }, []);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/projects/page.tsx(4 hunks)components/Members-Text.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/projects/page.tsx (2)
components/ui/BentoCard.tsx (2)
BentoGrid(7-21)BentoGridItem(23-103)app/projects/skeletonLoader.tsx (1)
SkeletonLoader(19-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (9)
components/Members-Text.tsx (5)
123-123: LGTM on error state addition.Adding an
errorstate to track fetch failures is appropriate for providing user feedback.
150-214: LGTM on the fetch logic with loading/error state management.The implementation correctly:
- Resets
loadinganderrorstates at the start of the fetch- Sets
errorto true on fetch failure while preserving abort handling- Uses a 500ms delayed loading teardown for smoother UX
One minor observation: the delay in
finallyblock (lines 211-213) will still execute after an abort, which may cause a brief flash if the component remounts quickly. However, since the component is unmounting anyway, the state update will be a no-op in most cases.
256-270: Clean consolidation of rendering logic.The
renderMembersTabhelper effectively centralizes the loading, error, empty, and populated state rendering. This reduces duplication across tabs and aligns with the similarrenderProjectspattern in the projects page.
273-288: Tabs array correctly uses the unified render helper.Each tab now delegates to
renderMembersTab, passing the appropriate member list andisFounderflag for the founders tab.
299-306: Particles positioning fix applied correctly.The
Particlescomponent usesfixed inset-0 z-0positioning, which ensures the background particles remain in place during scrolling. This aligns with the PR objective to fix the Projects page background particles.app/projects/page.tsx (4)
38-38: LGTM on error state addition.Adding an
errorstate to track fetch failures provides appropriate user feedback when the API call fails.
66-139: Well-structured render helper with one potential edge case.The
renderProjectsfunction cleanly handles all UI states. However, theclassNamelogic at line 134 uses hardcoded indices (3 and 6) for spanning columns, which could behave unexpectedly if the projects array order changes or items are filtered.Consider whether the spanning logic should be based on project properties rather than array position.
144-151: Particles positioning fix correctly applied.Changing from
absolutetofixedpositioning ensures the background particles remain in place during scrolling, which is the primary objective of this PR.
173-174: Clean invocation of the render helper.The previous inline rendering logic is now consolidated into a single
renderProjects()call, improving readability.
| {Array.from({ length: 6 }).map((_, i) => ( | ||
| <SkeletonLoader key={i} /> | ||
| ))} |
There was a problem hiding this comment.
Note: Related SkeletonLoader component has a bug.
The SkeletonLoader component being used here (from ./skeletonLoader.tsx) contains a bug at line 27 where class is used instead of className in JSX. This will cause the flex styling to not be applied.
Based on the relevant code snippet:
<div class="flex"> // Should be className="flex"Consider fixing this in a follow-up or as part of this PR.
🤖 Prompt for AI Agents
In app/projects/page.tsx around lines 70 to 72 the page maps SkeletonLoader
components; the referenced bug is in ./skeletonLoader.tsx at line 27 where JSX
uses class instead of className, preventing React from applying the flex styling
— open ./skeletonLoader.tsx and change the <div class="flex"> to <div
className="flex"> (and scan for any other incorrect JSX attribute usages such as
for/htmlFor and fix them similarly), then run the app to confirm the flex layout
is applied.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/projects/page.tsx (1)
44-62: Critical:setTimeoutexecutes even on fetch failure.The
setTimeout(lines 54-56) is placed outside theif (data.success)block, which causes it to execute even when the fetch fails. This creates two problems:
- When
data.successis false, the error is thrown (line 52) but thesetTimeouthas already been queued, causing a race condition with the catch block'ssetLoading(false).- Failed fetches experience an unnecessary 500ms delay before showing the error.
The
setTimeoutshould only execute on successful fetches.🔎 Proposed fix
const res = await fetch("/api/projects-with-members"); const data = await res.json(); if (data.success) { setProjects(data.data as Project[]); + setTimeout(() => { + setLoading(false); + }, 500); } else { console.error("Error loading project data"); throw new Error("Network response was not ok"); } - setTimeout(() => { - setLoading(false); - }, 500); } catch (err) {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/projects/page.tsx(3 hunks)components/Members-Text.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/Members-Text.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
app/projects/page.tsx (3)
38-38: LGTM! Clean error state management.The addition of explicit error state tracking and resetting both loading and error states at the start of the fetch is good practice and prevents stale states from previous fetch attempts.
Also applies to: 41-42
67-140: LGTM! Excellent refactor with centralized rendering logic.The
renderProjectshelper cleanly handles all four rendering states (loading, error, empty, populated) and significantly improves maintainability. The error UI with gradient icon is well-designed, and the skeleton loading pattern provides good UX.
148-148: LGTM! Correct fix for background particles.Changing Particles from
absolutetofixedpositioning ensures they remain stationary relative to the viewport during scrolling, which aligns with the PR objective to fix the background particles behavior.
Summary by CodeRabbit
Style
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.