Extract chess clock presets to shared constant (#432)

* 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 <noreply@anthropic.com>
2026-05-15 14:02:49 -07:00 · 2026-05-15 14:02:49 -07:00 · d92f73e449
commit d92f73e449
parent 6f70a19b73
4 changed files with 124 additions and 28 deletions
--- 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 {
--- 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", () => {
--- 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:
+
+  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";
 }

 /**
--- 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<Set<string>>(
    () => 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

        <main className="space-y-6">
          {showUpdateForm && (
-            <Form method="post" noValidate className="space-y-6">
+            <Form method="post" noValidate className="space-y-6" onSubmit={() => setHasUnsavedSettingsChanges(false)}>
              <input type="hidden" name="intent" value="update" />

              <LeagueBasicsSection