From 599bba8949e26ce83fdba81463125165bb1b5c6c Mon Sep 17 00:00:00 2001 From: Chris Parsons <438676+chrisparsons83@users.noreply.github.com> Date: Mon, 2 Mar 2026 16:46:35 -0800 Subject: [PATCH] refactor: address code review findings in Roster/Summary draft views (#57) - Extract groupPicksByTeamAndSport into shared picks-utils.ts to remove duplication between TeamRosterView and DraftSummaryView - Fix TeamRosterView: make teamPicks a proper useMemo dep on selectedTeamId, add useEffect to reset stale selection if draftSlots changes - Add accessible label/htmlFor to TeamRosterView team Select - Remove dead `season` prop from DraftSummaryView (was inherited from TeamsDraftedGrid but never used) - Fix DraftSummaryView: sport name cells are now , column headers have scope="col", Total header uses solid bg-muted to match Sport - Replace || null with explicit > 0 check in Total cell for clarity - Consolidate to summaryViewProps/rosterViewProps in route for consistency Co-authored-by: Claude Sonnet 4.6 --- app/components/draft/DraftSummaryView.tsx | 114 +++++++++++++++++ app/components/draft/TeamRosterView.tsx | 121 ++++++++++++++++++ app/components/draft/picks-utils.ts | 19 +++ .../leagues/$leagueId.draft.$seasonId.tsx | 69 ++++++++-- 4 files changed, 309 insertions(+), 14 deletions(-) create mode 100644 app/components/draft/DraftSummaryView.tsx create mode 100644 app/components/draft/TeamRosterView.tsx create mode 100644 app/components/draft/picks-utils.ts diff --git a/app/components/draft/DraftSummaryView.tsx b/app/components/draft/DraftSummaryView.tsx new file mode 100644 index 0000000..fc92dfd --- /dev/null +++ b/app/components/draft/DraftSummaryView.tsx @@ -0,0 +1,114 @@ +import { memo, useMemo } from "react"; +import { type DraftPick, groupPicksByTeamAndSport } from "./picks-utils"; + +interface DraftSummaryViewProps { + draftSlots: Array<{ id: string; team: { id: string; name: string } }>; + ownerMap?: Record; + picks: DraftPick[]; + sports: Array<{ id: string; name: string }>; +} + +export const DraftSummaryView = memo(function DraftSummaryView({ + draftSlots, + ownerMap = {}, + picks, + sports, +}: DraftSummaryViewProps) { + const picksByTeamAndSport = useMemo( + () => groupPicksByTeamAndSport(picks), + [picks] + ); + + const totalPicksBySport = useMemo(() => { + const map = new Map(); + sports.forEach((sport) => { + let count = 0; + picksByTeamAndSport.forEach((teamMap) => { + count += teamMap.get(sport.id)?.length ?? 0; + }); + map.set(sport.id, count); + }); + return map; + }, [picksByTeamAndSport, sports]); + + if (sports.length === 0) { + return ( +
+

No sports have been configured for this season.

+
+ ); + } + + return ( +
+ + + + + + {draftSlots.map((slot, index) => { + const displayName = ownerMap[slot.team.id] || slot.team.name; + const isLast = index === draftSlots.length - 1; + return ( + + ); + })} + + + + {sports.map((sport, sportIndex) => { + const isLastRow = sportIndex === sports.length - 1; + const sportTotal = totalPicksBySport.get(sport.id) ?? 0; + return ( + + + + {draftSlots.map((slot, slotIndex) => { + const count = + picksByTeamAndSport.get(slot.team.id)?.get(sport.id) + ?.length ?? 0; + const isLast = slotIndex === draftSlots.length - 1; + return ( + + ); + })} + + ); + })} + +
+ Sport + + Total + + {displayName} +
+ {sport.name} + + {sportTotal > 0 ? sportTotal : null} + = 2 ? "bg-accent/30 font-semibold" : "bg-background" + } ${!isLastRow ? "border-b" : ""} ${!isLast ? "border-r" : ""}`} + > + {count > 0 ? count : null} +
+
+ ); +}); diff --git a/app/components/draft/TeamRosterView.tsx b/app/components/draft/TeamRosterView.tsx new file mode 100644 index 0000000..08ab865 --- /dev/null +++ b/app/components/draft/TeamRosterView.tsx @@ -0,0 +1,121 @@ +import { memo, useEffect, useMemo, useState } from "react"; +import { Badge } from "~/components/ui/badge"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "~/components/ui/select"; +import { type DraftPick, groupPicksByTeamAndSport } from "./picks-utils"; + +interface TeamRosterViewProps { + draftSlots: Array<{ id: string; team: { id: string; name: string } }>; + ownerMap?: Record; + picks: DraftPick[]; + sports: Array<{ id: string; name: string }>; +} + +export const TeamRosterView = memo(function TeamRosterView({ + draftSlots, + ownerMap = {}, + picks, + sports, +}: TeamRosterViewProps) { + const [selectedTeamId, setSelectedTeamId] = useState( + draftSlots[0]?.team.id ?? "" + ); + + // Reset selection if the selected team is no longer present in the slot list + useEffect(() => { + if ( + draftSlots.length > 0 && + !draftSlots.some((s) => s.team.id === selectedTeamId) + ) { + setSelectedTeamId(draftSlots[0].team.id); + } + }, [draftSlots, selectedTeamId]); + + const picksByTeamAndSport = useMemo( + () => groupPicksByTeamAndSport(picks), + [picks] + ); + + const teamPicks = useMemo( + () => picksByTeamAndSport.get(selectedTeamId), + [picksByTeamAndSport, selectedTeamId] + ); + + const sportsWithPicks = useMemo( + () => sports.filter((s) => (teamPicks?.get(s.id)?.length ?? 0) > 0), + [sports, teamPicks] + ); + + const totalPicks = useMemo(() => { + let count = 0; + teamPicks?.forEach((sp) => (count += sp.length)); + return count; + }, [teamPicks]); + + if (draftSlots.length === 0) { + return ( +
+

No teams in this draft.

+
+ ); + } + + return ( +
+
+
+ + +
+ + {totalPicks === 0 ? ( +

+ No picks yet +

+ ) : ( + sportsWithPicks.map((sport) => { + const sportPicks = teamPicks?.get(sport.id) ?? []; + return ( +
+
+

{sport.name}

+ + {sportPicks.length} + +
+
    + {sportPicks.map((pick) => ( +
  • + {pick.participant.name} +
    • + ))} +
+
+ ); + }) + )} +
+
+ ); +}); diff --git a/app/components/draft/picks-utils.ts b/app/components/draft/picks-utils.ts new file mode 100644 index 0000000..733cd5a --- /dev/null +++ b/app/components/draft/picks-utils.ts @@ -0,0 +1,19 @@ +export type DraftPick = { + id: string; + team: { id: string; name: string }; + participant: { id: string; name: string }; + sport: { id: string; name: string }; +}; + +export function groupPicksByTeamAndSport( + picks: DraftPick[] +): Map> { + const map = new Map>(); + picks.forEach((pick) => { + if (!map.has(pick.team.id)) map.set(pick.team.id, new Map()); + const teamMap = map.get(pick.team.id)!; + if (!teamMap.has(pick.sport.id)) teamMap.set(pick.sport.id, []); + teamMap.get(pick.sport.id)!.push(pick); + }); + return map; +} diff --git a/app/routes/leagues/$leagueId.draft.$seasonId.tsx b/app/routes/leagues/$leagueId.draft.$seasonId.tsx index 8016602..987667a 100644 --- a/app/routes/leagues/$leagueId.draft.$seasonId.tsx +++ b/app/routes/leagues/$leagueId.draft.$seasonId.tsx @@ -14,7 +14,8 @@ import { getAuth } from "@clerk/react-router/server"; import { getTeamQueue } from "~/models/draft-queue"; import { buildOwnerMap } from "~/lib/owner-map"; import { DraftSidebar } from "~/components/DraftSidebar"; -import { TeamsDraftedGrid } from "~/components/draft/TeamsDraftedGrid"; +import { TeamRosterView } from "~/components/draft/TeamRosterView"; +import { DraftSummaryView } from "~/components/draft/DraftSummaryView"; import { QueueSection } from "~/components/draft/QueueSection"; import { AvailableParticipantsSection } from "~/components/draft/AvailableParticipantsSection"; import { SidebarRecentPicks } from "~/components/draft/SidebarRecentPicks"; @@ -37,7 +38,7 @@ type QueueItem = typeof schema.draftQueue.$inferSelect; const MOBILE_TABS_BASE = [ { id: "available" as const, label: "Available", Icon: Users }, { id: "board" as const, label: "Board", Icon: LayoutGrid }, - { id: "roster" as const, label: "Roster", Icon: ListChecks }, + { id: "teams" as const, label: "Teams", Icon: ListChecks }, { id: "controls" as const, label: "Controls", Icon: Settings }, ]; @@ -466,10 +467,11 @@ export default function DraftRoom() { const stored = localStorage.getItem("draftSidebarCollapsed"); return stored ? JSON.parse(stored) : false; }); - const [activeTab, setActiveTab] = useState<"participants" | "board" | "teams">("board"); - const [mobileTab, setMobileTab] = useState<"available" | "queue" | "board" | "roster" | "controls">( + const [activeTab, setActiveTab] = useState<"participants" | "board" | "rosters" | "summary">("board"); + const [mobileTab, setMobileTab] = useState<"available" | "queue" | "board" | "teams" | "controls">( !userTeam && isCommissioner ? "board" : "available" ); + const [teamsSubTab, setTeamsSubTab] = useState<"rosters" | "summary">("rosters"); const isMobile = useMediaQuery("(max-width: 767px)"); // Track autodraft status for all teams @@ -1395,13 +1397,17 @@ export default function DraftRoom() { onRollbackToPick: isCommissioner && !isDraftComplete ? handleRollbackToPick : undefined, }; - const teamsDraftedGridSeason = useMemo(() => ({ numFlexPicks }), [numFlexPicks]); - - const teamsDraftedGridProps = { + const summaryViewProps = { + draftSlots, + picks, + sports: seasonSportsData, + ownerMap, + }; + + const rosterViewProps = { draftSlots, picks, sports: seasonSportsData, - season: teamsDraftedGridSeason, ownerMap, }; @@ -1584,8 +1590,38 @@ export default function DraftRoom() { {mobileTab === "board" && ( )} - {mobileTab === "roster" && ( - + {mobileTab === "teams" && ( +
+
+ + +
+
+ {teamsSubTab === "rosters" ? ( + + ) : ( + + )} +
+
)} {mobileTab === "controls" && (
@@ -1649,7 +1685,7 @@ export default function DraftRoom() { - setActiveTab(value as "participants" | "board" | "teams") + setActiveTab(value as "participants" | "board" | "rosters" | "summary") } className="h-full flex flex-col" > @@ -1662,7 +1698,8 @@ export default function DraftRoom() { Available Participants Draft Board - Teams Drafted + Rosters + Summary {season.status === "draft" && !isDraftComplete && currentDraftSlot && ( @@ -1702,8 +1739,12 @@ export default function DraftRoom() { - - + + + + + +