-
Notifications
You must be signed in to change notification settings - Fork 1
feat: unified KPI card component with goal progress tracking #362
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces a new shared Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Pull Request ReviewOverviewThis PR successfully consolidates two separate metric card implementations ( ✅ Strengths
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/app/teams/_components/edit-team-metric-card.tsx (1)
4-4: Consider using barrel export for consistency.The KpiCard component is exported via the barrel export at
src/components/metric/index.ts. Using the barrel export would be more consistent with the module's public API.♻️ Optional refactor to use barrel export
-import { KpiCard } from "@/components/metric/kpi-card"; +import { KpiCard } from "@/components/metric";src/app/dashboard/[teamId]/_components/dashboard-sidebar.tsx (1)
13-13: Consider using barrel export for consistency.Same as edit-team-metric-card.tsx, using the barrel export would be more consistent.
♻️ Optional refactor to use barrel export
-import { KpiCard } from "@/components/metric/kpi-card"; +import { KpiCard } from "@/components/metric";src/components/metric/kpi-card.tsx (1)
197-246: Enhance accessibility of progress bars.The progress bars display goal and time progress visually but lack semantic ARIA attributes for screen readers. Adding
role="progressbar",aria-valuenow,aria-valuemin,aria-valuemax, and descriptivearia-labelattributes would improve accessibility.♻️ Proposed accessibility enhancements
{/* Goal Progress Row */} <div className="flex items-center gap-1.5 text-[10px]"> - <div className="bg-muted h-1 w-14 overflow-hidden rounded-full"> + <div + className="bg-muted h-1 w-14 overflow-hidden rounded-full" + role="progressbar" + aria-label="Goal progress" + aria-valuenow={Math.round(goalProgress.progressPercent)} + aria-valuemin={0} + aria-valuemax={100} + > <div className={cn( "h-full transition-all", goalProgress.progressPercent >= 100 ? "bg-green-500" : goalProgress.progressPercent >= 70 ? "bg-primary" : "bg-amber-500", )} style={{ width: `${Math.min(goalProgress.progressPercent, 100)}%`, }} /> </div> {/* Time Progress Row */} <div className="flex items-center gap-1.5 text-[10px]"> - <div className="bg-muted h-1 w-14 overflow-hidden rounded-full"> + <div + className="bg-muted h-1 w-14 overflow-hidden rounded-full" + role="progressbar" + aria-label="Time elapsed" + aria-valuenow={Math.round(timeElapsedPercent)} + aria-valuemin={0} + aria-valuemax={100} + > <div className="h-full bg-blue-500 transition-all" style={{ width: `${Math.min(timeElapsedPercent, 100)}%`, }} /> </div>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/dashboard/[teamId]/_components/dashboard-sidebar.tsxsrc/app/teams/_components/edit-team-metric-card.tsxsrc/components/metric/index.tssrc/components/metric/kpi-card.tsx
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (GEMINI.md)
Use TypeScript 5.9 with strict type checking for all frontend and backend code
Files:
src/components/metric/index.tssrc/components/metric/kpi-card.tsxsrc/app/teams/_components/edit-team-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-sidebar.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use @trivago/prettier-plugin-sort-imports with inline type imports for import organization
Files:
src/components/metric/index.tssrc/components/metric/kpi-card.tsxsrc/app/teams/_components/edit-team-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-sidebar.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/kpi-card.tsxsrc/app/teams/_components/edit-team-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-sidebar.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/kpi-card.tsxsrc/app/teams/_components/edit-team-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-sidebar.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (GEMINI.md)
Place colocated components in
_components/folders next to their parent componentUse shadcn/ui components from src/components/ui/; add new components via CLI: npx shadcn@latest add [component-name]
Files:
src/components/metric/kpi-card.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/kpi-card.tsxsrc/app/teams/_components/edit-team-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-sidebar.tsx
src/app/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
src/app/**/*.tsx: Use the dual tRPC API pattern: direct calls in Server Components (api.team.getById) for 10x faster performance, and React hooks in Client Components (api.team.getById.useQuery)
Use getUserDisplayName(userId, members) utility (client-side sync) from @/lib/helpers/get-user-name for displaying user names in components
Files:
src/app/teams/_components/edit-team-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-sidebar.tsx
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/components/dashboard-metric-card.tsx,src/app/dashboard/[teamId]/_components/public-dashboard-metric-card.tsx : Dashboard metric cards are duplicated with public variant. Consolidate into single component with `readOnly` mode prop instead of maintaining separate components.
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T21:27:04.176Z
Learning: Applies to src/app/(metric|dashboard)/**/*.tsx : Use three-stage metrics transformation: API → DataPoints (DataIngestionTransformer), DataPoints → ChartConfig (ChartTransformer), ChartConfig → UI (DashboardMetricChart)
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/lib/metrics/**/*.ts : Goal calculation utility is 271 lines and should be refactored to approximately 50 lines. Simplify logic and extract helper functions.
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T21:27:04.176Z
Learning: Applies to src/app/metric/_components/**/*.tsx : Use shared MetricDialogBase component from base/ for all metric dialog implementations
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/app/metric/_components/**/*.tsx : Metric dialog components have nearly identical wrapper patterns (5 files). Consider implementing a factory pattern or generic wrapper to reduce duplication across provider dialogs.
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/app/teams/[teamId]/_components/role-node.tsx,src/app/teams/[teamId]/_components/public-role-node.tsx : These role node components are 75% identical and should be consolidated. Extract shared `RoleNodeTemplate` component with `isEditable` prop to DRY up the code.
📚 Learning: 2025-12-29T21:27:04.176Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T21:27:04.176Z
Learning: Applies to src/app/metric/_components/**/*.tsx : Use shared MetricDialogBase component from base/ for all metric dialog implementations
Applied to files:
src/components/metric/index.tssrc/components/metric/kpi-card.tsxsrc/app/teams/_components/edit-team-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-sidebar.tsx
📚 Learning: 2025-12-29T12:52:42.935Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/components/dashboard-metric-card.tsx,src/app/dashboard/[teamId]/_components/public-dashboard-metric-card.tsx : Dashboard metric cards are duplicated with public variant. Consolidate into single component with `readOnly` mode prop instead of maintaining separate components.
Applied to files:
src/components/metric/index.tssrc/components/metric/kpi-card.tsxsrc/app/teams/_components/edit-team-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-sidebar.tsx
📚 Learning: 2025-12-29T21:27:04.176Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T21:27:04.176Z
Learning: Applies to src/app/metric/_components/**/*.tsx : Create metric dialogs with pattern: [Provider]MetricDialog.tsx (dialog wrapper) + [Provider]MetricContent.tsx (form content) in src/app/metric/_components/[provider]/, then register in index.ts
Applied to files:
src/components/metric/index.tssrc/components/metric/kpi-card.tsxsrc/app/teams/_components/edit-team-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-sidebar.tsx
📚 Learning: 2025-12-29T12:52:42.935Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/lib/metrics/**/*.ts : Goal calculation utility is 271 lines and should be refactored to approximately 50 lines. Simplify logic and extract helper functions.
Applied to files:
src/components/metric/index.ts
📚 Learning: 2025-12-29T12:52:42.935Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/app/metric/_components/**/*.tsx : Metric dialog components should follow the pattern: [Provider]MetricDialog.tsx (dialog wrapper) and [Provider]MetricContent.tsx (form content). Register in src/app/metric/_components/index.ts and inherit from shared MetricDialogBase.
Applied to files:
src/components/metric/index.tssrc/components/metric/kpi-card.tsxsrc/app/teams/_components/edit-team-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-sidebar.tsx
📚 Learning: 2025-12-29T21:27:04.176Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T21:27:04.176Z
Learning: Applies to src/app/(metric|dashboard)/**/*.tsx : Use three-stage metrics transformation: API → DataPoints (DataIngestionTransformer), DataPoints → ChartConfig (ChartTransformer), ChartConfig → UI (DashboardMetricChart)
Applied to files:
src/components/metric/index.tssrc/components/metric/kpi-card.tsxsrc/app/teams/_components/edit-team-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-sidebar.tsx
📚 Learning: 2025-12-29T12:52:42.935Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/app/metric/_components/**/*.tsx : Metric dialog components have nearly identical wrapper patterns (5 files). Consider implementing a factory pattern or generic wrapper to reduce duplication across provider dialogs.
Applied to files:
src/components/metric/index.tssrc/components/metric/kpi-card.tsxsrc/app/teams/_components/edit-team-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-sidebar.tsx
📚 Learning: 2025-12-20T22:12:00.576Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-20T22:12:00.576Z
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/components/metric/index.ts
📚 Learning: 2025-12-29T21:27:04.176Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T21:27:04.176Z
Learning: Applies to src/hooks/**/*.ts : For role-metric cache updates, optimistically update both role cache (role.getByTeamId) and dashboard cache (dashboard.getDashboardCharts) to maintain UI consistency
Applied to files:
src/components/metric/index.tssrc/app/teams/_components/edit-team-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-sidebar.tsx
📚 Learning: 2025-12-29T12:52:42.935Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/app/dashboard/[teamId]/**/*.{ts,tsx} : Dashboard cache updates for role-metric assignments must update both `role.getByTeamId` and `dashboard.getDashboardCharts` caches during mutations. Use onMutate for optimistic updates on both caches, then invalidate both on success.
Applied to files:
src/components/metric/index.tssrc/app/teams/_components/edit-team-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-sidebar.tsx
📚 Learning: 2025-12-29T12:52:42.935Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/app/teams/[teamId]/_components/role-node.tsx,src/app/teams/[teamId]/_components/public-role-node.tsx : These role node components are 75% identical and should be consolidated. Extract shared `RoleNodeTemplate` component with `isEditable` prop to DRY up the code.
Applied to files:
src/app/teams/_components/edit-team-metric-card.tsx
📚 Learning: 2025-12-20T22:12:00.576Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-20T22:12:00.576Z
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/teams/_components/edit-team-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-sidebar.tsx
📚 Learning: 2025-12-29T21:27:04.176Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T21:27:04.176Z
Learning: Applies to src/app/teams/[teamId]/**/*.tsx : Use reusable canvas library patterns from src/lib/canvas/ (create-canvas-store.tsx, use-auto-save.ts, save-status.tsx, edge-action-buttons.tsx, floating-edge-utils.ts)
Applied to files:
src/app/teams/_components/edit-team-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-sidebar.tsx
📚 Learning: 2025-12-29T21:27:04.176Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T21:27:04.176Z
Learning: Applies to src/app/teams/[teamId]/**/*.tsx : Use shared React Flow primitives (BaseNode, BaseHandle, ZoomSlider) from src/components/react-flow/ in all canvas implementations
Applied to files:
src/app/teams/_components/edit-team-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-sidebar.tsx
🧬 Code graph analysis (2)
src/app/teams/_components/edit-team-metric-card.tsx (2)
src/components/metric/index.ts (1)
KpiCard(2-2)src/components/metric/kpi-card.tsx (1)
KpiCard(55-283)
src/app/dashboard/[teamId]/_components/dashboard-sidebar.tsx (2)
src/components/metric/index.ts (1)
KpiCard(2-2)src/components/metric/kpi-card.tsx (1)
KpiCard(55-283)
⏰ 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: claude-review
🔇 Additional comments (8)
src/app/teams/_components/edit-team-metric-card.tsx (1)
11-16: LGTM! Clean delegation pattern.The component successfully delegates to the shared KpiCard while maintaining its public interface. The default props (showSettings=true, enableDragDrop=false) are appropriate for the edit team context.
src/components/metric/index.ts (1)
1-3: LGTM! Clean barrel export pattern.The barrel export provides a centralized public API for metric components, making imports more consistent across the codebase.
src/app/dashboard/[teamId]/_components/dashboard-sidebar.tsx (1)
180-191: LGTM! Successful integration of KpiCard.The prop mapping is correct and maintains backward compatibility:
isOnCanvasis properly computed from thechartNodesOnCanvasSetonToggleChartVisibility(parent prop) correctly maps toonToggleVisibility(KpiCard prop)- All drag-drop handlers are properly wired
- The component functionality is fully preserved
src/components/metric/kpi-card.tsx (5)
20-32: LGTM! Time formatting logic is sound.The helper correctly handles different cadences and uses
Math.maxto prevent negative values. Colocation is appropriate since this is only used within KpiCard.
66-84: LGTM! Component setup is well-structured.The initialization logic is correct:
- Processing state properly disables dragging
- Title fallback pattern (
chartTransform?.title ?? metric.name) aligns with established conventions- Time elapsed percentage calculation is mathematically sound
- Proper null handling with optional chaining and nullish coalescing
86-104: LGTM! Drag-and-drop implementation is robust.The implementation correctly:
- Uses
draggable={canDrag ? true : undefined}to avoid adding the attribute when false- Calls
stopPropagation()to prevent event bubbling- Guards handler invocation with conditional checks
- Applies appropriate styling for all drag states
256-279: LGTM! Role display implementation is correct.The role display properly handles multiple roles by showing the first role with a colored indicator and a "+N" count for additional roles. The non-null assertions on lines 262 and 265 are justified because they're guarded by the
roles.length > 0check. The "No role" fallback provides appropriate feedback.
34-53: LGTM! Well-documented interface with sensible defaults.The interface clearly separates required props from optional canvas-specific props. The JSDoc comments provide good context for each prop's purpose, and the defaults (showSettings=true, enableDragDrop=false) are appropriate for the primary use cases.
Summary
Creates a reusable KpiCard component that displays goal progress, time tracking, cadence, and role assignments in a compact format. This consolidates the previously separate EditTeamMetricCard and SidebarMetricCard implementations into a single unified component.
Key Changes
KpiCardcomponent atsrc/components/metric/kpi-card.tsxEditTeamMetricCardimplementation with KpiCard wrapperSidebarMetricCardin dashboard sidebar with KpiCardchartTransform?.title ?? metric.name)Summary by CodeRabbit
Refactor
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.