Skip to content

Conversation

@drifter089
Copy link
Owner

@drifter089 drifter089 commented Jan 9, 2026

Summary

Redesigns the member page UI with improved charts, navigation, and usability.

Key Changes

  • Fix pie chart legend overflow with flex-wrap and scrolling for many roles
  • Add team links as clickable badges on member cards linking to /teams/[teamId]
  • Replace radar chart with vertical bar chart for goals (color-coded by progress vs expected)
  • Add members sidebar toggle for quick navigation between members (closed by default)
  • Prefetch memberStats for sidebar data

Summary by CodeRabbit

  • New Features

    • Member cards now display associated teams as clickable badges linking to team pages
    • Added collapsible sidebar panel showing member list with statistics and toggle control
  • UI Improvements

    • Goals chart redesigned from radar to bar chart visualization with dynamic progress-based coloring
    • Chart legends now wrap responsively with truncated labels and scrolling for improved overflow handling

✏️ Tip: You can customize this high-level summary in your review settings.

@vercel
Copy link

vercel bot commented Jan 9, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
org-os Ready Ready Preview, Comment Jan 9, 2026 0:04am

@coderabbitai
Copy link

coderabbitai bot commented Jan 9, 2026

Warning

Rate limit exceeded

@drifter089 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 18 minutes and 57 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between d6b0208 and 4b40d51.

📒 Files selected for processing (3)
  • src/app/member/members-sidebar.tsx
  • src/components/charts/goals-radar-chart.tsx
  • src/components/ui/chart.tsx
📝 Walkthrough

Walkthrough

This pull request adds a members sidebar with statistics integration, updates the member card to display unique teams, refactors the goals chart from radar to bar chart with expected progress tracking, and enhances legend responsiveness in chart components.

Changes

Cohort / File(s) Summary
Member UI Components
src/app/member/member-card.tsx, src/app/member/members-sidebar.tsx, src/app/member/members-page-client.tsx, src/app/member/page.tsx
Added new MembersSidebar component with toggleable right-side sheet; integrated member stats data fetching via prefetch and useQuery; updated member card to display unique teams extracted from roles using useMemo; conditionally render teams section with team badges and links.
Goals Chart Refactoring
src/components/charts/goals-radar-chart.tsx
Migrated visualization from RadarChart to BarChart; introduced getBarColor helper function for dynamic progress-based coloring; extended ChartDataPoint with expectedProgress field; replaced GoalProgressBar component; updated layout, axes, and tooltip configuration for bar chart.
Chart Legend Enhancement
src/components/ui/chart.tsx
Enhanced legend responsiveness with wrapping layout, horizontal and vertical gaps, max-height constraint, and vertical scrolling; added truncation and fixed width to legend item labels.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 A sidebar blooms with members and stats so grand,
Radar charts transform to bars across the land,
Teams dance on cards with links in hand,
Legends wrap and scroll as planned!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes all three main UI improvements: bar chart replacement, team links, and sidebar addition.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/components/charts/goals-radar-chart.tsx (1)

308-308: Clamp expectedProgress to prevent visual issues.

While progress is properly clamped to the 0-100 range (line 307), expectedProgress is not validated. If the API returns values outside this range, it could cause:

  • Incorrect bar color categorization in getBarColor
  • The expected progress marker in the tooltip (line 250) positioned off-chart
  • Misleading progress comparisons in the tooltip (line 227)
🛡️ Proposed fix
           progress: Math.max(0, Math.min(100, gp.progressPercent ?? 0)),
-          expectedProgress: gp.expectedProgressPercent,
+          expectedProgress: Math.max(0, Math.min(100, gp.expectedProgressPercent)),
🧹 Nitpick comments (2)
src/components/ui/chart.tsx (1)

309-311: Consider slightly wider max-width for legend labels.

The max-w-[80px] truncation might be too aggressive for typical role or team names (e.g., "Engineering Manager" becomes "Engineeri..."). Consider max-w-[100px] or max-w-[120px] to improve readability while still preventing overflow.

♻️ Suggested adjustment
-              <span className="max-w-[80px] truncate text-xs">
+              <span className="max-w-[120px] truncate text-xs">
                {itemConfig?.label}
              </span>
src/app/member/members-sidebar.tsx (1)

65-88: Consider extracting panel width to a constant.

The toggle button position (right-[26.5rem]) is tightly coupled to the panel width (w-[26rem] on line 94). If the panel width changes, this value must be manually synchronized.

