[Feat]: Team Dashboard UI Enhancements #3530
Conversation
|
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]/dashboard/team-dashboard/components/team-stats-chart.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. WalkthroughThis pull request introduces several new components for the team dashboard in a web application. The changes include creating components for displaying team statistics, such as Changes
Sequence DiagramsequenceDiagram
participant User
participant TeamDashboard
participant DashboardHeader
participant DateRangePicker
participant TeamStatsGrid
participant TeamStatsChart
participant TeamStatsTable
User->>TeamDashboard: Access Dashboard
TeamDashboard->>DashboardHeader: Render Header
TeamDashboard->>DateRangePicker: Initialize Date Range
TeamDashboard->>TeamStatsGrid: Display Statistics
TeamDashboard->>TeamStatsChart: Render Performance Chart
TeamDashboard->>TeamStatsTable: Show Team Member Details
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
|
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (9)
apps/web/components/ui/chart.tsx (3)
66-66: EnsuredisplayNamematches the component nameThe
displayNameforChartContaineris set to"Chart". For clarity and consistency, consider setting it to"ChartContainer".Apply this diff to update the
displayName:-ChartContainer.displayName = "Chart" +ChartContainer.displayName = "ChartContainer"
255-255: AligndisplayNamewith component nameThe
displayNameforChartTooltipContentis set to"ChartTooltip". For consistency, consider setting it to"ChartTooltipContent".Apply this diff to update the
displayName:-ChartTooltipContent.displayName = "ChartTooltip" +ChartTooltipContent.displayName = "ChartTooltipContent"
315-315: AligndisplayNamewith component nameThe
displayNameforChartLegendContentis set to"ChartLegend". For consistency, consider setting it to"ChartLegendContent".Apply this diff to update the
displayName:-ChartLegendContent.displayName = "ChartLegend" +ChartLegendContent.displayName = "ChartLegendContent"apps/web/app/[locale]/dashboard/team-dashboard/data/mock-data.ts (1)
46-74: Use consistent data types for time values and activity levelsCurrently, time values like
trackedTime,manualTime,idleTime, andunknownActivityare stored as strings (e.g.,"8h 12m"), andactivityLevelis also a string (e.g.,"85%"). Storing these values as numbers in consistent units (e.g., total minutes for time and a number between 0 and 100 for activity level) would facilitate easier data manipulation and calculations.apps/web/app/[locale]/dashboard/team-dashboard/components/team-stats-chart.tsx (1)
80-103: Add interactivity to legend buttons for filtering data.The legend buttons are currently non-functional. They should toggle the visibility of corresponding lines in the chart.
+import { useState } from 'react'; export function TeamStatsChart() { + const [visibleLines, setVisibleLines] = useState({ + tracked: true, + manual: true, + idle: true + }); + + const toggleLine = (line: keyof typeof visibleLines) => { + setVisibleLines(prev => ({ + ...prev, + [line]: !prev[line] + })); + }; return ( // ... chart code <div className="flex gap-3 justify-center -mt-2"> <Button size="sm" variant="outline" + onClick={() => toggleLine('tracked')} + className={cn( "gap-2 px-3 py-1.5 h-8 text-xs font-normal", + !visibleLines.tracked && "opacity-50" )} > <div className="w-2 h-2 bg-blue-500 rounded-full" /> Tracked </Button> // ... similar changes for other buttons </div> ); }apps/web/app/[locale]/dashboard/team-dashboard/page.tsx (1)
36-66: Add error boundaries and loading states.The dashboard should gracefully handle errors and loading states for its child components.
import { ErrorBoundary } from '@/components/error-boundary'; import { Suspense } from 'react'; import { LoadingSkeleton } from '@/components/loading-skeleton'; // Wrap child components with error boundaries and loading states <ErrorBoundary fallback={<ErrorFallback />}> <Suspense fallback={<LoadingSkeleton />}> <DashboardHeader /> <TeamStatsGrid /> <Card className="p-6 w-full"> <TeamStatsChart /> </Card> </Suspense> </ErrorBoundary>apps/web/app/[locale]/dashboard/team-dashboard/components/date-range-picker.tsx (3)
21-32: Consider initializing dateRange as undefined.The initial dateRange is set to today's date, but this might not align with the UI's "Select date range" placeholder state. Consider initializing it as undefined for consistency with the unselected state.
- const [dateRange, setDateRange] = React.useState<DateRange | undefined>({ - from: new Date(), - to: new Date(), - }); + const [dateRange, setDateRange] = React.useState<DateRange | undefined>(undefined);
34-127: Enhance code reusability and type safety.The predefined ranges implementation could benefit from:
- A type-safe structure for range definitions
- A reusable helper for null checks
- A constant for weekStartsOn
+interface DateRangeOption { + label: string; + action: () => void; + isSelected: (range: DateRange | undefined) => boolean; +} + +const WEEK_STARTS_ON = 1; // Monday + +const isValidDateRange = (range: DateRange | undefined): range is Required<DateRange> => { + return Boolean(range?.from && range?.to); +}; -const predefinedRanges = [ +const predefinedRanges: DateRangeOption[] = [ // ... existing ranges with updated null checks: { isSelected: (range: DateRange | undefined) => { - if (!range?.from || !range?.to) return false; + if (!isValidDateRange(range)) return false; // ... rest of the implementation } } // ... update other ranges similarly ];
207-213: Consider removing the Apply button.The Apply button might be redundant since:
- Changes are already applied through handleDateRangeChange
- It only closes the popover, which could be handled by the selection itself
Consider either:
- Removing the Apply button and closing the popover on selection
- Or, implementing a preview state that only applies on clicking Apply
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
apps/web/app/[locale]/dashboard/app-url/page.tsx(1 hunks)apps/web/app/[locale]/dashboard/team-dashboard/components/dashboard-header.tsx(1 hunks)apps/web/app/[locale]/dashboard/team-dashboard/components/date-range-picker.tsx(1 hunks)apps/web/app/[locale]/dashboard/team-dashboard/components/team-stats-chart.tsx(1 hunks)apps/web/app/[locale]/dashboard/team-dashboard/components/team-stats-grid.tsx(1 hunks)apps/web/app/[locale]/dashboard/team-dashboard/components/team-stats-table.tsx(1 hunks)apps/web/app/[locale]/dashboard/team-dashboard/data/mock-data.ts(1 hunks)apps/web/app/[locale]/dashboard/team-dashboard/page.tsx(1 hunks)apps/web/components/ui/card.tsx(1 hunks)apps/web/components/ui/chart.tsx(1 hunks)apps/web/package.json(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/web/components/ui/card.tsx
- apps/web/app/[locale]/dashboard/app-url/page.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
apps/web/components/ui/chart.tsx
[error] 79-79: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (3)
apps/web/app/[locale]/dashboard/team-dashboard/components/date-range-picker.tsx (2)
1-19: LGTM! Well-structured component setup.The imports are well-organized, and the interface is properly typed with optional props.
129-140: LGTM! Well-implemented date formatting logic.The formatDateRange function handles all cases appropriately and uses date-fns utilities effectively.
apps/web/package.json (1)
101-101: Verify compatibility with updated dependencies.The dependency updates look appropriate for the UI enhancements. However, please ensure:
- The lucide-react update doesn't introduce breaking changes
- The recharts version is compatible with the React version in use (^18.2.0)
Also applies to: 128-128
✅ Verification successful
Dependencies are compatible with the project
Both lucide-react 0.473.0 and recharts 2.15.0 explicitly support React ^18.2.0, and the version updates follow semver guidelines, suggesting no breaking changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for breaking changes in lucide-react and verify recharts compatibility # Check lucide-react changelog gh api repos/lucide-icons/lucide/releases | jq -r '.[].body' | grep -A 10 "0.453.0" # Verify recharts peer dependencies curl -s https://registry.npmjs.org/recharts/2.15.0 | jq -r '.peerDependencies'Length of output: 279
Script:
#!/bin/bash # Try to get lucide-react release info from npm curl -s https://registry.npmjs.org/lucide-react | jq -r '.versions["0.473.0"].dist.tarball' | xargs curl -s | tar -tzf - package/README.md 2>/dev/null # Also check the current version's dependencies curl -s https://registry.npmjs.org/lucide-react/0.473.0 | jq -r '.peerDependencies'Length of output: 300
apps/web/app/[locale]/dashboard/team-dashboard/components/dashboard-header.tsx
Show resolved
Hide resolved
apps/web/app/[locale]/dashboard/team-dashboard/components/team-stats-grid.tsx
Show resolved
Hide resolved
apps/web/app/[locale]/dashboard/team-dashboard/components/team-stats-grid.tsx
Show resolved
Hide resolved
apps/web/app/[locale]/dashboard/team-dashboard/components/team-stats-chart.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/[locale]/dashboard/team-dashboard/components/team-stats-table.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/[locale]/dashboard/team-dashboard/components/team-stats-table.tsx
Outdated
Show resolved
Hide resolved
|
Report too large to display inline |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/web/app/[locale]/dashboard/team-dashboard/page.tsx (1)
28-35:⚠️ Potential issueRemove unsafe JSON.parse usage with translation string.
Using JSON.parse with translation strings is unsafe and could throw errors if the string is not valid JSON.
Apply this diff to fix the issue:
-{ title: JSON.parse(t('pages.home.BREADCRUMB')), href: '/' }, +{ title: t('pages.home.BREADCRUMB'), href: '/' },apps/web/app/[locale]/dashboard/team-dashboard/components/team-stats-table.tsx (1)
65-89: 🛠️ Refactor suggestionImplement pagination functionality.
The pagination UI is present but non-functional. Consider implementing proper pagination logic to enhance user experience.
Apply this diff to implement pagination:
+import { useState } from 'react'; + export function TeamStatsTable() { + const [currentPage, setCurrentPage] = useState(1); + const itemsPerPage = 10; + const totalPages = Math.ceil(members.length / itemsPerPage); + + const paginatedMembers = members.slice( + (currentPage - 1) * itemsPerPage, + currentPage * itemsPerPage + ); + return ( <div className="space-y-4"> <TableBody> - {members.map((member) => ( + {paginatedMembers.map((member) => ( // ... existing table row code ))} </TableBody> <div className="flex justify-between items-center px-2"> <div className="flex items-center space-x-6"> <p className="text-sm text-gray-500"> - Showing 1 to {members.length} of {members.length} entries + Showing {(currentPage - 1) * itemsPerPage + 1} to {Math.min(currentPage * itemsPerPage, members.length)} of {members.length} entries </p> </div> <div className="flex items-center space-x-2"> <Button variant="outline" size="icon" - disabled + disabled={currentPage === 1} + onClick={() => setCurrentPage(1)} > <ChevronsLeft className="w-4 h-4" /> </Button> // ... update other pagination buttons similarly </div> </div> </div> ); }
🧹 Nitpick comments (4)
apps/web/components/ui/chart.tsx (3)
9-17: Consider adding type safety for color values.The
colorproperty inChartConfigcould benefit from stronger typing to ensure valid color values are provided.export type ChartConfig = { [k in string]: { label?: React.ReactNode icon?: React.ComponentType } & ( - | { color?: string; theme?: never } + | { color?: `#${string}` | `rgb(${number},${number},${number})` | `rgba(${number},${number},${number},${number})`; theme?: never } | { color?: never; theme: Record<keyof typeof THEMES, string> } ) }
103-255: Consider decomposing the tooltip content into smaller components.The
ChartTooltipContentcomponent is quite large and handles multiple responsibilities. Breaking it down would improve maintainability and readability.Consider extracting these components:
TooltipLabelTooltipItemTooltipIndicatorExample structure:
const TooltipLabel = ({ value, formatter, payload, className }: TooltipLabelProps) => { if (formatter) { return <div className={className}>{formatter(value, payload)}</div>; } return value ? <div className={className}>{value}</div> : null; }; const TooltipIndicator = ({ type, color }: TooltipIndicatorProps) => { // Extract indicator rendering logic }; const TooltipItem = ({ item, config, formatter, index }: TooltipItemProps) => { // Extract item rendering logic }; const ChartTooltipContent = ({ ... }) => { // Main component becomes simpler with extracted components return ( <div> <TooltipLabel /> {payload.map((item, index) => ( <TooltipItem key={item.dataKey} item={item} index={index} /> ))} </div> ); };
318-354: Enhance type safety in the helper function.The
getPayloadConfigFromPayloadfunction uses type assertions and loose type checking. Consider using TypeScript's type guards for better type safety.+ interface PayloadType { + payload?: Record<string, unknown>; + [key: string]: unknown; + } function getPayloadConfigFromPayload( config: ChartConfig, - payload: unknown, + payload: PayloadType, key: string ) { - if (typeof payload !== "object" || payload === null) { + if (!payload) { return undefined; } + function isValidPayload(value: unknown): value is Record<string, unknown> { + return typeof value === "object" && value !== null; + } const payloadPayload = - "payload" in payload && - typeof payload.payload === "object" && - payload.payload !== null + "payload" in payload && isValidPayload(payload.payload) ? payload.payload : undefined; // ... rest of the function }apps/web/app/[locale]/dashboard/team-dashboard/components/team-stats-chart.tsx (1)
88-113: Add interactive legend functionality.Consider making the legend buttons interactive to toggle the visibility of corresponding chart lines, enhancing user experience.
Apply this diff to implement interactive legend:
+import { useState } from 'react'; + export function TeamStatsChart() { + const [visibleLines, setVisibleLines] = useState({ + tracked: true, + manual: true, + idle: true + }); + + const toggleLine = (line: keyof typeof visibleLines) => { + setVisibleLines(prev => ({ + ...prev, + [line]: !prev[line] + })); + }; + return ( <div className="flex flex-col"> <LineChart> {/* ... other chart config ... */} - <Line dataKey="tracked" /> - <Line dataKey="manual" /> - <Line dataKey="idle" /> + {visibleLines.tracked && <Line dataKey="tracked" />} + {visibleLines.manual && <Line dataKey="manual" />} + {visibleLines.idle && <Line dataKey="idle" />} </LineChart> <div className="flex gap-3 justify-center -mt-2"> <Button variant="outline" + onClick={() => toggleLine('tracked')} + className={cn( + "gap-2 px-3 py-1.5 h-8 text-xs font-normal", + !visibleLines.tracked && "opacity-50" + )} > <div className="w-2 h-2 bg-blue-500 rounded-full" /> Tracked </Button> {/* ... update other buttons similarly ... */} </div> </div> ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
apps/web/app/[locale]/dashboard/team-dashboard/components/team-stats-chart.tsx(1 hunks)apps/web/app/[locale]/dashboard/team-dashboard/components/team-stats-table.tsx(1 hunks)apps/web/app/[locale]/dashboard/team-dashboard/page.tsx(1 hunks)apps/web/components/ui/chart.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/web/components/ui/chart.tsx
[error] 79-79: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
apps/web/app/[locale]/dashboard/team-dashboard/components/team-stats-table.tsx
[error] 11-11: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (9)
apps/web/components/ui/chart.tsx (3)
35-66: LGTM! Well-structured component with proper TypeScript usage.The component correctly implements ref forwarding and provides comprehensive styling options.
68-99: Consider safer alternatives todangerouslySetInnerHTML.While the current implementation is controlled and likely safe, consider using CSS-in-JS solutions or styled-components for dynamic styling to eliminate potential XSS risks.
Alternative approaches:
- Use CSS-in-JS libraries like
@emotion/cssorstyled-components- Use CSS Custom Properties with dynamic values
- Create style elements programmatically using
CSSStyleSheetAPIExample using CSS Custom Properties:
const ChartStyle = ({ id, config }: { id: string; config: ChartConfig }) => { const colorConfig = Object.entries(config).filter( ([, config]) => config.theme || config.color ) React.useEffect(() => { if (!colorConfig.length) return; Object.entries(THEMES).forEach(([theme, prefix]) => { const element = document.querySelector(`${prefix} [data-chart="${id}"]`); if (!element) return; colorConfig.forEach(([key, itemConfig]) => { const color = itemConfig.theme?.[theme as keyof typeof itemConfig.theme] || itemConfig.color; if (color) { (element as HTMLElement).style.setProperty(`--color-${key}`, color); } }); }); }, [id, colorConfig]); return null; }🧰 Tools
🪛 Biome (1.9.4)
[error] 79-79: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
257-315: LGTM! Clean implementation of the legend components.The components are well-typed and properly handle refs and customization options.
apps/web/app/[locale]/dashboard/team-dashboard/page.tsx (3)
1-26: LGTM! Imports and hooks are well organized.The imports are properly structured, and hooks are initialized correctly.
37-64: LGTM! Layout structure is well-organized.The header section is properly structured with correct back button implementation and well-nested components.
65-77: LGTM! Main content and authentication are properly implemented.The main content is well-structured, and the authentication HOC is correctly applied.
apps/web/app/[locale]/dashboard/team-dashboard/components/team-stats-table.tsx (1)
18-64: LGTM! Table implementation is well-structured.The table layout is clean, and the activity level progress bar is properly implemented.
apps/web/app/[locale]/dashboard/team-dashboard/components/team-stats-chart.tsx (2)
7-30: LGTM! CustomTooltip is well-implemented with proper TypeScript types.The tooltip implementation is clean, type-safe, and follows best practices.
32-87: LGTM! Chart implementation is well-configured.The chart is properly set up with appropriate styling, axes configuration, and responsive container.
apps/web/app/[locale]/dashboard/team-dashboard/components/team-stats-table.tsx
Show resolved
Hide resolved
Description
#3251
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
Dependencies
lucide-reactto version 0.473.0.rechartslibrary for charting functionality.Chores