From 4e8f76996cd45a537b064e47788f6730bb318648 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Mon, 23 Mar 2026 02:08:10 +0000
Subject: [PATCH] Fix code review issues in TeamScoreBreakdown sortable table

- Move SortIndicator out of render (avoids remount on every render)
- Extract sortPicks helper outside component
- Restore default sort: sport name then pick number (was lost in prior commit)
- Replace unicode arrow chars with Lucide ArrowUp/ArrowDown/ArrowUpDown icons
- Add aria-sort attributes to sortable column headers for accessibility
- Change w-[90px] to min-w-[90px] on Pick # column to avoid clipping
- Flatten nested div inside Points sort button

https://claude.ai/code/session_01XBnm7eKxerR7WjwrqqJPwe
---
 .../standings/TeamScoreBreakdown.tsx          | 62 +++++++++++--------
 1 file changed, 37 insertions(+), 25 deletions(-)

diff --git a/app/components/standings/TeamScoreBreakdown.tsx b/app/components/standings/TeamScoreBreakdown.tsx
index 7833222..d7dd1b5 100644
--- a/app/components/standings/TeamScoreBreakdown.tsx
+++ b/app/components/standings/TeamScoreBreakdown.tsx
@@ -1,5 +1,6 @@
 import { useState } from "react";
 import { Link } from "react-router";
+import { ArrowUp, ArrowDown, ArrowUpDown } from "lucide-react";
 import { Card, CardContent } from "~/components/ui/card";
 import { Table, TableHeader, TableRow, TableHead, TableBody, TableCell } from "~/components/ui/table";
 import { Badge } from "~/components/ui/badge";
@@ -41,6 +42,33 @@ interface TeamScoreBreakdownProps {
 type SortColumn = "pick" | "points";
 type SortDirection = "asc" | "desc";
 
+function SortIndicator({ column, sortColumn, sortDirection }: { column: SortColumn; sortColumn: SortColumn; sortDirection: SortDirection }) {
+  if (sortColumn !== column) return <ArrowUpDown className="ml-1 h-3 w-3 text-muted-foreground/40" />;
+  return sortDirection === "asc"
+    ? <ArrowUp className="ml-1 h-3 w-3" />
+    : <ArrowDown className="ml-1 h-3 w-3" />;
+}
+
+function sortPicks(
+  picks: TeamScoreBreakdownProps["breakdown"]["picks"],
+  sortColumn: SortColumn,
+  sortDirection: SortDirection,
+) {
+  const dir = sortDirection === "asc" ? 1 : -1;
+  return picks.slice().sort((a, b) => {
+    if (sortColumn === "pick") {
+      const sportCmp = a.participant.sport.localeCompare(b.participant.sport);
+      return sportCmp !== 0 ? sportCmp * dir : (a.pickNumber - b.pickNumber) * dir;
+    }
+    // points: sort by actual first, then projected
+    const pointsDiff = a.points - b.points;
+    if (pointsDiff !== 0) return pointsDiff * dir;
+    const aProjected = a.projectedPoints ?? 0;
+    const bProjected = b.projectedPoints ?? 0;
+    return (aProjected - bProjected) * dir;
+  });
+}
+
 /**
  * Display detailed team score breakdown with all drafted participants
  * Phase 4.3: Team breakdown pages
@@ -74,23 +102,7 @@ export function TeamScoreBreakdown({
     }
   }
 
-  const allPicks = breakdown.picks.slice().sort((a, b) => {
-    const dir = sortDirection === "asc" ? 1 : -1;
-    if (sortColumn === "pick") {
-      return (a.pickNumber - b.pickNumber) * dir;
-    }
-    // points: sort by actual first, then projected
-    const pointsDiff = a.points - b.points;
-    if (pointsDiff !== 0) return pointsDiff * dir;
-    const aProjected = a.projectedPoints ?? 0;
-    const bProjected = b.projectedPoints ?? 0;
-    return (aProjected - bProjected) * dir;
-  });
-
-  function SortIndicator({ column }: { column: SortColumn }) {
-    if (sortColumn !== column) return <span className="ml-1 text-muted-foreground/40">↕</span>;
-    return <span className="ml-1">{sortDirection === "asc" ? "↑" : "↓"}</span>;
-  }
+  const allPicks = sortPicks(breakdown.picks, sortColumn, sortDirection);
 
   return (
     <div className="space-y-6">
@@ -130,12 +142,13 @@ export function TeamScoreBreakdown({
           <Table>
             <TableHeader>
               <TableRow>
-                <TableHead className="w-[90px]">
+                <TableHead className="min-w-[90px]">
                   <button
                     onClick={() => handleSort("pick")}
+                    aria-sort={sortColumn === "pick" ? (sortDirection === "asc" ? "ascending" : "descending") : "none"}
                     className="flex items-center hover:text-foreground cursor-pointer"
                   >
-                    Pick #<SortIndicator column="pick" />
+                    Pick #<SortIndicator column="pick" sortColumn={sortColumn} sortDirection={sortDirection} />
                   </button>
                 </TableHead>
                 <TableHead>Sport</TableHead>
@@ -144,14 +157,13 @@ export function TeamScoreBreakdown({
                 <TableHead className="text-right">
                   <button
                     onClick={() => handleSort("points")}
-                    className="flex items-center ml-auto hover:text-foreground cursor-pointer"
+                    aria-sort={sortColumn === "points" ? (sortDirection === "asc" ? "ascending" : "descending") : "none"}
+                    className="flex flex-col items-end ml-auto hover:text-foreground cursor-pointer"
                   >
-                    <div className="text-right">
-                      <div className="flex items-center">
-                        Points<SortIndicator column="points" />
-                      </div>
-                      <div className="text-xs font-normal text-muted-foreground">actual / projected</div>
+                    <div className="flex items-center">
+                      Points<SortIndicator column="points" sortColumn={sortColumn} sortDirection={sortDirection} />
                     </div>
+                    <div className="text-xs font-normal text-muted-foreground">actual / projected</div>
                   </button>
                 </TableHead>
               </TableRow>