♻️ Suggested refactor
+const SIDEBAR_WIDTH = "26rem";
+const SIDEBAR_WIDTH_CLASS = "w-[26rem]";
+const BUTTON_OFFSET_OPEN = "right-[26.5rem]";
+
 export function MembersSidebar({ members, memberStats }: MembersSidebarProps) {
   const [isOpen, setIsOpen] = useState(false);

   return (
     <>
       <button
         onClick={() => setIsOpen(!isOpen)}
         className={cn(
           "fixed top-1/2 z-50 -translate-y-1/2 transition-all duration-300 ease-in-out",
           "flex items-center gap-1 md:gap-1.5",
           "h-8 px-2 md:h-9 md:px-2.5",
           "bg-background rounded-md border",
           "shadow-md hover:shadow-lg",
           "text-xs font-medium md:text-sm",
           isOpen
-            ? "border-primary bg-accent right-[26.5rem]"
+            ? `border-primary bg-accent ${BUTTON_OFFSET_OPEN}`
             : "hover:bg-accent/50 right-4",
         )}
         aria-label={isOpen ? "Close Members sidebar" : "Open Members sidebar"}
       >
         {/* ... */}
       </button>

       <Sheet open={isOpen} onOpenChange={setIsOpen} modal={false}>
         <NonModalSheetContent
           side="right"
-          className="w-[26rem] overflow-hidden p-0 sm:max-w-none"
+          className={`${SIDEBAR_WIDTH_CLASS} overflow-hidden p-0 sm:max-w-none`}
           hideCloseButton
         >
           <MembersPanel members={members} memberStats={memberStats} />
         </NonModalSheetContent>
       </Sheet>
     </>
   );
 }
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e7e608a and d6b0208.

📒 Files selected for processing (6)
  • src/app/member/member-card.tsx
  • src/app/member/members-page-client.tsx
  • src/app/member/members-sidebar.tsx
  • src/app/member/page.tsx
  • src/components/charts/goals-radar-chart.tsx
  • src/components/ui/chart.tsx
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (GEMINI.md)

Use TypeScript 5.9 with strict type checking for all frontend and backend code

Files:

  • src/app/member/page.tsx
  • src/app/member/members-sidebar.tsx
  • src/app/member/members-page-client.tsx
  • src/components/ui/chart.tsx
  • src/app/member/member-card.tsx
  • src/components/charts/goals-radar-chart.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/app/member/page.tsx
  • src/app/member/members-sidebar.tsx
  • src/app/member/members-page-client.tsx
  • src/components/ui/chart.tsx
  • src/app/member/member-card.tsx
  • src/components/charts/goals-radar-chart.tsx
src/**/*/*.tsx

📄 CodeRabbit inference engine (GEMINI.md)

Client Components must use import { api } from '@/trpc/react' for standard HTTP/Hooks wrapper

Files:

  • src/app/member/page.tsx
  • src/app/member/members-sidebar.tsx
  • src/app/member/members-page-client.tsx
  • src/components/ui/chart.tsx
  • src/app/member/member-card.tsx
  • src/components/charts/goals-radar-chart.tsx
**/*.tsx

📄 CodeRabbit inference engine (GEMINI.md)

Use Tailwind CSS 4 for styling with shadcn/ui and Radix UI primitive components

Files:

  • src/app/member/page.tsx
  • src/app/member/members-sidebar.tsx
  • src/app/member/members-page-client.tsx
  • src/components/ui/chart.tsx
  • src/app/member/member-card.tsx
  • src/components/charts/goals-radar-chart.tsx
src/app/**/page.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

Use Next.js 15 App Router for page definitions and layout organization

Files:

  • src/app/member/page.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/member/page.tsx
  • src/app/member/members-sidebar.tsx
  • src/app/member/members-page-client.tsx
  • src/app/member/member-card.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/app/member/page.tsx
  • src/app/member/members-sidebar.tsx
  • src/app/member/members-page-client.tsx
  • src/components/ui/chart.tsx
  • src/app/member/member-card.tsx
  • src/components/charts/goals-radar-chart.tsx
src/components/**/*.tsx

📄 CodeRabbit inference engine (GEMINI.md)

Place colocated components in _components/ folders next to their parent component

Use shadcn/ui components from src/components/ui/; add new components via CLI: npx shadcn@latest add [component-name]

