refactor(dashboard): improve drawer UI, loading states, and cache handling #213
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR consolidates metric pipeline regeneration into the refresh flow by adding a forceRebuild flag to handleRefresh, removes the separate regeneration handler, updates loading state messaging to distinguish refresh from regeneration, changes cache invalidation from invalidate to refetch, and enhances server-side regeneration with explicit refreshStatus lifecycle tracking. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 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: 0
🧹 Nitpick comments (3)
src/components/metric/goal-editor.tsx (2)
141-147: Consider handling potential refetch errors.If
refetch()throws,setIsCalculating(false)on line 146 won't execute, leaving the overlay stuck. Wrap the refetch calls in try/catch or movesetIsCalculating(false)to afinallyblock.🔎 Proposed fix
onSuccess: async () => { toast.success("Goal saved"); setIsEditing(false); - await utils.metric.getGoal.refetch({ metricId }); - await utils.dashboard.getDashboardCharts.refetch(); - setIsCalculating(false); + try { + await utils.metric.getGoal.refetch({ metricId }); + await utils.dashboard.getDashboardCharts.refetch(); + } finally { + setIsCalculating(false); + } onSave?.(); },
159-164: Same refetch error handling concern applies here.The
deleteGoalMutation.onSuccesshas the same pattern. Consider consistent error handling across both mutations.src/app/dashboard/[teamId]/_components/dashboard-metric-chart.tsx (1)
602-614: Consider usinggetLoadingMessagefor consistency.The badge uses a nested ternary with abbreviated messages while the overlay uses
getLoadingMessage. If the abbreviated messages are intentional for badge space constraints, consider creating a separategetShortLoadingMessagehelper to avoid duplicated logic.🔎 Proposed approach
function getShortLoadingMessage(phase: LoadingPhase | undefined): string { switch (phase) { case "fetching-api": return "Fetching..."; case "running-transformer": return "Transforming..."; case "ai-regenerating": return "AI Regenerating..."; case "saving-data": return "Saving..."; case "updating-chart": return "Updating..."; default: return "Processing..."; } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsx(2 hunks)src/app/dashboard/[teamId]/_components/dashboard-metric-chart.tsx(2 hunks)src/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx(8 hunks)src/components/metric/goal-editor.tsx(3 hunks)src/server/api/services/transformation/chart-generator.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{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 orderingUse TypeScript 5.9 with strict type checking for all frontend and backend code
Files:
src/components/metric/goal-editor.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-chart.tsxsrc/server/api/services/transformation/chart-generator.tssrc/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
src/**/*.tsx
📄 CodeRabbit inference engine (GEMINI.md)
Prefer Server Components for initial data fetching; use Client Components ('use client') only for interactivity
Files:
src/components/metric/goal-editor.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-chart.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
src/**/*/*.tsx
📄 CodeRabbit inference engine (GEMINI.md)
Client Components must use
import { api } from '@/trpc/react'for standard HTTP/Hooks wrapper
Files:
src/components/metric/goal-editor.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-chart.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (GEMINI.md)
Place colocated components in
_components/folders next to their parent component
Files:
src/components/metric/goal-editor.tsx
**/*.tsx
📄 CodeRabbit inference engine (GEMINI.md)
Use Tailwind CSS 4 for styling with shadcn/ui and Radix UI primitive components
Files:
src/components/metric/goal-editor.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-chart.tsxsrc/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-chart.tsxsrc/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-chart.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
🧠 Learnings (13)
📚 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/components/metric/goal-editor.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-chart.tsxsrc/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/components/metric/goal-editor.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-chart.tsxsrc/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/components/metric/goal-editor.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-chart.tsxsrc/server/api/services/transformation/chart-generator.tssrc/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-chart.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
📚 Learning: 2025-12-20T22:12:00.566Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-20T22:12:00.566Z
Learning: Applies to src/app/teams/[teamId]/**/*.tsx : Avoid modifying `enrichNodesWithRoleData` flow without understanding the complete canvas serialization logic for saving/loading React Flow nodes to the database
Applied to files:
src/app/dashboard/[teamId]/_components/dashboard-metric-chart.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-card.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-chart.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
📚 Learning: 2025-12-20T22:12:00.566Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-20T22:12:00.566Z
Learning: Applies to src/app/teams/[teamId]/**/*.tsx : React Flow nodes must store minimal data (e.g., just `roleId`); fetch full Role data from TanStack Query cache in the Node component to keep canvas and sidebars in sync
Applied to files:
src/app/dashboard/[teamId]/_components/dashboard-metric-chart.tsxsrc/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-chart.tsx
📚 Learning: 2025-12-20T22:12:00.566Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-20T22:12:00.566Z
Learning: Applies to prisma/schema.prisma : When modifying the Prisma schema, especially for Metric Transformers, ensure AI-generated code generation and execution logic is updated accordingly
Applied to files:
src/server/api/services/transformation/chart-generator.ts
📚 Learning: 2025-12-20T22:12:00.566Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-20T22:12:00.566Z
Learning: The Metrics Pipeline processes data through 3 stages: DataIngestionTransformer (Raw API → MetricDataPoint), ChartTransformer (MetricDataPoint → Chart Config), and Visualization (Chart Config → Recharts UI)
Applied to files:
src/server/api/services/transformation/chart-generator.tssrc/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
📚 Learning: 2025-12-20T22:12:00.566Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-20T22:12:00.566Z
Learning: Applies to src/lib/metrics/**/*.ts : Metric transformation logic should be organized in `src/lib/metrics/` to support the 3-stage pipeline: Ingestion → Aggregation → Visualization
Applied to files:
src/server/api/services/transformation/chart-generator.tssrc/app/dashboard/[teamId]/_components/dashboard-metric-card.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]/**/*.{ts,tsx} : Use auto-save system with debounce (2s delay) that serializes nodes/edges to tRPC mutation, with beforeunload sendBeacon fallback for unsaved changes
Applied to files:
src/app/dashboard/[teamId]/_components/dashboard-metric-card.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 (4)
src/components/metric/goal-editor.tsx (1)
src/trpc/react.tsx (1)
api(28-28)
src/app/dashboard/[teamId]/_components/dashboard-metric-chart.tsx (1)
src/components/ui/card.tsx (1)
CardContent(94-94)
src/server/api/services/transformation/chart-generator.ts (5)
src/server/db.ts (1)
db(26-26)src/server/api/services/transformation/ai-code-generator.ts (1)
generateChartTransformerCode(436-504)src/server/api/services/transformation/index.ts (2)
generateChartTransformerCode(13-13)testChartTransformer(23-23)src/server/api/services/transformation/executor.ts (1)
testChartTransformer(234-244)src/server/api/utils/cache-strategy.ts (1)
invalidateCacheByTags(165-179)
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsx (1)
scripts/analyze-metrics-pipeline.mjs (1)
metric(615-627)
⏰ 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 (13)
src/components/metric/goal-editor.tsx (1)
328-338: LGTM!The calculating overlay implementation is well-structured with proper absolute positioning, z-index layering, and backdrop blur for visual feedback during async operations.
src/app/dashboard/[teamId]/_components/dashboard-metric-chart.tsx (2)
59-74: LGTM!The
getLoadingMessagehelper centralizes phase-to-message mapping, improving maintainability and consistency.
690-721: LGTM!The loading state rendering logic correctly handles all combinations:
- Processing with data: overlay with blur preserving chart visibility
- Processing without data: centered loading indicator
- Not processing without data: placeholder message
The
relativepositioning on CardContent properly anchors the absolute overlay.src/server/api/services/transformation/chart-generator.ts (3)
297-300: LGTM!Setting
refreshStatusto"ai-regenerating"at the start of the operation provides proper phase tracking for the UI polling mechanism.
389-395: LGTM!The error handling correctly ensures
refreshStatusis reset tonullbefore rethrowing, preventing orphaned status values that could leave the UI in a perpetual loading state.
377-381: LGTM!Cache invalidation is correctly positioned after successful database updates, ensuring the cache is only invalidated when the operation completes successfully.
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsx (3)
170-234: LGTM!The consolidated
handleRefreshwithforceRebuildflag cleanly unifies refresh and regeneration flows. The separate state flags (isProcessingvsisRegeneratingPipeline) provide accurate UI feedback, and thefinallyblock ensures proper cleanup.
197-200: Good change:refetch()overinvalidate().Using
refetch()ensures the loading state persists until fresh data arrives, providing better feedback thaninvalidate()which would show stale data immediately. This aligns with the PR's cache handling improvements.
225-233: LGTM!The dependency array correctly includes all values referenced within the callback closure.
src/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx (4)
187-227: LGTM!The Force Rebuild toggle implementation is well-designed:
- Clear tooltip explaining the feature's purpose
- Proper disabled state during processing
- Label association with
htmlForfor accessibility
240-295: LGTM!The consolidated chart controls section groups related settings logically (chart type, cadence, AI prompt, apply button) within a visually distinct bordered container, improving the drawer's usability.
346-358: Good optimization.Using
transition-[width]instead oftransition-allis more performant, animating only the intended property and avoiding potential unintended transitions on other properties.
435-449: LGTM!The simplified footer with a single Delete button reduces clutter while maintaining clear destructive action styling.
- Add check-in button for manual metrics in drawer - Enable page transitions for member navigation - Add getMemberStats prefetch - Resolve conflicts with PR #213 by combining features
Summary
Improves the dashboard metric drawer UI with better organization and adds proper loading feedback throughout the chart and goal management flows.
Key Changes
invalidate()torefetch()to ensure loading states persist until new data arrivesai-regenerating→updating-chart) for Apply Changes buttonSummary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.