Feat/dashboard conditional rendering optimization #3909
Conversation
…me errors by adding optional chaining for profile properties.
…n states - Added dynamic imports for `TeamMembers`, `Timer`, `ChatwootWidget`, `TeamInvitations`, `TeamOutstandingNotifications`, and `UnverifiedEmail` components to improve performance and user experience. - Introduced skeleton components for loading states: `TeamMembersSkeleton`, `TimerSkeleton`, `TeamInvitationsSkeleton`, `TeamNotificationsSkeleton`, and `UnverifiedEmailSkeleton` to enhance UI responsiveness during data fetching. - Refactored the main page component to utilize these dynamic imports and skeletons, ensuring a smoother loading experience for users.
…perience - Integrated `Suspense` for lazy loading of `TeamInvitations` and `TeamOutstandingNotifications` components, providing skeleton states during data fetching. - Updated `UnverifiedEmail` component to accept a user prop for optimization and backward compatibility. - Refactored `TeamInvitations` to accept invitations as props, streamlining data handling and improving performance. - Enhanced `TeamOutstandingNotifications` to receive necessary props directly, ensuring better modularity and reusability.
…section - Added `AuthUserTaskInputSkeleton` and `TaskTimerSectionSkeleton` for improved loading states in the task management UI. - Refactored `TaskTimerSection` to utilize dynamic imports with skeleton loading, enhancing user experience during data fetching. - Updated `MainPage` to conditionally render components based on user state, ensuring better performance and responsiveness. - Enhanced `TeamOutstandingNotifications` to filter valid tasks, improving notification accuracy and debugging visibility.
… management - Introduced notification state management for user and manager outstanding notifications using local storage. - Refactored `TeamOutstandingNotifications` and `ManagerOutstandingUsersNotification` components to utilize new notification helpers for better visibility and control. - Added constants for notification keys and configuration to streamline notification handling. - Improved task filtering logic to ensure accurate display of outstanding tasks and user interactions.
- Introduced `AppSidebarSkeleton`, `CollaborateSkeleton`, `ModalSkeleton`, `TeamsDropDownSkeleton`, and `UserNavAvatarSkeleton` to enhance user experience during data fetching. - Implemented lazy loading for `AppSidebar`, `Collaborate`, `TeamsDropDown`, and `UserNavAvatar` components using `Suspense` to display skeletons while loading. - Updated `MainLayout` and `Navbar` components to utilize new skeletons, improving performance and responsiveness during loading states.
- Refactored multiple components to utilize dynamic imports with `Suspense`, enhancing performance during loading states. - Updated `AppSidebar`, `Navbar`, and `TeamsDropDown` to lazy load modals and display skeletons, improving user experience while data is being fetched. - Added new entries to `.cspell.json` for improved spell checking across the project.
…ures - Implemented lazy loading for `TeamMemberHeader` and `NoTeam` components, optimizing loading states with `Suspense`. - Added new skeleton components: `TeamMemberHeaderSkeleton` and `NoTeamSkeleton` to improve user experience during data fetching. - Updated `MainPage` to utilize these new components, ensuring a consistent loading experience across team-related views. - Enhanced `.cspell.json` with additional entries for improved spell checking.
|
""" WalkthroughThis update introduces dynamic imports and React Suspense throughout the web app, enabling lazy loading for many UI components and modals. Numerous skeleton loader components are added to provide consistent loading states. Notification logic is refactored for outstanding tasks, centralizing state management and visibility checks. Several components are restructured to accept data via props. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MainLayout
participant LazyAppSidebar
participant AppSidebarSkeleton
User->>MainLayout: Load page
MainLayout->>LazyAppSidebar: Render with Suspense
LazyAppSidebar-->>AppSidebarSkeleton: While loading, show skeleton
LazyAppSidebar-->>MainLayout: Once loaded, render sidebar
sequenceDiagram
participant User
participant Navbar
participant LazyTeamsDropDown
participant TeamsDropDownSkeleton
User->>Navbar: Load navbar
Navbar->>LazyTeamsDropDown: Render with Suspense
LazyTeamsDropDown-->>TeamsDropDownSkeleton: While loading, show skeleton
LazyTeamsDropDown-->>Navbar: Once loaded, render dropdown
sequenceDiagram
participant User
participant MainPage
participant LazyTeamMembers
participant TeamMembersSkeleton
User->>MainPage: View team members
MainPage->>LazyTeamMembers: Render with Suspense
LazyTeamMembers-->>TeamMembersSkeleton: While loading, show skeleton
LazyTeamMembers-->>MainPage: Once loaded, render team members
Possibly related issues
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
yarn install v1.22.22 ✨ Finishing Touches
🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
Please review these files and clean up the unused code. |
Please review these files and clean up the unused code. |
There was a problem hiding this comment.
PR Summary
Major dashboard optimization implementing Next.js best practices for lazy loading and skeleton states, achieving 223-334KB bundle size reduction and improved loading performance.
- Introduces new skeleton components (
AppSidebarSkeleton,TeamMemberHeaderSkeleton, etc.) with prop-based configuration instead of hooks to prevent Suspense rendering issues - Implements lazy loading with
dynamic()imports andssr: falsefor heavy components like AppSidebar and Navbar - Removes double loading states by eliminating
loadingproperty fromdynamic()imports in favor of Suspense fallbacks - Adds comprehensive notification management system with rate limiting and persistence in
notifications.ts - Optimizes conditional rendering using pattern
{condition && (<Suspense fallback={<Skeleton />}><LazyComponent /></Suspense>)}
29 files reviewed, 31 comments
Edit PR Review Bot Settings | Greptile
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
apps/web/core/components/layouts/default-layout/navbar.tsx (1)
59-76: Extract HeaderSkeleton to a separate file.For consistency with other skeleton components and better maintainability, move this to a dedicated file.
Create
apps/web/core/components/common/skeleton/header-skeleton.tsx:import Skeleton from 'react-loading-skeleton'; import { Container } from '@/core/components'; export const HeaderSkeleton = () => { return ( <nav className="bg-white dark:bg-dark-high w-full nav-items--shadow fixed z-[999]"> <Container> <div className="w-full flex justify-between items-center min-h-[70px]"> <Skeleton height={45} width={200} borderRadius={20} className="dark:bg-[#272930]" /> <div className="flex items-center space-x-5"> <div className="hidden sm:block"> <Skeleton height={45} width={175} borderRadius={20} className="dark:bg-[#272930]" /> </div> <Skeleton circle={true} height={45} width={45} className="dark:bg-[#272930]" /> </div> </div> </Container> </nav> ); };Then import it:
-const HeaderSkeleton = () => { - // ... component body -}; +import { HeaderSkeleton } from '@/core/components/common/skeleton/header-skeleton';
♻️ Duplicate comments (1)
apps/web/core/lib/helpers/notifications.ts (1)
29-35: Static analysis warning appears to be a false positive.The CodeQL warning about storing sensitive data in clear text references "uniqueEmployees" which is not present in this function. This function stores notification state metadata (timestamps, counts, task hashes) which is not sensitive information.
🧹 Nitpick comments (11)
apps/web/core/types/interfaces/common/notification.ts (1)
1-6: Well-designed notification state interface.The
INotificationStateinterface provides a clear contract for notification state management with appropriate property types. Consider adding JSDoc comments to document the purpose of each property, especiallylastTaskHashandlastShownCount.+/** + * Interface for managing notification state across the application + */ export interface INotificationState { + /** Timestamp of when the notification was last dismissed */ lastDismissed: number; + /** Number of times the notification has been dismissed */ dismissCount: number; + /** Hash representing the last task state for change detection */ lastTaskHash: string; + /** Count of how many times the notification was shown */ lastShownCount: number; }apps/web/core/components/common/skeleton/timer-skeleton.tsx (1)
6-22: Consider simplifying the nested layout structure.The current nested flex layout with multiple responsive breakpoints creates unnecessary complexity for a skeleton component. The structure has deeply nested divs that could be streamlined.
- <div - className={clsxm( - 'flex justify-center items-center p-2 space-x-2 space-y-5 lg:flex-col xl:flex-row xl:space-y-0 min-w-[260px]', - className - )} - > - <div className="flex justify-center items-center pr-2 space-x-2 w-full xl:w-4/5"> - <div className="flex justify-between items-start w-full"> - <div className="mx-auto w-full"> - {/* Timer display */} - <div className="w-[200px] h-12 bg-[#F0F0F0] dark:bg-[#353741] animate-pulse rounded-lg" /> - - {/* Timer status */} - <div className="mt-2 w-24 h-4 bg-[#F0F0F0] dark:bg-[#353741] animate-pulse rounded" /> - </div> - </div> - </div> + <div + className={clsxm( + 'flex justify-center items-center p-2 space-x-4 lg:flex-col xl:flex-row min-w-[260px]', + className + )} + > + <div className="flex flex-col items-center space-y-2"> + {/* Timer display */} + <div className="w-[200px] h-12 bg-[#F0F0F0] dark:bg-[#353741] animate-pulse rounded-lg" /> + {/* Timer status */} + <div className="w-24 h-4 bg-[#F0F0F0] dark:bg-[#353741] animate-pulse rounded" /> + </div>apps/web/core/components/common/skeleton/unverified-email-skeleton.tsx (1)
32-32: Consider using named export for consistency.This component uses default export while
TimerSkeletonuses named export. Consider standardizing the export pattern across skeleton components.-export default UnverifiedEmailSkeleton; +export { UnverifiedEmailSkeleton };apps/web/core/components/layouts/app-sidebar.tsx (1)
47-58: Consider reorganizing imports for better readability.Move the
dynamicandSuspenseimports to the top with other React imports, and group the lazy component definitions together after all regular imports.+import dynamic from 'next/dynamic'; +import { Suspense } from 'react'; import { NavHome } from '../nav-home'; import { NavMain } from './nav-main'; -// Lazy load CreateTeamModal for performance optimization -import dynamic from 'next/dynamic'; -import { Suspense } from 'react'; import { ModalSkeleton } from '@/core/components/common/skeleton/modal-skeleton'; +// Lazy load modals for performance optimization +const LazySidebarCommandModal = dynamic( + () => import('./default-layout/header/sidebar-command-modal').then((mod) => ({ default: mod.SidebarCommandModal })), + { + ssr: false + // Note: Removed loading here to avoid double loading states + // Suspense fallback will handle all loading states uniformly + } +); + const LazyCreateTeamModal = dynamic( () => import('../features/teams/create-team-modal').then((mod) => ({ default: mod.CreateTeamModal })), { ssr: false // Note: Removed loading here to avoid double loading states // Suspense fallback will handle all loading states uniformly } );apps/web/core/components/common/skeleton/user-nav-avatar-skeleton.tsx (1)
1-19: Consider export consistency and sizing improvements.The component is well-implemented but could benefit from export pattern consistency and more flexible sizing.
- Export as default to match other skeleton components:
-export function UserNavAvatarSkeleton({ className }: UserNavAvatarSkeletonProps) { +const UserNavAvatarSkeleton = ({ className }: UserNavAvatarSkeletonProps) => { return ( // ... component body ); -} +}; + +export default UserNavAvatarSkeleton;
- Consider making the sizes configurable via props for reusability:
interface UserNavAvatarSkeletonProps { className?: string; + size?: 'sm' | 'md' | 'lg'; }apps/web/core/components/common/skeleton/team-invitations-skeleton.tsx (1)
4-34: Well-structured skeleton component!The component properly uses
EverCardfor consistency and implements appropriate skeleton placeholders. Consider adding aria-label for accessibility.Add accessibility attributes to improve screen reader support:
const TeamInvitationsSkeleton = ({ className }: { className?: string }) => { return ( - <div className={clsxm('mt-6', className)}> + <div className={clsxm('mt-6', className)} aria-label="Loading team invitations">apps/web/core/components/teams/team-invitations.tsx (1)
17-18: Consider improving prop interface naming and types.The
myInvitationsprop as a function() => voidseems unclear. Consider renaming it toonRefreshInvitationsor similar to better indicate its purpose.interface IProps { className?: string; myInvitationsList: TInvite[]; - myInvitations: () => void; + onRefreshInvitations: () => void; }Then update the destructuring and usage accordingly:
-const { className, myInvitationsList, myInvitations } = props; +const { className, myInvitationsList, onRefreshInvitations } = props; -myInvitations(); +onRefreshInvitations();apps/web/core/components/common/skeleton/task-timer-section-skeleton.tsx (1)
100-109: Consider extracting shimmer animation to a global CSS file.The shimmer animation is well-implemented but could be reused across other skeleton components. Consider moving it to a global CSS file or creating a reusable shimmer utility class.
- {/* Custom styles for shimmer animation */} - <style jsx>{` - @keyframes shimmer { - 0% { - transform: translateX(-100%); - } - 100% { - transform: translateX(100%); - } - } - `}</style> + {/* Shimmer animation can be extracted to global CSS */}Create a global CSS class like
.animate-shimmerthat can be reused across all skeleton components.apps/web/core/components/common/skeleton/app-sidebar-skeleton.tsx (1)
104-106: Consider making the number of report items configurable.The Reports section hardcodes 6 skeleton items. Consider making this configurable based on user roles or actual data expectations.
interface AppSidebarSkeletonProps { className?: string; + reportItemCount?: number; } -export function AppSidebarSkeleton({ className }: AppSidebarSkeletonProps) { +export function AppSidebarSkeleton({ className, reportItemCount = 6 }: AppSidebarSkeletonProps) { - {[...Array(6)].map((_, i) => ( + {[...Array(reportItemCount)].map((_, i) => (apps/web/core/components/common/skeleton/team-members-skeleton.tsx (1)
67-76: Fix the useless switch case.The static analysis tool correctly identifies that the
IssuesView.CARDScase is redundant since there's a default clause that handles the same case.const renderContent = () => { switch (view) { case IssuesView.TABLE: return renderTableView(); case IssuesView.BLOCKS: return renderBlockView(); - case IssuesView.CARDS: default: return renderCardView(); } };apps/web/core/components/common/skeleton/team-member-header-skeleton.tsx (1)
108-117: Fix the useless switch case (same issue as team-members-skeleton).The same static analysis issue exists here - the
IssuesView.CARDScase is redundant with the default clause.const renderHeaderSkeleton = () => { switch (view) { case IssuesView.TABLE: return renderTableHeaderSkeleton(); case IssuesView.BLOCKS: return renderBlockHeaderSkeleton(); - case IssuesView.CARDS: default: return renderCardHeaderSkeleton(); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
.cspell.json(8 hunks)apps/web/app/[locale]/page-component.tsx(3 hunks)apps/web/core/components/common/skeleton/app-sidebar-skeleton.tsx(1 hunks)apps/web/core/components/common/skeleton/auth-user-task-input-skeleton.tsx(1 hunks)apps/web/core/components/common/skeleton/collaborate-skeleton.tsx(1 hunks)apps/web/core/components/common/skeleton/modal-skeleton.tsx(1 hunks)apps/web/core/components/common/skeleton/no-team-skeleton.tsx(1 hunks)apps/web/core/components/common/skeleton/task-timer-section-skeleton.tsx(1 hunks)apps/web/core/components/common/skeleton/team-invitations-skeleton.tsx(1 hunks)apps/web/core/components/common/skeleton/team-member-header-skeleton.tsx(1 hunks)apps/web/core/components/common/skeleton/team-members-skeleton.tsx(1 hunks)apps/web/core/components/common/skeleton/team-notifications-skeleton.tsx(1 hunks)apps/web/core/components/common/skeleton/teams-dropdown-skeleton.tsx(1 hunks)apps/web/core/components/common/skeleton/timer-skeleton.tsx(1 hunks)apps/web/core/components/common/skeleton/unverified-email-skeleton.tsx(1 hunks)apps/web/core/components/common/skeleton/user-nav-avatar-skeleton.tsx(1 hunks)apps/web/core/components/common/unverified-email.tsx(1 hunks)apps/web/core/components/layouts/app-sidebar.tsx(4 hunks)apps/web/core/components/layouts/default-layout/main-layout.tsx(3 hunks)apps/web/core/components/layouts/default-layout/navbar.tsx(2 hunks)apps/web/core/components/pages/dashboard/task-timer-section.tsx(1 hunks)apps/web/core/components/teams/team-invitations.tsx(3 hunks)apps/web/core/components/teams/team-member-header.tsx(1 hunks)apps/web/core/components/teams/team-outstanding-notifications.tsx(3 hunks)apps/web/core/components/teams/teams-dropdown.tsx(2 hunks)apps/web/core/constants/config/notification.ts(1 hunks)apps/web/core/hooks/tasks/use-task-filter.ts(6 hunks)apps/web/core/lib/helpers/notifications.ts(1 hunks)apps/web/core/types/interfaces/common/notification.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Knip Review - Cleanup Unused Code - WEB
apps/web/core/components/common/unverified-email.tsx
[error] 1-1: Unused code detected in this file.
apps/web/core/components/teams/team-invitations.tsx
[error] 1-1: Unused code detected in this file.
apps/web/core/lib/helpers/notifications.ts
[error] 1-1: Unused code detected in this file.
apps/web/app/[locale]/page-component.tsx
[error] 1-1: Unused code detected in this file.
🪛 Biome (1.9.4)
apps/web/core/components/common/skeleton/team-member-header-skeleton.tsx
[error] 113-113: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
apps/web/core/components/common/skeleton/team-members-skeleton.tsx
[error] 72-72: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
🪛 GitHub Check: CodeQL
apps/web/core/lib/helpers/notifications.ts
[failure] 31-31: Clear text storage of sensitive information
This stores sensitive data returned by an access to uniqueEmployees as clear text.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: deploy
🔇 Additional comments (47)
apps/web/core/constants/config/notification.ts (1)
1-12: Well-structured notification configuration constants.The implementation follows TypeScript best practices with proper use of
as constfor immutability and reasonable configuration values for notification management..cspell.json (1)
11-571: Appropriate spell check dictionary updates.The new words align perfectly with the lazy loading and skeleton component functionality introduced in this PR. Terms like "skeleton", "Suspense", "collaborate", and "Optimized" support the new UI patterns.
apps/web/core/hooks/tasks/use-task-filter.ts (3)
68-72: Good defensive programming with optional chaining.The addition of optional chaining protects against runtime errors when
profile.tasksGroupedmight be undefined, which is especially important in lazy loading scenarios.
94-122: Consistent safe property access patterns.The nullish coalescing with
|| 0provides sensible defaults for count values, preventing NaN issues in the UI while maintaining backward compatibility.
230-230: Safe access to tasksGrouped property.Applying optional chaining to the returned
tasksGroupedproperty maintains consistency with the other safety improvements in this hook.apps/web/core/components/layouts/default-layout/main-layout.tsx (3)
3-4: Proper imports for lazy loading implementation.The addition of
Suspenseanddynamicimports correctly supports the lazy loading pattern being implemented.
19-25: Well-implemented lazy loading for AppSidebar.The dynamic import configuration is optimal:
ssr: falseprevents hydration issues with heavy hooks- Proper module resolution with default export handling
- Skeleton loading component provides good UX during loading
This follows Next.js best practices for performance optimization.
167-169: Proper Suspense wrapper with fallback.The Suspense implementation correctly provides a fallback skeleton while the lazy component loads, ensuring smooth user experience during the loading state.
apps/web/core/components/common/unverified-email.tsx (2)
15-22: Good implementation of optional user prop for flexibility.The addition of the optional
userprop with proper fallback to the hook maintains backward compatibility while enabling performance optimizations in parent components. The variable naming (propUservshookUser) clearly distinguishes the sources.
15-22: ```shell
#!/bin/bash
set -efile="apps/web/core/components/common/unverified-email.tsx"
echo "Analyzing imports and their usage in $file"Extract all named imports
grep -oP '^import .*{\K[^}]+' "$file" | tr ',' '\n' | sed 's/ //g' | while read -r name; do
Skip empty lines
[ -z "$name" ] && continue
Count occurrences of the symbol in the file
total=$(grep -ow "$name" "$file" | wc -l)
Subtract the one occurrence in the import statement
uses=$((total - 1))
if [ "$uses" -le 0 ]; then
echo "Unused import: $name"
else
echo "Used import: $name (uses: $uses)"
fi
done</details> <details> <summary>apps/web/core/components/teams/teams-dropdown.tsx (3)</summary> `14-26`: **Excellent lazy loading implementation!** The dynamic import setup for `CreateTeamModal` follows Next.js best practices with `ssr: false` and proper module resolution. The comment explaining the removal of the `loading` property to avoid double loading states is particularly valuable. --- `113-113`: **Minor className reordering noted.** The swap of `text-nowrap` and `whitespace-nowrap` classes doesn't affect functionality but maintains consistency. --- `119-123`: **Smart conditional rendering optimization.** The change from always rendering the modal with a conditional `open` prop to only mounting it when `isOpen && !!user?.isEmailVerified` is true provides better performance by avoiding unnecessary component creation. The Suspense wrapper with `ModalSkeleton` fallback ensures smooth loading states. </details> <details> <summary>apps/web/core/components/common/skeleton/teams-dropdown-skeleton.tsx (1)</summary> `1-16`: **Excellent skeleton component implementation.** The `TeamsDropDownSkeleton` maintains visual consistency with the actual dropdown component by using the same width constraints (`min-w-fit md:max-w-[223px]`) and styling patterns. The dark mode support and pulse animation provide a smooth loading experience. </details> <details> <summary>apps/web/core/components/common/skeleton/no-team-skeleton.tsx (1)</summary> `1-31`: **Comprehensive and well-structured skeleton component.** The `NoTeamSkeleton` provides an excellent placeholder with all necessary visual elements (icon, title, description, button). The `fullWidth` prop with sensible default and the responsive design with proper spacing create a polished loading experience. </details> <details> <summary>apps/web/core/components/common/skeleton/collaborate-skeleton.tsx (1)</summary> `1-17`: **Simple and effective button skeleton.** The `CollaborateSkeleton` provides a clean placeholder for the collaborate button with proper icon and text simulation. The styling is consistent with other skeleton components in the codebase and includes appropriate dark mode support. </details> <details> <summary>apps/web/core/components/common/skeleton/timer-skeleton.tsx (1)</summary> `16-16`: **LGTM! Consistent skeleton styling.** The skeleton blocks use consistent colors and animations that align with the project's design system. The dark mode support is properly implemented. Also applies to: 19-19 </details> <details> <summary>apps/web/core/components/common/skeleton/unverified-email-skeleton.tsx (1)</summary> `4-29`: **LGTM! Clean skeleton implementation.** The skeleton structure accurately represents an email verification UI with proper spacing, styling, and dark mode support. The use of EverCard provides consistent card styling. </details> <details> <summary>apps/web/core/components/pages/dashboard/task-timer-section.tsx (2)</summary> `9-19`: **LGTM! Proper dynamic import implementation.** The dynamic imports are correctly configured with `ssr: false` and appropriate loading fallbacks. This aligns with the PR objective of implementing lazy loading for performance optimization. --- `20-21`: **LGTM! Proper component structure and state management.** The component correctly implements conditional rendering, responsive design, and state management for the collapsible task input on mobile devices. Also applies to: 30-36 </details> <details> <summary>apps/web/core/components/common/skeleton/team-notifications-skeleton.tsx (1)</summary> `3-19`: **LGTM! Clean and consistent skeleton implementation.** The skeleton structure appropriately represents team notifications with proper spacing, consistent styling patterns, and dark mode support. The implementation follows established skeleton component conventions. </details> <details> <summary>apps/web/core/components/teams/team-member-header.tsx (2)</summary> `7-39`: **Excellent refactor to dynamic imports with proper configuration.** The transition from static to dynamic imports is well-executed with: - Proper `ssr: false` configuration - Clear comments explaining the loading state strategy - Consistent import patterns across all three components This aligns perfectly with the PR's lazy loading optimization objectives. --- `40-47`: **LGTM! Clean object lookup pattern.** The replacement of switch-case logic with `viewRenderMap` object lookup is a cleaner approach that improves readability and maintainability. The dynamic component selection works correctly. Also applies to: 50-50 </details> <details> <summary>apps/web/core/components/layouts/app-sidebar.tsx (2)</summary> `35-43`: **Well-implemented lazy loading pattern!** The dynamic import correctly uses module resolution and disables SSR. The comment explaining the removal of the loading property is helpful for future maintainers. --- `409-413`: **Excellent conditional rendering implementation!** The multiple conditions ensure the modal is only loaded when necessary, and the use of `ModalSkeleton` provides a consistent loading experience. </details> <details> <summary>apps/web/core/components/common/skeleton/modal-skeleton.tsx (1)</summary> `8-44`: ```shell #!/bin/bash # Re-run z-index search without unsupported --type flags, using globs instead rg -n 'z-\[?[0-9]+\]?|z-index:\s*[0-9]+' -g '*.tsx' -g '*.jsx' -g '*.css' -A2 -B2apps/web/core/components/layouts/default-layout/navbar.tsx (4)
9-57: Excellent lazy loading implementation!The consistent pattern of dynamic imports with
ssr: falseand unified loading state management through Suspense is well-executed.
116-120: Clean implementation of conditional lazy loading!The timer component is only loaded when needed, with an appropriate skeleton fallback.
123-133: Well-structured conditional rendering with proper fallbacks!The use of
publicTeam || falseensures the component always receives a boolean value, preventing potential undefined issues.
144-148: Efficient modal loading implementation!The modal is only loaded when open, reducing initial bundle size and improving performance.
apps/web/core/components/teams/team-invitations.tsx (3)
85-85: Approve the CSS class reordering.The reordering of flexbox classes maintains the same visual result while potentially improving consistency with the project's styling patterns.
160-176: Approve the modal styling improvements.The CSS class updates in the modal improve layout consistency and maintain proper styling for both light and dark themes.
13-13: Verify if TInvite import is actually used.The pipeline failure indicates unused code in this file. Check if all imports, particularly
TInvite, are being used throughout the component.#!/bin/bash # Description: Check usage of TInvite import in the file # Expected: Find references to TInvite being used in the component rg -A 5 -B 5 "TInvite" apps/web/core/components/teams/team-invitations.tsxapps/web/core/components/common/skeleton/task-timer-section-skeleton.tsx (2)
8-9: Good design decision: Internal state in skeleton component.Using internal state for the mobile toggle in a skeleton component is appropriate here as it provides a better user experience by maintaining interactivity during loading states.
52-54: Excellent accessibility consideration.The screen reader text provides proper accessibility for the toggle button, ensuring the component is usable by assistive technologies.
apps/web/core/components/common/skeleton/app-sidebar-skeleton.tsx (2)
16-21: Well-structured skeleton implementation.The skeleton correctly uses the actual Sidebar components while providing placeholder content. The absolute positioning of the SidebarTrigger matches the expected behavior of the real component.
81-86: Smart use of array mapping for repeated elements.Using
Array.from({ length: 3 })to generate project items with circular icons is an efficient way to create consistent skeleton structure without code duplication.apps/web/core/components/common/skeleton/team-members-skeleton.tsx (2)
12-12: Good practice: Avoiding hooks in skeleton components.The comment about removing
useAtomValuehook is crucial for Suspense fallback compatibility. This shows good understanding of React Suspense limitations.
46-64: Well-implemented block view skeleton.The block view skeleton properly represents the expected layout with avatar, name, role, and additional content placeholders. The responsive grid layout is appropriate.
apps/web/core/components/common/skeleton/team-member-header-skeleton.tsx (3)
15-15: Consistent Suspense compatibility approach.Like the team-members-skeleton, this component correctly avoids hooks to prevent Suspense rendering issues. This consistency shows good architectural planning.
17-58: Detailed and accurate card header skeleton.The card header skeleton accurately represents the column structure with proper spacing, separators, and responsive classes. The skeleton dimensions and positioning match the expected layout.
84-105: Well-designed block header skeleton.The block header skeleton includes proper status filter buttons, icons, and controls layout. The use of array mapping for repeated elements is consistent with other skeleton components.
apps/web/core/components/common/skeleton/auth-user-task-input-skeleton.tsx (1)
1-216: Well-implemented skeleton component with consistent design patterns.The skeleton component follows best practices with proper shimmer animations, dark mode support, and responsive design. The consistent styling approach across all dropdown controls enhances maintainability.
apps/web/core/lib/helpers/notifications.ts (1)
1-99: I couldn’t detect any usages with the previous query—let’s broaden the search to all helpers and functions in both .ts and .tsx files:#!/bin/bash echo "Searching for imports of the notifications helper module…" rg -g '*.ts' -g '*.tsx' "helpers/notifications" echo -e "\nSearching for usages of exported functions/constants…" for symbol in getNotificationState setNotificationState createTaskHash shouldShowNotification dismissNotification markOutstandingTasksVisited HAS_VISITED_OUTSTANDING_TASKS; do echo -e "\n🔍 $symbol:" rg -g '*.ts' -g '*.tsx' "$symbol" doneThis will confirm whether those exports are truly referenced elsewhere.
apps/web/app/[locale]/page-component.tsx (1)
36-90: Excellent implementation of lazy loading with Suspense boundaries.The refactoring successfully implements Next.js best practices for code splitting:
- All heavy components are dynamically imported with
ssr: false- Proper Suspense boundaries with skeleton fallbacks
- Comments clearly explain the removal of loading props to avoid double loading states
- Conditional rendering ensures components only load when needed
This should significantly improve initial page load performance as mentioned in the PR objectives (223-334KB bundle size reduction).
Also applies to: 144-204
apps/web/core/components/teams/team-outstanding-notifications.tsx (2)
20-41: Good refactoring to prop-based component architecture.The component now follows better React patterns by:
- Receiving data as props instead of fetching internally
- Improving testability and reusability
- Maintaining clear separation of concerns
54-61: Efficient uniqueness implementation using Set and Map.The refactored filtering logic is more performant:
- Using
Setfor unique task IDs eliminates duplicates efficiently- Using
Mapfor unique employees prevents duplicate entries- Clear and readable implementation
Also applies to: 125-133
apps/web/core/components/pages/dashboard/task-timer-section.tsx
Outdated
Show resolved
Hide resolved
Please review these files and clean up the unused code. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/core/components/common/skeleton/team-member-header-skeleton.tsx (1)
17-105: Consider extracting repeated skeleton styles into a utility.The skeleton styling pattern
bg-[#F0F0F0] dark:bg-[#353741] animate-pulse roundedis repeated extensively throughout the component. This creates maintenance overhead and inconsistency risk.Consider creating a utility class or constant:
+const SKELETON_BASE_CLASSES = "bg-[#F0F0F0] dark:bg-[#353741] animate-pulse rounded"; // Then use throughout the component: -<div className="w-32 h-4 bg-[#F0F0F0] dark:bg-[#353741] animate-pulse rounded" /> +<div className={`w-32 h-4 ${SKELETON_BASE_CLASSES}`} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/web/core/components/common/auto-complete-dropdown.tsx(1 hunks)apps/web/core/components/common/skeleton/collaborate-skeleton.tsx(1 hunks)apps/web/core/components/common/skeleton/team-member-header-skeleton.tsx(1 hunks)apps/web/core/components/layouts/app-sidebar.tsx(4 hunks)apps/web/core/components/layouts/default-layout/navbar.tsx(2 hunks)apps/web/core/components/pages/dashboard/task-timer-section.tsx(1 hunks)apps/web/core/components/pages/teams/team/team-members-views/user-team-block/user-team-block-header.tsx(1 hunks)apps/web/core/components/teams/team-member-header.tsx(1 hunks)apps/web/core/hooks/tasks/use-task-filter.ts(6 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/web/core/components/pages/teams/team/team-members-views/user-team-block/user-team-block-header.tsx
- apps/web/core/components/common/auto-complete-dropdown.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/web/core/hooks/tasks/use-task-filter.ts
- apps/web/core/components/teams/team-member-header.tsx
- apps/web/core/components/layouts/default-layout/navbar.tsx
- apps/web/core/components/layouts/app-sidebar.tsx
- apps/web/core/components/common/skeleton/collaborate-skeleton.tsx
- apps/web/core/components/pages/dashboard/task-timer-section.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
apps/web/core/components/common/skeleton/team-member-header-skeleton.tsx
[error] 113-113: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: deploy
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
apps/web/core/components/common/skeleton/team-member-header-skeleton.tsx (3)
1-8: LGTM! Clean imports and interface design.The imports are appropriate and the interface follows good TypeScript practices with optional props and clear naming.
10-16: Excellent approach for Suspense compatibility.The component correctly avoids hooks and accepts props, which prevents the rendering issues mentioned in the comment. The default values are sensible.
119-124: Perfect component structure and prop forwarding.The return statement correctly wraps the skeleton in a Container and forwards all necessary props. The conditional rendering logic is clean and maintainable.
apps/web/core/components/common/skeleton/team-member-header-skeleton.tsx
Outdated
Show resolved
Hide resolved
Please review these files and clean up the unused code. |
Please review these files and clean up the unused code. |
🚀 Optimize Dashboard Page with Lazy Loading and Unified Loading States
This PR : (WIP for #3789 )
Implement comprehensive lazy loading optimization across MainLayout, dashboard page, and skeleton management following Next.js article best practices.
Description
Please describe what you did, and why.
This PR implements comprehensive lazy loading optimization across the entire application:
dynamic()importsThese changes improve initial page load performance by 223-334KB bundle reduction, eliminate loading state flickering, and provide a seamless user experience with proper skeleton loading states.
What Was Changed
Major Changes
Here are the major changes that this PR adds:
dynamic()imports withssr: falsefor AppSidebar, Navbar, TeamMemberHeader, and Modal componentsloadingproperties fromdynamic()to avoid double loading states (following Next.js article best practices)AppSidebarSkeleton,NavbarSkeleton,TeamMemberHeaderSkeleton,NoTeamSkeletonfollowing[ComponentName]Skeleton.tsxconvention<Suspense>wrappers with appropriate fallback skeletons{condition && (<Suspense fallback={<Skeleton />}><LazyComponent /></Suspense>)}useAtomValuehooks from skeleton componentsMinor Changes
Here are the minor changes that this PR adds:
fullWidthpassed as prop)apps/web/ai-guides/How to Test This PR
Please explain clearly how to test the changes locally:
yarn web:devhttp://localhost:3030If this PR affects UI:
Screenshots (if needed)
You can also add videos or logs here.
Previous screenshots
Current screenshots
Uploading ever-teams-show.mp4…
th proper bundle splitting
ever-teams-show.mp4
Related Issues
Please list related issues, tasks or discussions:
Type of Change
✅ Checklist
Please confirm you did the following before asking for review:
Notes for the Reviewer (Optional)
Add here any context, help, or known issues for the person reviewing:
loadingproperties fromdynamic()imports to avoid double loading statesapps/web/ai-guides/Summary by CodeRabbit
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores
Documentation