Files:

  • src/components/ui/chart.tsx
  • src/components/charts/goals-radar-chart.tsx
🧠 Learnings (23)
📓 Common learnings
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|org)/**/*.tsx : Use shared MembersList component at src/components/member/member-list.tsx for displaying members in canvas sidebar and org page; it includes getMemberDisplayInfo() utility for initials/name logic
📚 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|org)/**/*.tsx : Use shared MembersList component at src/components/member/member-list.tsx for displaying members in canvas sidebar and org page; it includes getMemberDisplayInfo() utility for initials/name logic

Applied to files:

  • src/app/member/page.tsx
  • src/app/member/members-sidebar.tsx
  • src/app/member/members-page-client.tsx
  • src/app/member/member-card.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/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/app/member/page.tsx
  • src/app/member/member-card.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/member/member-list.tsx : The MembersList component is consolidated and reused by both canvas sidebar (canvas-side-panels.tsx) and org page (MembersListClient.tsx). Use this component for all member list displays to maintain consistency.

Applied to files:

  • src/app/member/page.tsx
  • src/app/member/members-sidebar.tsx
  • src/app/member/members-page-client.tsx
  • src/app/member/member-card.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/app/member/page.tsx
  • src/app/member/member-card.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]/_components/**/*.tsx : Implement cache-first node pattern for role nodes: store only roleId in node data, fetch display data from TanStack Query cache using useRoleData hook

Applied to files:

  • src/app/member/page.tsx
  • src/app/member/member-card.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/server/api/**/*.ts : Use enrichRolesWithUserNames() for flat role arrays and enrichChartRolesWithUserNames() for nested chart roles from src/server/api/utils/organization-members.ts

Applied to files:

  • src/app/member/page.tsx
  • src/app/member/member-card.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 : Use shared MetricDialogBase component from base/ for all metric dialog implementations

Applied to files:

  • src/app/member/members-sidebar.tsx
  • src/components/ui/chart.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/**/*.tsx : Use getUserDisplayName(userId, members) utility (client-side sync) from @/lib/helpers/get-user-name for displaying user names in components

Applied to files:

  • src/app/member/members-page-client.tsx
  • src/app/member/member-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/member/members-page-client.tsx
  • src/app/member/member-card.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/**/*.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)

Applied to files:

  • src/app/member/members-page-client.tsx
  • src/app/member/member-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/**/*/*.tsx : Client Components must use `import { api } from '@/trpc/react'` for standard HTTP/Hooks wrapper

Applied to files:

  • src/app/member/members-page-client.tsx
  • src/app/member/member-card.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/ui/chart.tsx
  • src/components/charts/goals-radar-chart.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 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/ui/chart.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/ui/chart.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/ui/chart.tsx
  • src/app/member/member-card.tsx
  • src/components/charts/goals-radar-chart.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/member/member-card.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/member/member-card.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]/**/*.{ts,tsx} : Cache-first node system: Role nodes store only `roleId` in node.data. Display data must be fetched from TanStack Query cache using the `useRoleData()` hook which queries the `role.getByTeamId` procedure.

Applied to files:

  • src/app/member/member-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/**/*.tsx : Prefer Server Components for initial data fetching; use Client Components ('use client') only for interactivity

Applied to files:

  • src/app/member/member-card.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/member/member-card.tsx
  • src/components/charts/goals-radar-chart.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 : 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/member/member-card.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/charts/goals-radar-chart.tsx
🧬 Code graph analysis (3)
src/app/member/page.tsx (1)
src/trpc/react.tsx (1)
  • api (28-28)
src/app/member/members-page-client.tsx (2)
src/trpc/react.tsx (1)
  • api (28-28)
src/app/member/members-sidebar.tsx (1)
  • MembersSidebar (59-102)
src/components/charts/goals-radar-chart.tsx (1)
src/components/ui/chart.tsx (1)
  • ChartTooltip (358-358)
⏰ 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 (10)
src/app/member/member-card.tsx (2)

58-68: LGTM: Efficient team extraction logic.

The useMemo implementation with Map-based deduplication correctly extracts unique teams from member roles. The dependency array is properly configured.


128-142: LGTM: Team navigation implemented correctly.

The team badges provide clear visual affordances with hover states and proper navigation links. The conditional rendering ensures the section only appears when teams exist.

src/components/ui/chart.tsx (1)

280-282: LGTM: Legend overflow fixed with flex-wrap and scrolling.

