[jules] enhance: Add complete skeleton loading for GroupDetails page#297
[jules] enhance: Add complete skeleton loading for GroupDetails page#297
Conversation
Co-authored-by: Devasy23 <110348311+Devasy23@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
✅ Deploy Preview for split-but-wiser ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughThis pull request introduces skeleton loading for the GroupDetails page. A new GroupDetailsSkeleton component is created with framer-motion animations and dual-theme support, integrated into GroupDetails.tsx to replace the basic loading placeholder, and documented in the knowledge base and changelog. Changes
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #297 +/- ##
=======================================
Coverage ? 63.55%
=======================================
Files ? 21
Lines ? 2456
Branches ? 254
=======================================
Hits ? 1561
Misses ? 831
Partials ? 64
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.Jules/todo.md:
- Around line 60-66: The completed skeleton loading task for the web should be
moved out of the "### Mobile" section into the "### Web" section; locate the
checklist entry referencing web/pages/GroupDetails.tsx and
web/components/skeletons/GroupDetailsSkeleton.tsx (the lines showing "- [x]
**[ux]** Complete skeleton loading for GroupDetails page" and its metadata) and
cut/paste it so it appears with the other completed Web tasks (place it after
the last completed Web task and before the "### Mobile" header) to keep
platform-specific tasks grouped correctly.
In `@web/components/skeletons/GroupDetailsSkeleton.tsx`:
- Around line 31-38: The skeleton currently renders 6 avatar placeholders (five
in the map plus one extra) which doesn't match GroupDetails hero; update the JSX
in GroupDetailsSkeleton.tsx so it renders the same elements as the real hero:
five member avatar Skeletons (the existing map), one overflow indicator
Skeleton, and one Settings-button Skeleton (total 7). Keep the same Skeleton
component, className logic and THEMES.NEOBRUTALISM conditional for border color
so the placeholders visually match the real UI.
- Line 1: Remove the unused React import from GroupDetailsSkeleton by deleting
the top-level "import React from 'react';" statement in the GroupDetailsSkeleton
component file; ensure there are no other direct React.* references (hooks or
types) in the file—if there are, keep or replace them with named imports (e.g.,
import { useState } from 'react')—otherwise remove the import to rely on the new
JSX transform.
- Around line 13-18: The skeleton's hero in GroupDetailsSkeleton.tsx uses fixed
heights (h-48 / h-64 on the motion.div) which don't match the content-driven
Immersive Header in GroupDetails.tsx (large text sizes text-5xl/text-7xl and
padding p-8/p-12), causing layout shift; fix by either removing the fixed
h-48/h-64 from the motion.div and making it content-driven (use h-auto/min-h
plus matching padding/typography spacing) so it mirrors the Immersive Header
rendering, or apply the same fixed height/padding/typography constraints to the
Immersive Header container so both components (GroupDetailsSkeleton's motion.div
and the Immersive Header in GroupDetails.tsx) share identical sizing and
spacing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 771e0628-5bae-473a-a88b-a6acf13ad4fc
⛔ Files ignored due to path filters (1)
web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.Jules/changelog.md.Jules/knowledge.md.Jules/todo.mdweb/components/skeletons/GroupDetailsSkeleton.tsxweb/pages/GroupDetails.tsx
| - [x] **[ux]** Complete skeleton loading for GroupDetails page | ||
| - Completed: 2026-03-06 | ||
| - Files modified: `web/pages/GroupDetails.tsx`, `web/components/skeletons/GroupDetailsSkeleton.tsx` | ||
| - Context: Replace ActivityIndicator with skeleton layout matching content | ||
| - Impact: Better loading experience, less jarring layout shift | ||
| - Size: ~120 lines | ||
|
|
There was a problem hiding this comment.
Task entry placed under wrong section.
This Web skeleton loading task is placed under the "### Mobile" section (which starts at line 51), but the affected files are in web/. It should be placed under the "### Web" section (starting at line 11) with the other completed web tasks.
📝 Suggested fix: Move entry under Web section
Move lines 60-66 to after line 49 (after the last completed Web task) and before the "### Mobile" section.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.Jules/todo.md around lines 60 - 66, The completed skeleton loading task for
the web should be moved out of the "### Mobile" section into the "### Web"
section; locate the checklist entry referencing web/pages/GroupDetails.tsx and
web/components/skeletons/GroupDetailsSkeleton.tsx (the lines showing "- [x]
**[ux]** Complete skeleton loading for GroupDetails page" and its metadata) and
cut/paste it so it appears with the other completed Web tasks (place it after
the last completed Web task and before the "### Mobile" header) to keep
platform-specific tasks grouped correctly.
| @@ -0,0 +1,136 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused React import.
With React 19 and the new JSX transform, the React import is no longer necessary when you're only using JSX without explicit React.* calls.
♻️ Suggested cleanup
-import React from 'react';
import { useTheme } from '../../contexts/ThemeContext';📝 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.
| import React from 'react'; | |
| import { useTheme } from '../../contexts/ThemeContext'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/components/skeletons/GroupDetailsSkeleton.tsx` at line 1, Remove the
unused React import from GroupDetailsSkeleton by deleting the top-level "import
React from 'react';" statement in the GroupDetailsSkeleton component file;
ensure there are no other direct React.* references (hooks or types) in the
file—if there are, keep or replace them with named imports (e.g., import {
useState } from 'react')—otherwise remove the import to rely on the new JSX
transform.
| <motion.div | ||
| initial={{ y: -20, opacity: 0 }} | ||
| animate={{ y: 0, opacity: 1 }} | ||
| className={`relative overflow-hidden rounded-3xl h-48 md:h-64 ${ | ||
| style === THEMES.NEOBRUTALISM ? 'border-2 border-black shadow-[8px_8px_0px_0px_rgba(0,0,0,1)]' : 'border border-white/10 shadow-2xl' | ||
| }`} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the actual hero section's height constraints in GroupDetails.tsx
rg -n 'Immersive Header|motion\.div' web/pages/GroupDetails.tsx | head -20
# Check for h-48, h-64, or min-h classes in the hero section
sed -n '571,624p' web/pages/GroupDetails.tsx | rg 'h-\d+|min-h'Repository: Devasy/splitwiser
Length of output: 1467
🏁 Script executed:
# Read the full Immersive Header section
sed -n '570,625p' web/pages/GroupDetails.tsxRepository: Devasy/splitwiser
Length of output: 4108
🏁 Script executed:
# Also read the GroupDetailsSkeleton file to see the skeleton structure
cat -n web/components/skeletons/GroupDetailsSkeleton.tsxRepository: Devasy/splitwiser
Length of output: 8068
Hero skeleton height (h-48/h-64) does not match actual component height, causing potential layout shift.
The skeleton constrains the hero to fixed heights with h-full, but the actual Immersive Header in GroupDetails.tsx (lines 571-624) is content-driven with no height constraints. Given the large text (text-5xl md:text-7xl), padding (p-8 md:p-12), and avatars, the actual component will likely exceed these fixed dimensions when content loads, creating visible layout shift.
Consider either:
- Adding matching height constraints to the actual hero section, or
- Making the skeleton height content-driven to match the actual component
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/components/skeletons/GroupDetailsSkeleton.tsx` around lines 13 - 18, The
skeleton's hero in GroupDetailsSkeleton.tsx uses fixed heights (h-48 / h-64 on
the motion.div) which don't match the content-driven Immersive Header in
GroupDetails.tsx (large text sizes text-5xl/text-7xl and padding p-8/p-12),
causing layout shift; fix by either removing the fixed h-48/h-64 from the
motion.div and making it content-driven (use h-auto/min-h plus matching
padding/typography spacing) so it mirrors the Immersive Header rendering, or
apply the same fixed height/padding/typography constraints to the Immersive
Header container so both components (GroupDetailsSkeleton's motion.div and the
Immersive Header in GroupDetails.tsx) share identical sizing and spacing.
| {[1, 2, 3, 4, 5].map((i) => ( | ||
| <Skeleton | ||
| key={i} | ||
| className={`w-12 h-12 rounded-full border-4 ${style === THEMES.NEOBRUTALISM ? 'border-black' : 'border-indigo-600'}`} | ||
| /> | ||
| ))} | ||
| <Skeleton className={`w-12 h-12 rounded-full border-4 ${style === THEMES.NEOBRUTALISM ? 'border-black' : 'border-indigo-600'}`} /> | ||
| </div> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Avatar count mismatch with actual UI.
The skeleton renders 6 avatar placeholders (5 in loop + 1 outside), but the actual GroupDetails hero renders up to 5 member avatars + an overflow indicator + a Settings button (potentially 7 elements). This is a minor visual discrepancy during the skeleton-to-content transition.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/components/skeletons/GroupDetailsSkeleton.tsx` around lines 31 - 38, The
skeleton currently renders 6 avatar placeholders (five in the map plus one
extra) which doesn't match GroupDetails hero; update the JSX in
GroupDetailsSkeleton.tsx so it renders the same elements as the real hero: five
member avatar Skeletons (the existing map), one overflow indicator Skeleton, and
one Settings-button Skeleton (total 7). Keep the same Skeleton component,
className logic and THEMES.NEOBRUTALISM conditional for border color so the
placeholders visually match the real UI.
Implemented a comprehensive skeleton loading state for the GroupDetails page to replace the generic
PageLoader. This prevents jarring layout shifts and provides a more polished user experience during data fetching.Key Features:
framer-motionpulsing animations.todo.md,changelog.md,knowledge.md) with completion details and patterns.PR created automatically by Jules for task 5677304916246418878 started by @Devasy23
Summary by CodeRabbit