UI skeleton 569 #571
UI skeleton 569 #571
Conversation
📝 WalkthroughWalkthroughThe changes introduce a client-side loading state with skeleton card UI components displayed during data initialization. Styling enhancements add transitions, hover effects, and rounded borders across card components. A new SkeletonCard component and accompanying CSS animation support the loading experience with an 800ms delay before content renders. Changes
Sequence DiagramsequenceDiagram
participant User
participant Home
participant useEffect1
participant useEffect2
participant SkeletonCard
participant CardEffect
User->>Home: Visit home page
Home->>useEffect1: Component mounts
useEffect1->>Home: Set loading = true<br/>Shuffle & set randomProjects
Home->>SkeletonCard: Render 3 SkeletonCards<br/>(loading = true)
SkeletonCard-->>User: Display skeleton cards
useEffect2->>useEffect2: Wait 800ms (timer)
useEffect2->>Home: Set loading = false
Home->>CardEffect: Render CardEffect components<br/>(for each project)
CardEffect-->>User: Display actual cards with content
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/index.jsx (1)
24-39:⚠️ Potential issue | 🔴 CriticalCritical: Duplicate
useEffect— Lines 37-39 mutate the source array and overwrite the correct shuffle.There are two
useEffect(() => { ... }, [])hooks that both shuffle and setrandomProjectson mount. The second one (Lines 37-39) is leftover code that:
- Mutates the imported
projectsarray in-place via.sort()(no spread/copy), corrupting the shared module-level array for any other consumer.- Overwrites the result of the first effect (Lines 24-27), which correctly uses
[...projects]to avoid mutation.Remove the duplicate effect entirely.
🐛 Fix — remove the duplicate effect
useEffect(() => { const shuffled = [...projects].sort(() => 0.5 - Math.random()).slice(0, 3) setRandomProjects(shuffled) }, []) useEffect(() => { const timer = setTimeout(() => { setLoading(false) }, 800) return () => clearTimeout(timer) }, []) - useEffect(() => { - setRandomProjects(projects.sort(() => 0.5 - Math.random()).slice(0, 3)) - }, [])
🤖 Fix all issues with AI agents
In `@src/components/skeletonCard.jsx`:
- Line 3: Remove the redundant aria-busy from the skeleton card and add an
accessible, visually-hidden status message so screen readers hear that content
is loading: in src/components/skeletonCard.jsx update the <div
className="skeleton-card" role="status"> to include a hidden text node (e.g., a
span with a "sr-only" or visually-hidden class) such as "Loading content" and
remove the aria-busy attribute (the parent grid in index.jsx already provides
aria-busy="true"), ensuring the role="status" has accessible text for assistive
technologies.
In `@src/pages/index.jsx`:
- Around line 29-35: The artificial 800ms delay in the useEffect (the
setTimeout/clearTimeout timer that calls setLoading(false)) creates unnecessary
latency even though the statically imported projects data is immediately
available; remove the timeout and instead derive loading from actual data
readiness (e.g., initialize loading to false or
setLoading(Boolean(!projects.length)) and remove the timer code in the
useEffect), or if you only want a transient skeleton for the first render, drop
the setTimeout and let the component render once with loading true then
immediately set loading false synchronously based on the presence of the
imported projects; update or remove the useEffect, timer, and clearTimeout
references accordingly (functions/identifiers: useEffect, setLoading, timer,
clearTimeout, projects).
In `@src/pages/projects.jsx`:
- Around line 35-53: The card (MuiCard) sets cursor: 'pointer' but only the
inner <a> is clickable, causing a misleading affordance; either remove the
cursor style from the MuiCard sx prop or make the entire card clickable by
moving the link/onclick to the MuiCard (e.g., wrap MuiCard with the anchor/Link
or add an onClick that navigates to the same href and set role="link" and
tabIndex for accessibility); update the MuiCard's sx to drop cursor: 'pointer'
if you choose not to make the whole card act as a link.
- Around line 48-51: The boxShadow style inside the hover style object in
src/pages/projects.jsx uses an invalid alpha value (rgba(255,255,0,255)); update
the boxShadow value to use a valid alpha between 0 and 1 (for example
'rgba(255,255,0,1)') or an equivalent color string; locate the '&:hover' style
block that contains transform and boxShadow and replace the alpha 255 with 1 to
fix the CSS.
🧹 Nitpick comments (6)
src/style/tailwind.css (2)
4-10: Hardcoded dark background won't adapt to light mode.
#2a2a2alooks correct on a dark theme but will appear as a dark rectangle on a light-themed page, creating a jarring visual mismatch. Consider adding a light-mode variant (e.g.,#e5e7eb) or converting this to Tailwind utility classes withdark:variants so it respects the theme.Also, Tailwind already ships an
animate-pulseutility. Rolling a custom keyframe duplicates built-in functionality.♻️ Suggested approach using Tailwind utilities instead of custom CSS
Replace the custom CSS class with Tailwind utilities directly on the
SkeletonCarddiv:- <div className="skeleton-card" aria-busy="true" role="status" /> + <div + className="h-[180px] w-full rounded-lg bg-zinc-200 dark:bg-zinc-800 animate-pulse" + aria-busy="true" + role="status" + />This eliminates the need for the custom
.skeleton-cardclass and@keyframes pulseentirely, and automatically handles light/dark themes.
9-16: Missingprefers-reduced-motionconsideration.Users who prefer reduced motion will still see the continuous pulse animation. Consider wrapping the animation in a
@media (prefers-reduced-motion: no-preference)query, or using Tailwind'smotion-safe:animate-pulseif you switch to utility classes.src/components/Banner.jsx (1)
19-27: Inconsistent indentation in the reformatted block.The
<Link>and<a>elements lost their nesting indentation relative to the parent<div>. This is a formatting-only change but the result is harder to scan than the surrounding JSX.src/components/Card.jsx (1)
59-71: Inconsistent indentation insideCard.Cta.The
<ChevronRightIcon>on Line 67 and the closing</Link>on Line 69 are not aligned with the rest of the JSX tree. Minor formatting nit.src/components/skeletonCard.jsx (1)
1-5: File naming inconsistency — use PascalCase to match other components.Other component files in
src/components/use PascalCase (Card.jsx,Banner.jsx,CardEffect.jsx). This file should beSkeletonCard.jsxto stay consistent.src/pages/index.jsx (1)
22-35: Inconsistent indentation breaks readability.Lines 23-35 are flush-left or under-indented compared to the component body (Line 22 is indented correctly at 2 spaces inside the function). This makes the hook boundaries hard to spot visually.
| @@ -0,0 +1,5 @@ | |||
| export default function SkeletonCard() { | |||
| return ( | |||
| <div className="skeleton-card" aria-busy="true" role="status" /> | |||
There was a problem hiding this comment.
role="status" without accessible text is silent to screen readers.
The div has no text content, so assistive technologies will announce nothing for this live region. Add a visually-hidden label so users know content is loading.
Also, aria-busy="true" is already set on the parent grid container in index.jsx, making it redundant here on each individual card.
♻️ Suggested fix
- <div className="skeleton-card" aria-busy="true" role="status" />
+ <div className="skeleton-card" role="status">
+ <span className="sr-only">Loading project…</span>
+ </div>📝 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.
| <div className="skeleton-card" aria-busy="true" role="status" /> | |
| <div className="skeleton-card" role="status"> | |
| <span className="sr-only">Loading project…</span> | |
| </div> |
🤖 Prompt for AI Agents
In `@src/components/skeletonCard.jsx` at line 3, Remove the redundant aria-busy
from the skeleton card and add an accessible, visually-hidden status message so
screen readers hear that content is loading: in src/components/skeletonCard.jsx
update the <div className="skeleton-card" role="status"> to include a hidden
text node (e.g., a span with a "sr-only" or visually-hidden class) such as
"Loading content" and remove the aria-busy attribute (the parent grid in
index.jsx already provides aria-busy="true"), ensuring the role="status" has
accessible text for assistive technologies.
| useEffect(() => { | ||
| const timer = setTimeout(() => { | ||
| setLoading(false) | ||
| }, 800) | ||
|
|
||
| return () => clearTimeout(timer) | ||
| }, []) |
There was a problem hiding this comment.
Artificial 800ms delay when data is already synchronously available.
The projects data is statically imported (Line 17) — it's available immediately. The 800ms setTimeout creates a fake loading state that adds latency on every page visit for no functional benefit. The PR notes this is "temporary and replaceable by real async data later," but until then it actively degrades perceived performance — the opposite of the stated goal.
Consider tying the loading state to actual data readiness instead:
♻️ Suggested simplification
- const [randomProjects, setRandomProjects] = useState([])
- const [loading, setLoading] = useState(true)
- useEffect(() => {
- const shuffled = [...projects].sort(() => 0.5 - Math.random()).slice(0, 3)
- setRandomProjects(shuffled)
- }, [])
-
- useEffect(() => {
- const timer = setTimeout(() => {
- setLoading(false)
- }, 800)
-
- return () => clearTimeout(timer)
- }, [])
+ const [randomProjects, setRandomProjects] = useState([])
+ const loading = randomProjects.length === 0
+
+ useEffect(() => {
+ const shuffled = [...projects].sort(() => 0.5 - Math.random()).slice(0, 3)
+ setRandomProjects(shuffled)
+ }, [])This shows skeletons only for the single render cycle before the effect fires, matching the actual data lifecycle.
🤖 Prompt for AI Agents
In `@src/pages/index.jsx` around lines 29 - 35, The artificial 800ms delay in the
useEffect (the setTimeout/clearTimeout timer that calls setLoading(false))
creates unnecessary latency even though the statically imported projects data is
immediately available; remove the timeout and instead derive loading from actual
data readiness (e.g., initialize loading to false or
setLoading(Boolean(!projects.length)) and remove the timer code in the
useEffect), or if you only want a transient skeleton for the first render, drop
the setTimeout and let the component render once with loading true then
immediately set loading false synchronously based on the presence of the
imported projects; update or remove the useEffect, timer, and clearTimeout
references accordingly (functions/identifiers: useEffect, setLoading, timer,
clearTimeout, projects).
| <MuiCard | ||
| className='dark:bg-[#2A2A2A] dark:border-white' | ||
| sx={{ | ||
| height: 400, | ||
| borderRadius: 2, | ||
| border: '1px solid', | ||
| borderColor: '#3c982c', | ||
| boxShadow: '0px 4px 4px #00000040', | ||
| backdropFilter: 'blur(4px) brightness(100%)', | ||
| display: 'flex', | ||
| flexDirection: 'column', | ||
| transition: 'all 0.3s ease', | ||
| cursor: 'pointer', | ||
| '&:hover': { | ||
| transform: 'translateY(-6px)', | ||
| boxShadow: '1px 2px 1px 1px rgba(255,255,0,255)', | ||
| }, | ||
| }} | ||
| > |
There was a problem hiding this comment.
cursor: 'pointer' on the card implies the entire card is clickable, but only the inner link is.
The pointer cursor appears over the entire card surface, yet clicking anywhere outside the <a> tag does nothing. This creates a misleading affordance. Either make the whole card a clickable link or remove the cursor: 'pointer'.
🤖 Prompt for AI Agents
In `@src/pages/projects.jsx` around lines 35 - 53, The card (MuiCard) sets cursor:
'pointer' but only the inner <a> is clickable, causing a misleading affordance;
either remove the cursor style from the MuiCard sx prop or make the entire card
clickable by moving the link/onclick to the MuiCard (e.g., wrap MuiCard with the
anchor/Link or add an onClick that navigates to the same href and set
role="link" and tabIndex for accessibility); update the MuiCard's sx to drop
cursor: 'pointer' if you choose not to make the whole card act as a link.
| '&:hover': { | ||
| transform: 'translateY(-6px)', | ||
| boxShadow: '1px 2px 1px 1px rgba(255,255,0,255)', | ||
| }, |
There was a problem hiding this comment.
Invalid rgba alpha value — use 1 instead of 255.
rgba(255,255,0,255) uses 255 for the alpha channel, but CSS rgba() expects a value between 0 and 1 (or 0%–100%). Browsers silently clamp this to 1, so it works today, but it's technically invalid and confusing to future readers.
🐛 Fix
- boxShadow: '1px 2px 1px 1px rgba(255,255,0,255)',
+ boxShadow: '1px 2px 1px 1px rgba(255, 255, 0, 1)',📝 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.
| '&:hover': { | |
| transform: 'translateY(-6px)', | |
| boxShadow: '1px 2px 1px 1px rgba(255,255,0,255)', | |
| }, | |
| '&:hover': { | |
| transform: 'translateY(-6px)', | |
| boxShadow: '1px 2px 1px 1px rgba(255, 255, 0, 1)', | |
| }, |
🤖 Prompt for AI Agents
In `@src/pages/projects.jsx` around lines 48 - 51, The boxShadow style inside the
hover style object in src/pages/projects.jsx uses an invalid alpha value
(rgba(255,255,0,255)); update the boxShadow value to use a valid alpha between 0
and 1 (for example 'rgba(255,255,0,1)') or an equivalent color string; locate
the '&:hover' style block that contains transform and boxShadow and replace the
alpha 255 with 1 to fix the CSS.
1 participant
This PR introduces skeleton loading placeholders for the Projects section on the Home page to improve perceived performance and user experience.
✨ What’s improved
Displays skeleton cards while project data is loading
Prevents blank spaces and layout shifts
Provides immediate visual feedback on slower networks
Improves accessibility using ARIA loading attributes
🛠️ Implementation details
Added a reusable SkeletonCard component
Implemented a loading state in pages/index.jsx
Skeletons match the final card layout to avoid reflow
Loading state is temporary and can be replaced with real async data later
♿ Accessibility
Uses aria-busy and aria-live="polite" to announce loading state to assistive technologies
🧪 Testing
Tested locally on the Home page
Verified skeletons appear on initial load and transition smoothly to project cards
Summary by CodeRabbit
New Features
Improvements