The responsive layout with flex-wrap and scrollable container effectively handles charts with many legend items, addressing the pie chart legend overflow issue mentioned in the PR.

src/app/member/members-sidebar.tsx (1)

16-52: LGTM: Custom non-modal sheet implementation.

The NonModalSheetContent wrapper correctly implements non-modal behavior with proper accessibility (sr-only title, close button labels) and animation classes.

src/app/member/members-page-client.tsx (2)

40-40: LGTM: Member stats query integrated correctly.

The getMemberStats query follows the client component tRPC pattern and correctly omits loading/error states since memberStats is optional for the MembersSidebar component.

As per coding guidelines: Client Components use import { api } from '@/trpc/react' for standard HTTP/Hooks wrapper.


46-48: LGTM: Sidebar placement and conditional rendering.

Correctly places the sidebar outside the main container for fixed positioning and only renders when members exist.

src/app/member/page.tsx (1)

9-9: LGTM: Member stats prefetch added correctly.

The getMemberStats prefetch follows the dual tRPC API pattern for Server Components and enables the sidebar to display statistics without additional client-side loading.

As per coding guidelines: Use direct calls in Server Components (api.organization.getMemberStats) for 10x faster performance.

src/components/charts/goals-radar-chart.tsx (3)

119-130: LGTM! Color logic is consistent across the component.

The helper function provides a clear and reasonable color coding scheme based on progress thresholds. The HSL values correctly match the Tailwind color classes used in the tooltip (lines 237-243), ensuring visual consistency across bars, tooltip, and legend dots.


383-429: Chart configuration looks appropriate for the bar chart visualization.

The XAxis configuration with angled labels (angle={-45}, textAnchor="end", height={60}) properly accommodates goal names, and the YAxis with percentage formatting is clear. The animation settings provide smooth visual feedback.

Note: interval={0} on the XAxis displays all labels, which could cause overlap if many goals are present. This appears intentional for ensuring all goals remain visible, though you may want to monitor usability with larger datasets.


431-487: LGTM! Legend implementation aligns with PR objectives.

The legend correctly uses flex-wrap to handle overflow (addressing the PR objective), and the dynamic color coding via getBarColor (lines 442-445) ensures visual consistency with the bars and tooltip. The interactive hover states and tooltips enhance usability.

@claude
Copy link
Contributor

claude bot commented Jan 9, 2026

PR Review: Member Page Redesign

Great work on improving the member page UX! The changes enhance navigation and data visualization. Here's my detailed review:


Strengths

1. Good UX Improvements

  • Team links as clickable badges provide excellent navigation
  • Sidebar toggle with consistent styling matches canvas side panels
  • Bar chart is more intuitive than radar chart for goal comparison
  • Color-coded bars based on progress vs expected provide instant visual feedback

2. Performance Optimization

  • Proper use of useMemo for team extraction prevents unnecessary recalculations
  • Prefetching getMemberStats improves perceived performance

3. Accessibility Enhancements

  • Good use of aria-label on the sidebar toggle button
  • Screen reader text for titles and close buttons
  • Semantic HTML with proper button elements

⚠️ Issues & Concerns

1. Potential Null Reference Error (Critical)

Location: src/app/member/member-card.tsx:63-64

While you check role.team exists, other parts of the codebase access role.team.id directly without null checks. This suggests roles should always have a team, but the defensive check here indicates uncertainty.

Recommendation:

  • Either roles always have teams (remove the check), or ensure all role.team accesses are null-safe throughout the codebase

2. Chart Legend Overflow Fix May Not Work on Mobile

Location: src/components/ui/chart.tsx:280-281

Fixed max-h-100px may be too restrictive on mobile devices. overflow-y-auto with flex-wrap can create awkward scrolling behavior. Truncation at max-w-80px might cut important text on small screens.

Recommendation: Consider responsive max-height values


3. Hardcoded Magic Number in Sidebar Toggle

Location: src/app/member/members-sidebar.tsx:75

The right-26.5rem value is hardcoded and must match the sidebar width. If the sidebar width changes, this breaks.

Recommendation: Define as constants or use CSS variables


4. Accessibility: Missing ARIA Description

Location: src/app/member/members-sidebar.tsx:39-41

While you have a title, the sidebar content lacks a description for screen readers. Consider adding SheetPrimitive.Description.


5. Removed Code Without Clear Justification

Location: src/components/charts/goals-radar-chart.tsx

The GoalProgressBar component and logic for displaying less than 3 goals differently was removed. The PR description doesn't explain why. Now these cases go straight to the bar chart, which may not look good with only 1-2 bars.

Recommendation: Test the UI with 1-2 goals to ensure it looks acceptable


6. Color Values Should Use CSS Variables

Location: src/components/charts/goals-radar-chart.tsx:119-130

Hardcoded HSL color values bypass the theme system and won't adapt to dark mode or custom themes.

Recommendation: Use Tailwind CSS variables or chart colors instead


7. Non-Modal Sheet May Cause Layout Issues

Location: src/app/member/members-sidebar.tsx:91

Setting modal=false means the sheet doesn't create an overlay or trap focus. This can lead to users accidentally clicking content behind the sheet and keyboard navigation confusion.

Recommendation: Document this decision if intentional, and test keyboard navigation thoroughly


8. Missing Error Boundaries

The new sidebar and chart components don't have error boundaries. If MembersPanel or the bar chart crashes, the entire page may break.


🧪 Testing Recommendations

Critical Tests Needed:

  1. Null team data: What happens if a role has team null?
  2. Single goal: Does the bar chart look good with only 1 bar?
  3. Many goals (10+): Does the X-axis become unreadable?
  4. Long team names: Do truncated team badges still make sense?
  5. Mobile layout: Does the sidebar overlap content on small screens?
  6. Keyboard navigation: Can users navigate the sidebar without a mouse?

📊 Performance Notes

Good: useMemo prevents re-computation, prefetching reduces loading states

Potential Issue: Bar chart renders Cell components in a loop. With many goals (15+), this could impact rendering performance. Consider virtualization or memoization.


🔒 Security Review

✅ No security issues detected:

  • No user input rendered without sanitization
  • Links use Next.js Link component
  • No direct DOM manipulation

🎯 Summary

Overall: This is a solid UX improvement, but needs attention to edge cases and potential runtime errors.

Priority Fixes:

  1. 🔴 Critical: Verify role.team null safety across codebase
  2. 🟡 Important: Test with 1-2 goals and adjust UI if needed
  3. 🟡 Important: Use CSS variables for colors (theme support)
  4. 🟢 Nice to have: Extract magic numbers and improve responsive design

Let me know if you'd like help addressing any of these issues! 🚀

@claude
Copy link
Contributor

claude bot commented Jan 9, 2026

PR Review: Member Page Redesign

Overview

This PR makes solid improvements to the member page UI by replacing the radar chart with a bar chart, adding team navigation, fixing legend overflow, and introducing a members sidebar. The implementation is generally well-structured and follows the project's patterns.


✅ Code Quality & Best Practices

Strong Points:

  • Proper use of useMemo for team extraction (member-card.tsx:59-68) - prevents unnecessary recalculations
  • Consistent with project patterns - uses tRPC query caching correctly
  • Good component reusability - leverages MembersPanel from shared components
  • Accessibility considerations - proper aria-label and sr-only elements in sidebar
  • Type safety - proper TypeScript usage with RouterOutputs types

Areas for Improvement:

  1. Duplicate display logic in member-card.tsx (lines 27-40)

    • Functions getDisplayName() and getInitials() duplicate logic from src/components/member/member-list.tsx:16-28
    • Per CLAUDE.md consolidation guidelines, use the shared getMemberDisplayInfo() utility instead:
    import { getMemberDisplayInfo } from "@/components/member/member-list";
    
    // Then in component:
    const { displayName, initials } = getMemberDisplayInfo(member);
  2. Non-modal sheet pattern (members-sidebar.tsx:19-58)

    • Custom NonModalSheetContent component reimplements sheet primitives
    • Consider extracting to a shared component if this pattern is reused elsewhere
    • The modal={false} prop (line 94) is good for UX but needs documentation
  3. Magic constants (members-sidebar.tsx:16-17)

    • SIDEBAR_WIDTH and SIDEBAR_WIDTH_OFFSET are hardcoded
    • The 0.5rem offset feels fragile - consider calculating dynamically or document why needed

