Address code-review findings on draft schedule Gantt
- Use make_interval(months => $n) instead of parameterized interval
multiplication so Postgres can infer the bound param's type (the prior
form risked a "could not determine data type of parameter" runtime error)
- Fix subject/verb grammar in the coverage-gap warning for a single sport
("1 sport has" vs "N sports have")
- Stack overlapping draft windows for the same sport into separate lanes
via greedy interval scheduling so bars no longer render on top of one
another; row height grows with the number of concurrent windows
- Add a test covering overlapping windows rendering as separate bars
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fd51U9DPMNz4KzBeucR6CK
This commit is contained in:
parent
7f021aa534
commit
77963a13f4
4 changed files with 83 additions and 9 deletions
|
|
@ -46,6 +46,40 @@ const STATUS_LABELS: Record<DraftScheduleWindow["status"], string> = {
|
||||||
|
|
||||||
const clampPct = (n: number) => Math.max(0, Math.min(100, n));
|
const clampPct = (n: number) => Math.max(0, Math.min(100, n));
|
||||||
|
|
||||||
|
// Row layout (px). A single-lane row is LANE_HEIGHT + ROW_V_PAD tall; extra
|
||||||
|
// concurrent windows add one lane each so overlapping bars never stack on top
|
||||||
|
// of one another.
|
||||||
|
const LANE_HEIGHT = 30;
|
||||||
|
const LANE_GAP = 8;
|
||||||
|
const ROW_V_PAD = 14;
|
||||||
|
|
||||||
|
const rowHeight = (laneCount: number) => Math.max(laneCount, 1) * LANE_HEIGHT + ROW_V_PAD;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Greedy interval-scheduling: assign each window to the first lane whose last
|
||||||
|
* bar ends before this one starts, otherwise open a new lane. Windows arrive
|
||||||
|
* sorted by draftOn (see findDraftScheduleForHorizon).
|
||||||
|
*/
|
||||||
|
function assignLanes(
|
||||||
|
windows: DraftScheduleWindow[],
|
||||||
|
start: Date
|
||||||
|
): { placed: Array<{ window: DraftScheduleWindow; lane: number }>; laneCount: number } {
|
||||||
|
const laneEnds: number[] = [];
|
||||||
|
const placed = windows.map((window) => {
|
||||||
|
const startDay = differenceInCalendarDays(parseISO(window.draftOn), start);
|
||||||
|
const endDay = differenceInCalendarDays(parseISO(window.draftOff), start);
|
||||||
|
let lane = laneEnds.findIndex((laneEnd) => laneEnd < startDay);
|
||||||
|
if (lane === -1) {
|
||||||
|
lane = laneEnds.length;
|
||||||
|
laneEnds.push(endDay);
|
||||||
|
} else {
|
||||||
|
laneEnds[lane] = endDay;
|
||||||
|
}
|
||||||
|
return { window, lane };
|
||||||
|
});
|
||||||
|
return { placed, laneCount: Math.max(laneEnds.length, 1) };
|
||||||
|
}
|
||||||
|
|
||||||
export function DraftScheduleGantt({ sports, today, months }: DraftScheduleGanttProps) {
|
export function DraftScheduleGantt({ sports, today, months }: DraftScheduleGanttProps) {
|
||||||
const start = parseISO(today);
|
const start = parseISO(today);
|
||||||
const end = addMonths(start, months);
|
const end = addMonths(start, months);
|
||||||
|
|
@ -58,6 +92,12 @@ export function DraftScheduleGantt({ sports, today, months }: DraftScheduleGantt
|
||||||
.map((date) => ({ date, left: pct(date) }))
|
.map((date) => ({ date, left: pct(date) }))
|
||||||
.filter((m) => m.left >= 0 && m.left <= 100);
|
.filter((m) => m.left >= 0 && m.left <= 100);
|
||||||
|
|
||||||
|
// Pre-compute lane assignment + height for each sport row.
|
||||||
|
const rows = sports.map((sport) => {
|
||||||
|
const { placed, laneCount } = assignLanes(sport.windows, start);
|
||||||
|
return { sport, placed, height: rowHeight(laneCount) };
|
||||||
|
});
|
||||||
|
|
||||||
if (sports.length === 0) {
|
if (sports.length === 0) {
|
||||||
return (
|
return (
|
||||||
<Card>
|
<Card>
|
||||||
|
|
@ -106,10 +146,11 @@ export function DraftScheduleGantt({ sports, today, months }: DraftScheduleGantt
|
||||||
{/* Sport labels column */}
|
{/* Sport labels column */}
|
||||||
<div className="w-40 shrink-0">
|
<div className="w-40 shrink-0">
|
||||||
<div className="h-8" aria-hidden="true" />
|
<div className="h-8" aria-hidden="true" />
|
||||||
{sports.map((sport) => (
|
{rows.map(({ sport, height }) => (
|
||||||
<div
|
<div
|
||||||
key={sport.id}
|
key={sport.id}
|
||||||
className="flex h-11 items-center border-b border-border/50 pr-2"
|
style={{ height }}
|
||||||
|
className="flex items-center border-b border-border/50 pr-2"
|
||||||
>
|
>
|
||||||
<span className="truncate text-sm font-medium" title={sport.name}>
|
<span className="truncate text-sm font-medium" title={sport.name}>
|
||||||
{sport.name}
|
{sport.name}
|
||||||
|
|
@ -150,17 +191,18 @@ export function DraftScheduleGantt({ sports, today, months }: DraftScheduleGantt
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
{/* Rows */}
|
{/* Rows */}
|
||||||
{sports.map((sport) => (
|
{rows.map(({ sport, placed, height }) => (
|
||||||
<div
|
<div
|
||||||
key={sport.id}
|
key={sport.id}
|
||||||
className="relative h-11 border-b border-border/50"
|
style={{ height }}
|
||||||
|
className="relative border-b border-border/50"
|
||||||
>
|
>
|
||||||
{sport.windows.length === 0 ? (
|
{placed.length === 0 ? (
|
||||||
<span className="absolute left-2 top-1/2 -translate-y-1/2 text-xs italic text-muted-foreground/70">
|
<span className="absolute left-2 top-1/2 -translate-y-1/2 text-xs italic text-muted-foreground/70">
|
||||||
No draft window
|
No draft window
|
||||||
</span>
|
</span>
|
||||||
) : (
|
) : (
|
||||||
sport.windows.map((w) => {
|
placed.map(({ window: w, lane }) => {
|
||||||
const left = clampPct(pct(parseISO(w.draftOn)));
|
const left = clampPct(pct(parseISO(w.draftOn)));
|
||||||
const right = clampPct(pct(parseISO(w.draftOff)));
|
const right = clampPct(pct(parseISO(w.draftOff)));
|
||||||
const width = Math.max(right - left, 0.75);
|
const width = Math.max(right - left, 0.75);
|
||||||
|
|
@ -168,10 +210,12 @@ export function DraftScheduleGantt({ sports, today, months }: DraftScheduleGantt
|
||||||
<Link
|
<Link
|
||||||
key={w.id}
|
key={w.id}
|
||||||
to={`/admin/sports-seasons/${w.id}`}
|
to={`/admin/sports-seasons/${w.id}`}
|
||||||
className="absolute top-1/2 flex h-6 -translate-y-1/2 items-center overflow-hidden rounded px-1.5 text-xs font-medium text-background transition-opacity hover:opacity-80"
|
className="absolute flex items-center overflow-hidden rounded px-1.5 text-xs font-medium text-background transition-opacity hover:opacity-80"
|
||||||
style={{
|
style={{
|
||||||
left: `${left}%`,
|
left: `${left}%`,
|
||||||
width: `${width}%`,
|
width: `${width}%`,
|
||||||
|
top: ROW_V_PAD / 2 + lane * LANE_HEIGHT,
|
||||||
|
height: LANE_HEIGHT - LANE_GAP,
|
||||||
backgroundColor: STATUS_COLORS[w.status],
|
backgroundColor: STATUS_COLORS[w.status],
|
||||||
}}
|
}}
|
||||||
title={`${w.name} (${w.year}) · ${w.draftOn} → ${w.draftOff}`}
|
title={`${w.name} (${w.year}) · ${w.draftOn} → ${w.draftOff}`}
|
||||||
|
|
|
||||||
|
|
@ -36,6 +36,35 @@ describe("DraftScheduleGantt", () => {
|
||||||
expect(bar).toHaveAttribute("href", "/admin/sports-seasons/ss-1");
|
expect(bar).toHaveAttribute("href", "/admin/sports-seasons/ss-1");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("renders overlapping windows as separate, clickable bars", () => {
|
||||||
|
const overlappingWindow = {
|
||||||
|
id: "ss-2",
|
||||||
|
name: "2027 NBA Playoffs",
|
||||||
|
year: 2027,
|
||||||
|
status: "active" as const,
|
||||||
|
draftOn: "2026-08-15",
|
||||||
|
draftOff: "2026-10-01",
|
||||||
|
sport: { id: "nba", name: "NBA", slug: "nba", iconUrl: null },
|
||||||
|
};
|
||||||
|
|
||||||
|
renderGantt([
|
||||||
|
{
|
||||||
|
id: "nba",
|
||||||
|
name: "NBA",
|
||||||
|
slug: "nba",
|
||||||
|
iconUrl: null,
|
||||||
|
windows: [nbaWindow, overlappingWindow],
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
|
||||||
|
expect(
|
||||||
|
screen.getByRole("link", { name: /2026 NBA Playoffs/ })
|
||||||
|
).toHaveAttribute("href", "/admin/sports-seasons/ss-1");
|
||||||
|
expect(
|
||||||
|
screen.getByRole("link", { name: /2027 NBA Playoffs/ })
|
||||||
|
).toHaveAttribute("href", "/admin/sports-seasons/ss-2");
|
||||||
|
});
|
||||||
|
|
||||||
it("shows a 'No draft window' hint for a sport with no windows", () => {
|
it("shows a 'No draft window' hint for a sport with no windows", () => {
|
||||||
renderGantt([
|
renderGantt([
|
||||||
{ id: "golf", name: "Golf", slug: "golf", iconUrl: null, windows: [] },
|
{ id: "golf", name: "Golf", slug: "golf", iconUrl: null, windows: [] },
|
||||||
|
|
|
||||||
|
|
@ -194,7 +194,7 @@ export async function findDraftScheduleForHorizon(
|
||||||
monthsAhead: number
|
monthsAhead: number
|
||||||
): Promise<DraftScheduleWindow[]> {
|
): Promise<DraftScheduleWindow[]> {
|
||||||
const db = database();
|
const db = database();
|
||||||
const horizonEnd = sql`CURRENT_DATE + (${monthsAhead} * interval '1 month')`;
|
const horizonEnd = sql`CURRENT_DATE + make_interval(months => ${monthsAhead})`;
|
||||||
const seasons = await db.query.sportsSeasons.findMany({
|
const seasons = await db.query.sportsSeasons.findMany({
|
||||||
where: (ss, { and }) =>
|
where: (ss, { and }) =>
|
||||||
and(
|
and(
|
||||||
|
|
|
||||||
|
|
@ -91,7 +91,8 @@ export default function AdminDraftSchedule({ loaderData }: Route.ComponentProps)
|
||||||
</CardTitle>
|
</CardTitle>
|
||||||
<CardDescription className="text-amber-700 dark:text-amber-400/80">
|
<CardDescription className="text-amber-700 dark:text-amber-400/80">
|
||||||
{sportsWithoutCoverage.length} sport
|
{sportsWithoutCoverage.length} sport
|
||||||
{sportsWithoutCoverage.length !== 1 ? "s" : ""} have no draft window
|
{sportsWithoutCoverage.length !== 1 ? "s" : ""}{" "}
|
||||||
|
{sportsWithoutCoverage.length !== 1 ? "have" : "has"} no draft window
|
||||||
open or upcoming in the next {months} months.
|
open or upcoming in the next {months} months.
|
||||||
</CardDescription>
|
</CardDescription>
|
||||||
</CardHeader>
|
</CardHeader>
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue