feat(dashboard): redesign metric drawer with larger chart and better layout #203
Conversation
…t and native overflow
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe pull request restructures two dashboard metric UI components. The metric card's drawer layout transitions from responsive max-height/width constraints to a fixed 96vh flex column. The metric drawer undergoes extensive UI reorganization, replacing a scroll-area-based multi-column layout with a modular card-based grid featuring prominent chart area, configuration panels, and data pipeline health sections. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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: 1
🧹 Nitpick comments (5)
src/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx (5)
111-111: Unused prop:_isUpdatingis never referenced.The
isUpdatingprop is aliased to_isUpdatingand never used in the component. Either remove it from the props interface or use it (e.g., to disable the save button or show a loading state).🔎 If the prop is intentionally unused, remove it from the interface
interface DashboardMetricDrawerProps { // ... - isUpdating: boolean; // ... } export function DashboardMetricDrawer({ // ... - isUpdating: _isUpdating, // ... }: DashboardMetricDrawerProps) {
120-120: Misleading underscore prefix on_onClose.The underscore prefix conventionally indicates an unused variable, but
_onCloseis actually used inhandleDelete(line 162). Rename it toonClosefor clarity.🔎 Proposed fix
- onClose: _onClose, + onClose, }: DashboardMetricDrawerProps) {And update line 162:
const handleDelete = () => { onDelete(); - _onClose(); + onClose(); };
454-466: Inconsistent disabled logic between regenerate buttons.The "Apply Customization" button (line 458) is disabled only when
isProcessing, but the Quick Actions "Regenerate" button (line 400) also checks!hasChartChanges. Consider aligning the behavior—if there are no changes to apply, this button should probably also be disabled or provide visual feedback.🔎 Proposed fix for consistent behavior
<Button size="sm" className="w-full" onClick={handleApplyChanges} - disabled={isProcessing} + disabled={isProcessing || !hasChartChanges} >
491-529: Section heading renders without content whenteamIdis null.The "Goals & Alignment" heading (lines 493-496) is always rendered, but the content (lines 497-528) is conditional on
teamId. This results in an empty section with just a heading when no team context exists.🔎 Proposed fix: wrap entire section conditionally
- {/* Col 2: Business Goals */} - <div className="space-y-4"> - <h4 className="flex items-center gap-2 font-semibold"> - <Target className="h-4 w-4" /> - Goals & Alignment - </h4> - {teamId && ( - <div className="space-y-4"> + {/* Col 2: Business Goals */} + {teamId && ( + <div className="space-y-4"> + <h4 className="flex items-center gap-2 font-semibold"> + <Target className="h-4 w-4" /> + Goals & Alignment + </h4> + <div className="space-y-4"> {/* ... cards ... */} </div> - )} - </div> + </div> + )}
556-570: "Rebuild Transformers" should be disabled during any processing.The button is only disabled when
isRegeneratingPipeline(line 560), but it should likely also be disabled whenisProcessingto prevent conflicting operations.🔎 Proposed fix
<Button variant="secondary" size="sm" onClick={onRegeneratePipeline} - disabled={isRegeneratingPipeline} + disabled={isProcessing || isRegeneratingPipeline} className="h-8 w-full text-xs" >
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsx(1 hunks)src/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx(6 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use the tRPC dual API pattern: for server components, directly call tRPC server API (10x faster); for client components, use React hooks with TanStack Query
Use inline type imports with @trivago/prettier-plugin-sort-imports for consistent import ordering
Files:
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
src/app/dashboard/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Three-stage metrics transformation pipeline: Stage 1 (API → DataPoints via DataIngestionTransformer), Stage 2 (DataPoints → ChartConfig via ChartTransformer), Stage 3 (ChartConfig → UI via DashboardMetricChart with Recharts)
Files:
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
src/app/dashboard/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Consolidate dashboard metric card duplication: add readOnly mode to dashboard-metric-card.tsx instead of maintaining separate public-dashboard-metric-card.tsx component
Files:
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.818Z
Learning: Applies to src/app/dashboard/**/*.tsx : Consolidate dashboard metric card duplication: add readOnly mode to dashboard-metric-card.tsx instead of maintaining separate public-dashboard-metric-card.tsx component
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.818Z
Learning: Applies to src/app/dashboard/**/*.{ts,tsx} : Three-stage metrics transformation pipeline: Stage 1 (API → DataPoints via DataIngestionTransformer), Stage 2 (DataPoints → ChartConfig via ChartTransformer), Stage 3 (ChartConfig → UI via DashboardMetricChart with Recharts)
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.818Z
Learning: Applies to src/app/metric/_components/**/*.tsx : Use MetricDialogBase from base/ for shared metric dialog functionality to reduce duplication across provider-specific dialogs
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.818Z
Learning: Applies to src/app/metric/_components/**/*.tsx : Consider factory pattern for metric dialogs to reduce the 5 nearly identical wrapper components (currently duplicated across providers)
📚 Learning: 2025-12-20T16:32:46.818Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.818Z
Learning: Applies to src/app/dashboard/**/*.tsx : Consolidate dashboard metric card duplication: add readOnly mode to dashboard-metric-card.tsx instead of maintaining separate public-dashboard-metric-card.tsx component
Applied to files:
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
📚 Learning: 2025-12-20T16:32:46.818Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.818Z
Learning: Applies to src/app/dashboard/**/*.{ts,tsx} : Three-stage metrics transformation pipeline: Stage 1 (API → DataPoints via DataIngestionTransformer), Stage 2 (DataPoints → ChartConfig via ChartTransformer), Stage 3 (ChartConfig → UI via DashboardMetricChart with Recharts)
Applied to files:
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
📚 Learning: 2025-12-20T16:32:46.818Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.818Z
Learning: Applies to src/app/metric/_components/**/*.tsx : Use MetricDialogBase from base/ for shared metric dialog functionality to reduce duplication across provider-specific dialogs
Applied to files:
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
📚 Learning: 2025-12-20T16:32:46.818Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.818Z
Learning: Applies to src/app/metric/_components/**/*.tsx : Consider factory pattern for metric dialogs to reduce the 5 nearly identical wrapper components (currently duplicated across providers)
Applied to files:
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
📚 Learning: 2025-12-20T16:32:46.818Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.818Z
Learning: Applies to src/app/metric/_components/**/*.tsx : Metric dialogs should follow the pattern: [Provider]MetricDialog.tsx (dialog wrapper) + [Provider]MetricContent.tsx (form content) and be registered in src/app/metric/_components/index.ts
Applied to files:
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
📚 Learning: 2025-12-20T16:32:46.818Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.818Z
Learning: Applies to src/app/teams/[teamId]/**/*.tsx : Consolidate role node duplication: extract shared RoleNodeTemplate from role-node.tsx and public-role-node.tsx with isEditable prop instead of maintaining separate components
Applied to files:
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
📚 Learning: 2025-12-20T16:32:46.818Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.818Z
Learning: Applies to **/*canvas*.{ts,tsx} : Use React Flow primitives (BaseNode, BaseHandle, ZoomSlider) from src/components/react-flow/ for consistent canvas styling
Applied to files:
src/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
🧬 Code graph analysis (2)
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsx (1)
src/components/ui/drawer.tsx (1)
DrawerContent(131-131)
src/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx (4)
src/lib/utils.ts (1)
cn(4-6)src/lib/helpers/format-value.ts (1)
formatValue(6-17)src/components/metric/goal-editor.tsx (1)
GoalEditor(86-398)src/components/metric/role-assignment.tsx (1)
RoleAssignment(39-236)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (2)
src/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx (1)
178-301: Well-structured redesign with clear visual hierarchy.The new modular card-based grid layout with a prominent hero chart section (450px), organized 3-column configuration grid, and consistent spacing improves the UX significantly. The separation of concerns (Chart Configuration, Goals & Alignment, Data Pipeline) is intuitive.
Also applies to: 411-599
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsx (1)
406-441: LGTM! Proper flex layout for the redesigned drawer.The
min-h-0on the inner wrapper is essential for proper overflow behavior within flex containers, and the fixed96vhheight provides a consistent full-screen drawer experience. The layout correctly enables the innerDashboardMetricDrawerto handle its own scrolling.
| <DrawerDescription className="flex items-center gap-3"> | ||
| {lastFetchedAt && ( | ||
| <span className="flex items-center gap-1.5 text-xs"> | ||
| <Clock className="h-3.5 w-3.5" /> | ||
| Updated{" "} | ||
| {formatDistanceToNow(new Date(lastFetchedAt), { | ||
| addSuffix: true, | ||
| })} | ||
| </span> | ||
| )} | ||
| <span className="text-border">|</span> | ||
| <span className="text-xs"> | ||
| Polls {pollFrequency.toLowerCase()} | ||
| </span> | ||
| {lastError && ( | ||
| <Badge variant="destructive" className="h-5 px-1.5 text-[10px]"> | ||
| Error | ||
| </Badge> | ||
| )} | ||
| </DrawerDescription> |
There was a problem hiding this comment.
Separator displays even when lastFetchedAt is missing.
The | separator (line 207) is always rendered regardless of whether lastFetchedAt exists, which will result in a leading separator when there's no timestamp.
🔎 Proposed fix: make the separator conditional
<DrawerDescription className="flex items-center gap-3">
{lastFetchedAt && (
- <span className="flex items-center gap-1.5 text-xs">
- <Clock className="h-3.5 w-3.5" />
- Updated{" "}
- {formatDistanceToNow(new Date(lastFetchedAt), {
- addSuffix: true,
- })}
- </span>
+ <>
+ <span className="flex items-center gap-1.5 text-xs">
+ <Clock className="h-3.5 w-3.5" />
+ Updated{" "}
+ {formatDistanceToNow(new Date(lastFetchedAt), {
+ addSuffix: true,
+ })}
+ </span>
+ <span className="text-border">|</span>
+ </>
)}
- <span className="text-border">|</span>
<span className="text-xs">
Polls {pollFrequency.toLowerCase()}
</span>🤖 Prompt for AI Agents
In src/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx around
lines 197 to 216, the vertical separator "|" is always rendered causing a
leading separator when lastFetchedAt is absent; update the JSX so the separator
is rendered only when lastFetchedAt exists (i.e., conditionally render the <span
className="text-border">|</span> together with the timestamp block), ensuring
spacing remains correct and other elements (poll frequency, error badge) render
as before.
Redesigned the dashboard metric drawer to provide a better user experience.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.