From d92f73e449719153589c6f25bcbff545f2e6466f Mon Sep 17 00:00:00 2001 From: Chris Parsons <438676+chrisparsons83@users.noreply.github.com> Date: Fri, 15 May 2026 14:02:49 -0700 Subject: [PATCH] Extract chess clock presets to shared constant (#432) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix custom chess-clock timer detection and save settings blocker - Extract getInitialDraftSpeed() helper so both useState init and resetSettingsFormState use the same logic; unrecognized presets now produce a "custom:{bank}:{incr}" string instead of falling back to "standard", so the custom section auto-expands correctly (e.g. 8 hr bank + 30 min increment is no longer shown as Standard) - resetSettingsFormState was not resetting draftSpeed at all; fixed - Clear hasUnsavedSettingsChanges in the Form's onSubmit so the useBlocker check is false at the moment the save navigation starts, preventing the "Leave without saving?" dialog from firing on save - Replace the cleared-on-success useEffect with one that re-marks dirty when the action returns an error, so the warning reappears after a failed save https://claude.ai/code/session_01DgHNkvJXGizx41CE2tkVS1 * Address code review: single source of truth for presets, useEffect guard, tests - Extract CHESS_CLOCK_PRESETS (pure data) into draft-timer.ts as the canonical source of truth; DraftSpeedPicker.tsx now spreads those values into CHESS_PRESETS rather than duplicating the numbers - getInitialDraftSpeed moved to draft-timer.ts and rewritten to use CHESS_CLOCK_PRESETS.find() — no more hardcoded bankSec/incrSec values in three separate places - parseDraftSpeed switch replaced with CHESS_CLOCK_PRESETS.find() and an explicit fallback comment; eliminates the silent default-handles- "standard" fragility called out in review - useEffect that re-marks settings dirty now guards against non-settings errors (draft-order, commissioner actions) which carry a `section` field — previously any error actionData would spuriously set hasUnsavedSettingsChanges=true - Add parseDraftSpeed and getInitialDraftSpeed test suites to draft-timer.test.ts, including parametrised preset coverage via it.each(CHESS_CLOCK_PRESETS) https://claude.ai/code/session_01DgHNkvJXGizx41CE2tkVS1 * Fix lint: use !== null/undefined instead of != null oxlint enforces eqeqeq; the null check in getInitialDraftSpeed used != which triggered two errors. https://claude.ai/code/session_01DgHNkvJXGizx41CE2tkVS1 --------- Co-authored-by: Claude --- app/components/league/DraftSpeedPicker.tsx | 9 +-- app/lib/__tests__/draft-timer.test.ts | 72 ++++++++++++++++++++++ app/lib/draft-timer.ts | 46 +++++++++++--- app/routes/leagues/$leagueId.settings.tsx | 25 +++----- 4 files changed, 124 insertions(+), 28 deletions(-) diff --git a/app/components/league/DraftSpeedPicker.tsx b/app/components/league/DraftSpeedPicker.tsx index a65a634..ae37695 100644 --- a/app/components/league/DraftSpeedPicker.tsx +++ b/app/components/league/DraftSpeedPicker.tsx @@ -10,6 +10,7 @@ import { SelectValue, } from "~/components/ui/select"; import { cn } from "~/lib/utils"; +import { CHESS_CLOCK_PRESETS } from "~/lib/draft-timer"; // ─── Types & constants ──────────────────────────────────────────────────────── @@ -37,10 +38,10 @@ export const CHESS_PRESETS: { bankSec: number; incrSec: number; }[] = [ - { value: "fast", label: "Fast", icon: Zap, desc: "~1 hour", bankSec: 60, incrSec: 10 }, - { value: "standard", label: "Standard", icon: Timer, desc: "~90 minutes", bankSec: 120, incrSec: 15 }, - { value: "slow", label: "Slow", icon: Coffee, desc: "~1 week", bankSec: 28800, incrSec: 3600 }, - { value: "very-slow", label: "Very Slow", icon: Snail, desc: "~2 weeks", bankSec: 43200, incrSec: 3600 }, + { ...CHESS_CLOCK_PRESETS[0], label: "Fast", icon: Zap, desc: "~1 hour" }, + { ...CHESS_CLOCK_PRESETS[1], label: "Standard", icon: Timer, desc: "~90 minutes" }, + { ...CHESS_CLOCK_PRESETS[2], label: "Slow", icon: Coffee, desc: "~1 week" }, + { ...CHESS_CLOCK_PRESETS[3], label: "Very Slow", icon: Snail, desc: "~2 weeks" }, ]; export function isSlowPreset(s: string): boolean { diff --git a/app/lib/__tests__/draft-timer.test.ts b/app/lib/__tests__/draft-timer.test.ts index 04b24ff..f3bdb41 100644 --- a/app/lib/__tests__/draft-timer.test.ts +++ b/app/lib/__tests__/draft-timer.test.ts @@ -3,8 +3,80 @@ import { formatClockTime, calculateTimeAfterPick, getTimerColorClass, + parseDraftSpeed, + getInitialDraftSpeed, + CHESS_CLOCK_PRESETS, } from "../draft-timer"; +// ─── parseDraftSpeed ───────────────────────────────────────────────────────── + +describe("parseDraftSpeed", () => { + describe("standard mode", () => { + it("returns the raw seconds for both fields", () => { + expect(parseDraftSpeed("120", "standard")).toEqual({ draftInitialTime: 120, draftIncrementTime: 120 }); + expect(parseDraftSpeed("28800", "standard")).toEqual({ draftInitialTime: 28800, draftIncrementTime: 28800 }); + }); + + it("falls back to 90s for null or unparseable input", () => { + expect(parseDraftSpeed(null, "standard")).toEqual({ draftInitialTime: 90, draftIncrementTime: 90 }); + expect(parseDraftSpeed("abc", "standard")).toEqual({ draftInitialTime: 90, draftIncrementTime: 90 }); + }); + }); + + describe("chess_clock mode — named presets", () => { + it.each(CHESS_CLOCK_PRESETS)("maps $value to bankSec=$bankSec / incrSec=$incrSec", ({ value, bankSec, incrSec }) => { + expect(parseDraftSpeed(value, "chess_clock")).toEqual({ + draftInitialTime: bankSec, + draftIncrementTime: incrSec, + }); + }); + }); + + describe("chess_clock mode — custom string", () => { + it("parses custom:{bank}:{incr} correctly", () => { + expect(parseDraftSpeed("custom:28800:1800", "chess_clock")).toEqual({ + draftInitialTime: 28800, + draftIncrementTime: 1800, + }); + }); + }); + + describe("chess_clock mode — fallback", () => { + it("returns standard preset values for null or unknown input", () => { + expect(parseDraftSpeed(null, "chess_clock")).toEqual({ draftInitialTime: 120, draftIncrementTime: 15 }); + expect(parseDraftSpeed("unknown", "chess_clock")).toEqual({ draftInitialTime: 120, draftIncrementTime: 15 }); + }); + }); +}); + +// ─── getInitialDraftSpeed ───────────────────────────────────────────────────── + +describe("getInitialDraftSpeed", () => { + it("returns increment seconds as string for standard mode", () => { + expect(getInitialDraftSpeed({ draftTimerMode: "standard", draftInitialTime: 120, draftIncrementTime: 120 })).toBe("120"); + expect(getInitialDraftSpeed({ draftTimerMode: "standard", draftInitialTime: 28800, draftIncrementTime: 28800 })).toBe("28800"); + }); + + it("defaults to '90' for standard mode with missing increment", () => { + expect(getInitialDraftSpeed({ draftTimerMode: "standard", draftInitialTime: null, draftIncrementTime: null })).toBe("90"); + }); + + it.each(CHESS_CLOCK_PRESETS)("recognises the $value preset from its DB values", ({ value, bankSec, incrSec }) => { + expect(getInitialDraftSpeed({ draftTimerMode: "chess_clock", draftInitialTime: bankSec, draftIncrementTime: incrSec })).toBe(value); + }); + + it("returns custom:{bank}:{incr} for non-preset chess clock values", () => { + expect(getInitialDraftSpeed({ draftTimerMode: "chess_clock", draftInitialTime: 28800, draftIncrementTime: 1800 })) + .toBe("custom:28800:1800"); + }); + + it("defaults to 'standard' preset key when chess clock has no saved values", () => { + expect(getInitialDraftSpeed({ draftTimerMode: "chess_clock", draftInitialTime: null, draftIncrementTime: null })).toBe("standard"); + expect(getInitialDraftSpeed(null)).toBe("standard"); + expect(getInitialDraftSpeed(undefined)).toBe("standard"); + }); +}); + // ─── formatClockTime ───────────────────────────────────────────────────────── describe("formatClockTime", () => { diff --git a/app/lib/draft-timer.ts b/app/lib/draft-timer.ts index 63a0455..10b2d5a 100644 --- a/app/lib/draft-timer.ts +++ b/app/lib/draft-timer.ts @@ -51,6 +51,14 @@ export function calculateTimeAfterPick( return bankTime + incrementTime; } +/** Pure preset data for chess-clock mode — the single source of truth shared with DraftSpeedPicker. */ +export const CHESS_CLOCK_PRESETS: { value: string; bankSec: number; incrSec: number }[] = [ + { value: "fast", bankSec: 60, incrSec: 10 }, + { value: "standard", bankSec: 120, incrSec: 15 }, + { value: "slow", bankSec: 28800, incrSec: 3600 }, + { value: "very-slow", bankSec: 43200, incrSec: 3600 }, +]; + /** * Converts a draftSpeed form value + timerMode into DB time fields. * @@ -73,16 +81,36 @@ export function parseDraftSpeed( const incr = parseInt(incrStr ?? "", 10); if (!isNaN(bank) && !isNaN(incr)) return { draftInitialTime: bank, draftIncrementTime: incr }; } - switch (draftSpeed) { - case "fast": - return { draftInitialTime: 60, draftIncrementTime: 10 }; - case "slow": - return { draftInitialTime: 28800, draftIncrementTime: 3600 }; - case "very-slow": - return { draftInitialTime: 43200, draftIncrementTime: 3600 }; - default: - return { draftInitialTime: 120, draftIncrementTime: 15 }; + + const preset = CHESS_CLOCK_PRESETS.find((p) => p.value === draftSpeed); + if (preset) return { draftInitialTime: preset.bankSec, draftIncrementTime: preset.incrSec }; + + // Unknown/null speed — fall back to "standard" preset + return { draftInitialTime: 120, draftIncrementTime: 15 }; +} + +type DraftSpeedSource = { + draftTimerMode?: "chess_clock" | "standard" | null; + draftInitialTime?: number | null; + draftIncrementTime?: number | null; +} | null | undefined; + +/** + * Derives the draftSpeed UI value (preset key or "custom:{bank}:{incr}") from + * saved DB fields. Used to initialise and reset the DraftSpeedPicker. + */ +export function getInitialDraftSpeed(season: DraftSpeedSource): string { + const mode = season?.draftTimerMode ?? "chess_clock"; + if (mode === "standard") return season?.draftIncrementTime?.toString() ?? "90"; + const preset = CHESS_CLOCK_PRESETS.find( + (p) => p.bankSec === season?.draftInitialTime && p.incrSec === season?.draftIncrementTime + ); + if (preset) return preset.value; + if (season?.draftInitialTime !== null && season?.draftInitialTime !== undefined && + season?.draftIncrementTime !== null && season?.draftIncrementTime !== undefined) { + return `custom:${season.draftInitialTime}:${season.draftIncrementTime}`; } + return "standard"; } /** diff --git a/app/routes/leagues/$leagueId.settings.tsx b/app/routes/leagues/$leagueId.settings.tsx index cdc82da..4d79451 100644 --- a/app/routes/leagues/$leagueId.settings.tsx +++ b/app/routes/leagues/$leagueId.settings.tsx @@ -12,6 +12,7 @@ import { Users, } from "lucide-react"; import { type ScoringRules, DEFAULT_SCORING_RULES } from "~/lib/scoring-types"; +import { getInitialDraftSpeed } from "~/lib/draft-timer"; import type { Route } from "./+types/$leagueId.settings"; export { action, loader } from "./$leagueId.settings.server"; import { OMNIFANTASY_SCORING } from "~/components/league/ScoringPresetPicker"; @@ -148,15 +149,7 @@ export default function LeagueSettings({ loaderData, actionData }: Route.Compone pointsFor7th: season?.pointsFor7th ?? DEFAULT_SCORING_RULES.pointsFor7th, pointsFor8th: season?.pointsFor8th ?? DEFAULT_SCORING_RULES.pointsFor8th, }); - const initTimerMode = season?.draftTimerMode ?? "chess_clock"; - const [draftSpeed, setDraftSpeed] = useState(() => { - if (initTimerMode === "standard") return season?.draftIncrementTime?.toString() ?? "90"; - if (season?.draftInitialTime === 60 && season?.draftIncrementTime === 10) return "fast"; - if (season?.draftInitialTime === 120 && season?.draftIncrementTime === 15) return "standard"; - if (season?.draftInitialTime === 28800 && season?.draftIncrementTime === 3600) return "slow"; - if (season?.draftInitialTime === 43200 && season?.draftIncrementTime === 3600) return "very-slow"; - return "standard"; - }); + const [draftSpeed, setDraftSpeed] = useState(() => getInitialDraftSpeed(season)); const [selectedSports, setSelectedSports] = useState>( () => getInitialSelectedSports({ season, allSportsSeasons }) ); @@ -184,13 +177,14 @@ export default function LeagueSettings({ loaderData, actionData }: Route.Compone setHasDraftOrderChanges(false); }, [draftSlots, teams]); - // Clear settings dirty flag after a successful (non-error) submission + // Re-mark dirty if the settings save returned an error (success path redirects away, so no cleanup + // needed there). Guard against non-settings errors (e.g. draft-order) which carry a `section` field. useEffect(() => { - if (!hasUnsavedSettingsChanges || navigation.state !== "idle") return; - if (actionData && !("error" in actionData)) { - setHasUnsavedSettingsChanges(false); + if (navigation.state !== "idle") return; + if (actionData && "error" in actionData && !("section" in actionData)) { + setHasUnsavedSettingsChanges(true); } - }, [actionData, hasUnsavedSettingsChanges, navigation.state]); + }, [actionData, navigation.state]); const canEditSports = season?.status === "pre_draft"; const canEditTeamCount = season?.status === "pre_draft"; @@ -235,6 +229,7 @@ export default function LeagueSettings({ loaderData, actionData }: Route.Compone setDiscordWebhookUrl(league.discordWebhookUrl ?? ""); setTeamCountValue(teamCount); setTimerMode(season?.draftTimerMode ?? "chess_clock"); + setDraftSpeed(getInitialDraftSpeed(season)); setOvernightMode(season?.overnightPauseMode ?? "none"); setOvernightStart(season?.overnightPauseStart ?? "23:00"); setOvernightEnd(season?.overnightPauseEnd ?? "07:00"); @@ -345,7 +340,7 @@ export default function LeagueSettings({ loaderData, actionData }: Route.Compone
{showUpdateForm && ( -
+ setHasUnsavedSettingsChanges(false)}>