🐛 Potential Bugs

  1. Race condition risk (members-sidebar.tsx:72)

    • Toggle button uses inline style with SIDEBAR_WIDTH_OFFSET
    • If sidebar animation is interrupted, button position might desync from sidebar
    • Consider using CSS classes instead of inline styles for state-dependent positioning
  2. Missing null check (member-card.tsx:63)

    • role.team is accessed without checking if it exists
    • While TypeScript types may enforce this, add a defensive check:
    if (role.team?.id && !teamMap.has(role.team.id)) {
  3. Potential layout shift (members-page-client.tsx:46)

    • Sidebar is conditionally rendered based on members && members.length > 0
    • This can cause cumulative layout shift (CLS) on initial load
    • Consider always rendering container with conditional content

🚀 Performance Considerations

Good:

  • Proper prefetching in page.tsx (line 9) ✅
  • Uses TanStack Query caching from parent components ✅
  • useMemo for expensive team extraction ✅

Concerns:

  1. Chart legend scrolling (chart.tsx:280)

    • Adding overflow-y-auto with fixed max-h-[80px] creates scrollable legends
    • For members with many roles (10+), this could hide information
    • Consider pagination or "show more" toggle instead of scroll
  2. Bar chart re-renders (goals-radar-chart.tsx:414-428)

    • Each chart cell gets color calculated on every render
    • Consider memoizing getBarColor results:
    const barColors = useMemo(() => 
      chartData.map(entry => getBarColor(entry.progress, entry.expectedProgress)),
      [chartData]
    );
  3. Missing loading states (members-sidebar.tsx)

    • Sidebar shows member stats but doesn't handle loading state
    • If memberStats is undefined, user sees incomplete UI

🔒 Security Concerns

No significant security issues identified. Good practices:

  • ✅ Uses Next.js Link for client-side navigation (no direct href manipulation)
  • ✅ Team IDs from trusted tRPC queries
  • ✅ No user-supplied data in dangerouslySetInnerHTML

🧪 Test Coverage

Missing:

  • No test coverage for the new sidebar functionality
  • No tests for team badge navigation behavior
  • No tests for bar chart rendering with different data states

Recommendations:
Add Playwright tests for:

  1. Sidebar toggle interaction
  2. Team badge click navigation to /teams/[teamId]
  3. Chart legend overflow behavior with many items
  4. Bar chart color coding at different progress levels

Example test structure:

test('member sidebar toggles correctly', async ({ authenticatedPage }) => {
  await authenticatedPage.goto('/member');
  
  const toggleButton = authenticatedPage.getByLabel('Open Members sidebar');
  await toggleButton.click();
  
  await expect(authenticatedPage.getByText('Members')).toBeVisible();
  // Verify sidebar panel is visible
});

📊 Chart Migration (Radar → Bar)

Excellent refactor:

  • ✅ Removed 84 lines of unused radar chart code
  • ✅ Bar chart more readable for goal progress comparison
  • ✅ Color coding by status is intuitive
  • ✅ Maintains all tooltip functionality

One concern:

  • Line 386: Fixed height of 220px might cause issues with many goals
  • Consider dynamic height based on chartData.length:
    className={cn("mx-auto w-full", chartData.length > 5 ? "h-[300px]" : "h-[220px]")}

📝 Additional Notes

  1. Status color mapping (goals-radar-chart.tsx:119-124)

    • Duplicates status logic that exists in STATUS_CONFIG
    • Consider deriving colors from config to maintain single source of truth
  2. Import organization per project guidelines ✅

    • Using @trivago/prettier-plugin-sort-imports correctly
    • Inline type imports properly formatted
  3. Component file naming

    • Follows kebab-case convention
    • Clear, descriptive names

🎯 Summary

Approval Recommendation: ✅ Approve with minor suggestions

This is a solid PR that improves UX without introducing breaking changes. The main issues are:

  1. Code duplication (minor - easy fix)
  2. Missing test coverage (should add before merge)
  3. Minor performance optimizations (can address in follow-up)

Priority fixes before merge:

  1. Replace duplicate display name logic with shared utility
  2. Add null check for role.team access
  3. Add basic Playwright test for sidebar functionality

Nice-to-haves for follow-up:

  • Extract non-modal sheet pattern to shared component
  • Memoize bar color calculations
  • Add loading states for member stats in sidebar

Great work on the redesign! The bar chart is much clearer than the radar chart for this use case. 🎉

@drifter089 drifter089 merged commit 55df760 into main Jan 9, 2026
5 checks passed
drifter089 added a commit that referenced this pull request Jan 9, 2026
)

* feat: redesign member page with bar chart, team links, and sidebar

* fix: address code review feedback for member page redesign
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant