-
Notifications
You must be signed in to change notification settings - Fork 62
[Feat]: Add Skeleton Loading State for project list #3725
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Create ProjectListSkeleton component for loading state - Replace spinner with skeleton in project list table - Show table structure with placeholder content during loading - Add animated loading effects for better UX
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here: https://app.greptile.com/review/github.
2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
WalkthroughThis pull request replaces the existing spinner-based loading indicator in the ProjectsTable component with a skeleton loading component. The Changes
Sequence Diagram(s)sequenceDiagram
participant UI as User Interface
participant DT as DataTable Component
participant PLS as ProjectListSkeleton Component
participant SK as Skeleton Elements
UI->>DT: Request project list
DT->>PLS: Trigger loading state rendering
PLS->>SK: Render skeleton headers and rows
UI->>DT: Data loaded (replacing skeleton)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/web/app/[locale]/projects/components/page-component.tsxOops! Something went wrong! :( ESLint: 8.46.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/apps/web/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (8)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/app/[locale]/projects/components/project-views/list-view/list-skeleton.tsx (2)
4-82
: Consider accessibility improvementsWhile the skeleton implementation is visually effective, it could benefit from accessibility enhancements.
Consider adding ARIA attributes to improve accessibility for screen reader users:
-export function ProjectListSkeleton() { +export function ProjectListSkeleton() { return ( - <div className="w-full"> + <div className="w-full" role="status" aria-busy="true" aria-label="Loading projects"> <div className="rounded-md border">Also consider memoizing the component for performance optimization:
-export function ProjectListSkeleton() { +export const ProjectListSkeleton = React.memo(function ProjectListSkeleton() { // Component implementation -} +});
35-76
: Consider responsive skeleton rowsThe fixed number of skeleton rows (10) might be excessive on smaller screens.
Consider making the number of skeleton rows responsive based on screen size:
-{Array.from({ length: 10 }).map((_, index) => ( +{Array.from({ length: useResponsiveRowCount() }).map((_, index) => (Where
useResponsiveRowCount
could be a custom hook that returns a different number based on viewport size:function useResponsiveRowCount() { const [rowCount, setRowCount] = useState(10); useEffect(() => { const handleResize = () => { if (window.innerWidth < 640) { setRowCount(5); } else { setRowCount(10); } }; handleResize(); window.addEventListener('resize', handleResize); return () => window.removeEventListener('resize', handleResize); }, []); return rowCount; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/app/[locale]/projects/components/project-views/list-view/data-table.tsx
(2 hunks)apps/web/app/[locale]/projects/components/project-views/list-view/list-skeleton.tsx
(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
apps/web/app/[locale]/projects/components/project-views/list-view/data-table.tsx (1)
apps/web/app/[locale]/projects/components/project-views/list-view/list-skeleton.tsx (1)
ProjectListSkeleton
(4-82)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: deploy
🔇 Additional comments (7)
apps/web/app/[locale]/projects/components/project-views/list-view/data-table.tsx (4)
37-37
: LGTM! Proper import of the new skeleton componentThe import statement correctly references the new skeleton component from the local directory.
468-468
: Great UX improvement with skeleton loadingReplacing the spinner with a skeleton loading state provides users with a better visual indication of the content structure while data is being fetched.
470-470
: UI consistency enhancementAdding a border to the table container ensures visual consistency between the skeleton state and the loaded data view.
31-31
:✅ Verification successful
Confirm unused import
The
HorizontalSeparator
import has been added, but it appears to only be used in the dropdown menu component and was likely already present in the file. Check if this is a new addition or if the diff is showing it as changed when it wasn't.
🏁 Script executed:
#!/bin/bash # Check if HorizontalSeparator was already imported in previous versions git show HEAD~1:apps/web/app/[locale]/projects/components/project-views/list-view/data-table.tsx | grep -n "import.*HorizontalSeparator"Length of output: 214
HorizontalSeparator import is confirmed as pre-existing
- The previous version imported
{ HorizontalSeparator, SpinnerLoader }
(line 31), and the current diff only removesSpinnerLoader
.- This confirms that
HorizontalSeparator
was already part of the file and isn’t a new addition.- If the removal of
SpinnerLoader
was unintentional, please verify its usage in the dropdown menu component; otherwise, no changes are needed regardingHorizontalSeparator
.apps/web/app/[locale]/projects/components/project-views/list-view/list-skeleton.tsx (3)
1-3
: LGTM! Proper imports for skeleton componentAll necessary components are imported correctly.
4-33
: Well-structured table header skeletonThe table header skeleton effectively mimics the actual table structure with appropriate column widths and placeholder elements.
34-77
: Comprehensive row skeleton implementationThe table body implementation creates a realistic loading state with appropriate skeleton elements for each type of data. The use of Array.from to generate multiple skeleton rows is efficient.
- Added back navigation arrow in timesheet detail view - Implemented breadcrumb navigation for better user orientation - Added proper dark mode support for navigation elements - Maintained consistent spacing and alignment with flex layout - Ensured responsive design with proper height constraints
Description
Please include a summary of the changes and the related issues.
Type of Change
Checklist
Previous screenshots
Please add here videos or images of the previous status
Current screenshots
Please add here videos or images of the current (new) status
Summary by CodeRabbit
New Features
Style