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
This commit is contained in:
Claude 2026-03-23 02:08:10 +00:00
parent 906fb6865a
commit 4e8f76996c
No known key found for this signature in database

View file

@ -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>