From 45c7950805974ae5068ed1cbbc589f25d138a088 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Fri, 19 Jun 2026 05:37:48 +0000
Subject: [PATCH] Address code review on mobile team breakdown

- Use the shared Radix Select primitive for the mobile sort control
  instead of a one-off native <select>, matching the design system.
- Fix the sort-direction toggle aria-label to describe the action it
  performs rather than the current state.
- Drop a redundant flex wrapper around PointsValue in the desktop cell.
- Add jsdom stubs (scrollIntoView, pointer-capture) to the shared test
  setup so Radix Select can be exercised in unit tests, and update the
  mobile sort tests to drive the Radix dropdown.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EGQoRPjRycNSjwLrUwuKhw
---
 .../standings/TeamScoreBreakdown.tsx          | 40 +++++++++++--------
 .../__tests__/TeamScoreBreakdown.test.tsx     | 12 ++++--
 app/test/setup.ts                             |  8 ++++
 3 files changed, 41 insertions(+), 19 deletions(-)

diff --git a/app/components/standings/TeamScoreBreakdown.tsx b/app/components/standings/TeamScoreBreakdown.tsx
index e2a9423..f4afdb9 100644
--- a/app/components/standings/TeamScoreBreakdown.tsx
+++ b/app/components/standings/TeamScoreBreakdown.tsx
@@ -3,6 +3,13 @@ import { Link } from "react-router";
 import { ArrowUp, ArrowDown, ArrowUpDown } from "lucide-react";
 import { Button } from "~/components/ui/button";
 import { Card, CardContent } from "~/components/ui/card";
+import {
+  Select,
+  SelectContent,
+  SelectItem,
+  SelectTrigger,
+  SelectValue,
+} from "~/components/ui/select";
 import { Table, TableHeader, TableRow, TableHead, TableBody, TableCell } from "~/components/ui/table";
 import { Badge } from "~/components/ui/badge";
 
@@ -271,9 +278,7 @@ export function TeamScoreBreakdown({
                     <PositionCell pick={pick} />
                   </TableCell>
                   <TableCell className="text-right">
-                    <div className="flex flex-col items-end">
-                      <PointsValue pick={pick} />
-                    </div>
+                    <PointsValue pick={pick} />
                   </TableCell>
                 </TableRow>
               ))}
@@ -286,26 +291,29 @@ export function TeamScoreBreakdown({
       <div className="md:hidden space-y-3" data-testid="picks-mobile">
         {/* Sort control */}
         <div className="flex items-center gap-2">
-          <label htmlFor="picks-sort" className="text-sm text-muted-foreground shrink-0">
+          <span id="picks-sort-label" className="text-sm text-muted-foreground shrink-0">
             Sort by
-          </label>
-          <select
-            id="picks-sort"
+          </span>
+          <Select
             value={sortColumn}
-            onChange={(e) => handleSortColumn(e.target.value as SortColumn)}
-            className="flex-1 rounded-md border border-input bg-transparent px-3 py-2 text-sm shadow-xs outline-none focus-visible:ring-[3px] focus-visible:ring-ring/50"
+            onValueChange={(v) => handleSortColumn(v as SortColumn)}
           >
-            {SORT_OPTIONS.map((opt) => (
-              <option key={opt.value} value={opt.value}>
-                {opt.label}
-              </option>
-            ))}
-          </select>
+            <SelectTrigger className="flex-1" aria-labelledby="picks-sort-label">
+              <SelectValue />
+            </SelectTrigger>
+            <SelectContent>
+              {SORT_OPTIONS.map((opt) => (
+                <SelectItem key={opt.value} value={opt.value}>
+                  {opt.label}
+                </SelectItem>
+              ))}
+            </SelectContent>
+          </Select>
           <Button
             variant="outline"
             size="icon"
             onClick={() => setSortDirection(sortDirection === "asc" ? "desc" : "asc")}
-            aria-label={`Sort ${sortDirection === "asc" ? "ascending" : "descending"}`}
+            aria-label={`Sort ${sortDirection === "asc" ? "descending" : "ascending"}`}
           >
             {sortDirection === "asc" ? (
               <ArrowUp className="h-4 w-4" />
diff --git a/app/components/standings/__tests__/TeamScoreBreakdown.test.tsx b/app/components/standings/__tests__/TeamScoreBreakdown.test.tsx
index f895db5..5d1c66b 100644
--- a/app/components/standings/__tests__/TeamScoreBreakdown.test.tsx
+++ b/app/components/standings/__tests__/TeamScoreBreakdown.test.tsx
@@ -519,6 +519,11 @@ describe("TeamScoreBreakdown", () => {
       );
     }
 
+    async function selectSortColumn(user: ReturnType<typeof userEvent.setup>, label: string) {
+      await user.click(screen.getByRole("combobox", { name: /sort by/i }));
+      await user.click(screen.getByRole("option", { name: label }));
+    }
+
     it("renders a card per pick with all fields visible", () => {
       renderWithRouter(
         <TeamScoreBreakdown
@@ -571,7 +576,7 @@ describe("TeamScoreBreakdown", () => {
       );
 
       // picks: Team A=100, Driver B=50, Team C=0; points defaults to descending
-      await user.selectOptions(screen.getByLabelText(/sort by/i), "points");
+      await selectSortColumn(user, "Points");
       expect(getCardOrder()).toEqual(["Team A", "Driver B", "Team C"]);
     });
 
@@ -587,8 +592,9 @@ describe("TeamScoreBreakdown", () => {
         />
       );
 
-      await user.selectOptions(screen.getByLabelText(/sort by/i), "points"); // desc
-      await user.click(screen.getByRole("button", { name: /sort descending/i }));
+      await selectSortColumn(user, "Points"); // desc
+      // While descending, the toggle's action is to sort ascending.
+      await user.click(screen.getByRole("button", { name: /sort ascending/i }));
       // now ascending: Team C (0), Driver B (50), Team A (100)
       expect(getCardOrder()).toEqual(["Team C", "Driver B", "Team A"]);
     });
diff --git a/app/test/setup.ts b/app/test/setup.ts
index f39c595..adf939d 100644
--- a/app/test/setup.ts
+++ b/app/test/setup.ts
@@ -29,6 +29,14 @@ process.env.NODE_ENV = 'test';
 // jsdom does not implement scrollTo on elements
 HTMLElement.prototype.scrollTo = vi.fn();
 
+// jsdom does not implement these APIs that Radix UI relies on for pointer
+// interactions and option positioning (Select, DropdownMenu, etc.). Without
+// them, opening a Radix Select in a test throws.
+HTMLElement.prototype.scrollIntoView = vi.fn();
+HTMLElement.prototype.hasPointerCapture = vi.fn(() => false);
+HTMLElement.prototype.setPointerCapture = vi.fn();
+HTMLElement.prototype.releasePointerCapture = vi.fn();
+
 // Mock BetterAuth
 vi.mock('~/lib/auth.server', () => ({
   auth: {