diff --git a/client/src/components/mana/AmountInput.tsx b/client/src/components/mana/AmountInput.tsx new file mode 100644 index 0000000000..258685dac4 --- /dev/null +++ b/client/src/components/mana/AmountInput.tsx @@ -0,0 +1,208 @@ +import { useId } from "react"; +import { useTranslation } from "react-i18next"; + +import { gameButtonClass } from "../ui/buttonStyles.ts"; + +/** + * Shared bounded-amount control for the engine's amount prompts + * (PayAmountChoice / ChooseXValue / AssistPayment). + * + * The `[min, max]` window is ENGINE-OWNED and arrives as props — this component holds no bound of + * its own, no default, and no fallback. `parseAmount` is the single sanitization authority; it + * REJECTS (returns null) rather than coercing, so a player never submits a number they did not type. + */ + +/** Digit reading of `raw`, ignoring the window. Recovery uses this; SUBMISSION uses `parseAmount`. */ +function digitsOf(raw: string): number | null { + return /^\d+$/.test(raw) ? Number(raw) : null; +} + +export function parseAmount(raw: string, min: number, max: number): number | null { + // Digits only. `Number()` alone is NOT sufficient: MEASURED, Number("") === 0, + // Number(" 7 ") === 7, Number("1.5") === 1.5, Number("1e3") === 1000, Number("+2") === 2 and + // Number("0x10") === 16 all land INSIDE a typical window. + const value = digitsOf(raw); + return value !== null && value >= min && value <= max ? value : null; +} + +export interface AmountInputLabels { + /** aria-label for the numeric text box. */ + input: string; + /** aria-label for the − stepper. */ + decrease: string; + /** aria-label for the + stepper. */ + increase: string; +} + +export function AmountInput({ + raw, + onRawChange, + min, + max, + onSubmit, + labels, +}: { + raw: string; + onRawChange: (raw: string) => void; + min: number; + max: number; + /** Called on Enter. MUST itself reject an invalid amount — AmountInput deliberately does not + * re-guard, because a second guard would make the caller's guard unobservable and untestable. */ + onSubmit: () => void; + labels: AmountInputLabels; +}) { + const { t } = useTranslation("game"); + const amount = parseAmount(raw, min, max); + const hintId = useId(); + const errorId = useId(); + + // Recovery anchor. With the slider deleted the steppers are the only non-typing way out of an + // invalid entry, so they stay LIVE while `amount === null` and snap back into [min, max]. They + // step from the DIGIT reading, not from `amount`: `parseAmount` collapses "junk" and "out of + // range" into the same null, so stepping from `amount ?? min` would throw away a perfectly + // readable 1001 and jump to min. `parseAmount` gates SUBMISSION; `step` performs RECOVERY + // toward the window. + const step = (delta: number) => + onRawChange(String(Math.min(Math.max((digitsOf(raw) ?? min) + delta, min), max))); + const decDisabled = amount !== null && amount <= min; + const incDisabled = amount !== null && amount >= max; + + // ponytail: no showSlider/showSteppers flag — the slider is deleted, not configurable. + // ponytail: no role="alert" — assertive per-keystroke announcements are the anti-pattern; + // aria-invalid + aria-describedby is the association. + // ponytail: no pattern="[0-9]*" — inputMode="numeric" carries the modern-iOS keypad; add back + // only on a legacy-iOS report. + // ponytail: the null-guard lives once, in the caller's handleCommit — a second guard in + // onKeyDown would make it unobservable. + // role="spinbutton" IS carried, reversing an earlier note here that claimed no in-repo + // precedent and unwanted aria-value* upkeep. Both premises were wrong: `ManaCurve.tsx` already + // uses aria-valuenow, and the three controls this box replaced announced their value NATIVELY + // (`type="range"` ⇒ role slider, `type="number"` ⇒ role spinbutton). Dropping to a bare + // `type="text"` therefore made the ACCEPTED amount inaudible — pressing +/− or the arrow keys + // mutated a value nothing exposed. The role is descriptive rather than decorative because the + // box implements the pattern's CORE keyboard interaction (ArrowUp/ArrowDown step, clamped to + // the window) — not the full APG list. Home/End (jump to min/max) are deliberately NOT + // remapped: the host is a real editable text field where they carry load-bearing caret + // semantics, and native `` — whose implicit role is already spinbutton — + // does not remap them either. + // `aria-valuenow` uses the VALIDATED amount, so it is absent while the entry is out of range + // rather than contradicting aria-valuemin/max; `aria-invalid` carries that state instead. + return ( +
+
+ + onRawChange(e.target.value)} + onKeyDown={(e) => { + if (e.key === "Enter") { + e.preventDefault(); + onSubmit(); + return; + } + // type="text" has no native stepping; the accessibility floor requires arrows to step. + if (e.key === "ArrowUp") { + e.preventDefault(); + step(1); + } else if (e.key === "ArrowDown") { + e.preventDefault(); + step(-1); + } + }} + aria-label={labels.input} + role="spinbutton" + aria-valuenow={amount ?? undefined} + aria-valuemin={min} + aria-valuemax={max} + aria-invalid={amount === null} + // The window is ANNOUNCED, not merely displayed. `type="range"` announced min/max/now + // natively; a text box announces nothing, so the range hint is permanently associated + // and the error message is appended to it while invalid. + aria-describedby={amount === null ? `${hintId} ${errorId}` : hintId} + // `w-24` not `w-20`: four digits must fit for the 1000 case. `h-11` (44px) because this + // is the PRIMARY tap target of the control — fixing only the steppers would leave the + // main one short. The `::before` idiom cannot be used here regardless: `` is a + // replaced element and renders no pseudo-elements. + className={`h-11 w-24 rounded-lg border bg-gray-950/80 px-2 text-center font-mono text-base font-semibold shadow-inner outline-none transition focus:ring-2 ${ + amount === null + ? "border-red-400/60 text-red-200 focus:ring-red-400/30" + : "border-cyan-400/30 text-cyan-100 focus:ring-cyan-400/30" + }`} + /> + + + {min > 0 ? t("mana.minMax", { min, max }) : t("mana.maxOnly", { max })} + +
+ + {/* PERMANENTLY MOUNTED, and a live region — both properties are load-bearing. + `aria-invalid`/`aria-describedby` are resolved by a screen reader when focus ARRIVES at + the box, but here they flip while focus is already inside it, so association alone + announces nothing: the player would have to tab away and back to learn the entry was + refused. Mounting the node INTO a live region is equally unreliable (regions announce + MUTATIONS of existing content), so the region pre-exists and only its text changes. + `role="status"` (polite) rather than "alert": the entry is being corrected mid-typing + and must not interrupt what is already being read. `min-h-4` reserves the line so + recovering from an invalid entry does not shift the panel under the pointer. */} +

+ {amount === null ? t("mana.amountOutOfRange", { min, max }) : ""} +

+
+ ); +} diff --git a/client/src/components/mana/AssistPaymentUI.tsx b/client/src/components/mana/AssistPaymentUI.tsx index 2224db42c4..0d9323066b 100644 --- a/client/src/components/mana/AssistPaymentUI.tsx +++ b/client/src/components/mana/AssistPaymentUI.tsx @@ -5,6 +5,7 @@ import { useTranslation } from "react-i18next"; import { useCanActForWaitingState } from "../../hooks/usePlayerId.ts"; import { useGameStore } from "../../stores/gameStore.ts"; import { gameButtonClass } from "../ui/buttonStyles.ts"; +import { AmountInput, parseAmount } from "./AmountInput.tsx"; /** * CR 702.132a: Assist — the chosen helper decides how much of the spell's @@ -21,15 +22,30 @@ export function AssistPaymentUI() { const isAssistPayment = waitingFor?.type === "AssistPayment"; const max = isAssistPayment ? waitingFor.data.max_generic : 0; - const [value, setValue] = useState(0); + // Prompt identity, not just bounds: a successor prompt with the SAME max would otherwise leave + // `raw` holding the previous decision, letting the player submit an amount they chose for a + // different prompt. BOTH seats are keyed on, not just `caster` — `chosen` is the seat actually + // being asked to pay, so one caster polling two different players in succession changes only + // `chosen`, and keying on `caster` alone would not fire. + const caster = isAssistPayment ? waitingFor.data.caster : null; + const chosen = isAssistPayment ? waitingFor.data.chosen : null; + const [raw, setRaw] = useState("0"); useEffect(() => { - if (isAssistPayment) setValue(0); - }, [isAssistPayment, max]); + if (isAssistPayment) setRaw("0"); + }, [isAssistPayment, max, caster, chosen]); + + // CR 702.132a: "the player you chose may pay for any amount of the generic mana in the + // spell's total cost" — the variant's domain is 0..max_generic. This is NOT a + // frontend-invented bound: `WaitingFor::AssistPayment` carries no `min` field, and the + // engine's own prompt projection (`game::interaction::number_projection`) synthesizes + // `min: 0` for this variant. The client mirrors the engine's projection. + const amount = parseAmount(raw, 0, max); const handleCommit = useCallback(() => { - dispatch({ type: "CommitAssistPayment", data: { generic: value } }); - }, [dispatch, value]); + if (amount === null) return; + dispatch({ type: "CommitAssistPayment", data: { generic: amount } }); + }, [dispatch, amount]); if (!isAssistPayment || !canAct) return null; @@ -47,32 +63,39 @@ export function AssistPaymentUI() { {t("assist.payment.title")} -
- -
+ {/* Reusing `assist.payment.title` as the box's accessible name preserves today's + accessible name byte-for-byte (the slider already carried it) and adds no key. */} +
diff --git a/client/src/components/mana/ChooseXValueUI.tsx b/client/src/components/mana/ChooseXValueUI.tsx index e3803f219d..88bb201540 100644 --- a/client/src/components/mana/ChooseXValueUI.tsx +++ b/client/src/components/mana/ChooseXValueUI.tsx @@ -6,13 +6,16 @@ import { useCanActForWaitingState } from "../../hooks/usePlayerId.ts"; import { useGameStore } from "../../stores/gameStore.ts"; import { manaCostToShards } from "../../viewmodel/costLabel.ts"; import { gameButtonClass } from "../ui/buttonStyles.ts"; +import { AmountInput, parseAmount } from "./AmountInput.tsx"; import { ManaSymbol } from "./ManaSymbol.tsx"; /** * Overlay for the `WaitingFor::ChooseXValue` state. * - * CR 107.1b + CR 601.2f: X must be chosen as part of determining total cost, - * before mana is paid. The engine computes the upper bound (`max`) from the + * CR 107.3a + CR 601.2b: the controller of the spell chooses and announces X as part of + * casting it; CR 601.2f then locks in the total cost that X feeds, so the value must be + * settled before mana is paid. CR 107.1b (a negative number can't be chosen) is why the + * lower bound is never below 0. The engine computes the upper bound (`max`) from the * player's pool + untapped free-to-tap producers; this component is a pure * display layer that dispatches the caster's chosen value via `ChooseX`. */ @@ -31,16 +34,20 @@ export function ChooseXValueUI() { const pendingCast = isChooseX ? waitingFor.data.pending_cast : null; const xCostPreviews = isChooseX ? waitingFor.data.x_cost_previews : undefined; - const [value, setValue] = useState(0); - const clampedValue = Math.min(Math.max(value, min), max); + const [raw, setRaw] = useState(String(defaultValue)); + const amount = parseAmount(raw, min, max); const pendingCostShards = useMemo(() => { if (!pendingCast) return null; - const previewCost = xCostPreviews?.find(([x]) => x === clampedValue)?.[1]; + // While the entry is invalid there is no chosen X, so there is no cost to preview. Falling + // back to `min` here would render the mana cost of an X the caster never typed — the same + // defect on the display side that the commit guard below fixes on the dispatch side. + if (amount === null) return null; + const previewCost = xCostPreviews?.find(([x]) => x === amount)?.[1]; const cost = previewCost ?? pendingCast.cost; const shards = manaCostToShards(cost); return shards.length > 0 ? shards : null; - }, [pendingCast, xCostPreviews, clampedValue]); + }, [pendingCast, xCostPreviews, amount]); const cardName = useMemo(() => { if (!gameState || !pendingCast) return null; @@ -48,35 +55,36 @@ export function ChooseXValueUI() { }, [gameState, pendingCast]); useEffect(() => { - if (isChooseX) setValue(defaultValue); - }, [isChooseX, defaultValue]); - - const clampValue = useCallback( - (nextValue: number) => Math.min(Math.max(nextValue, min), max), - [max, min], - ); - - const handleValueChange = useCallback( - (nextValue: number) => { - if (!Number.isFinite(nextValue)) return; - setValue(Math.min(nextValue, max)); - }, - [max], - ); + if (isChooseX) setRaw(String(defaultValue)); + // `pendingCast?.object_id` keys the reset on prompt IDENTITY, not just its window: a + // successor ChooseXValue for a DIFFERENT spell with the same [min, max] would otherwise + // leave `raw` holding the X chosen for the previous spell. + // `max` is a dependency even though it does not appear in the body: re-entering ChooseXValue + // with a NARROWER max but an unchanged min leaves `defaultValue` identical, so without it the + // effect would not fire and a now-out-of-range entry would persist with no way to self-heal. + // Both sibling prompts already key their reset on their full window plus their own identity + // fields; this closes that asymmetry. (An earlier version enumerated those arrays here — the + // same commit that added the seat deps made the enumeration wrong, so it names the shape + // instead of the members.) + }, [isChooseX, defaultValue, max, pendingCast?.object_id]); const handleCommit = useCallback(() => { + // Sanitization gate: an out-of-range X is REJECTED, not clamped. Typing 99 under max=5 + // used to silently cast for 5 — a value the caster never chose. CR 107.3a: the controller + // "chooses and announces the value of X"; CR 601.2f governs total cost, not that choice. + if (amount === null) return; dispatch({ type: "ChooseX", - data: { value: clampedValue }, + data: { value: amount }, }); - }, [clampedValue, dispatch]); + }, [amount, dispatch]); const handleCancel = useCallback(() => { dispatch({ type: "CancelCast" }); }, [dispatch]); - // CR 601.2f: X is chosen by the caster; opponents observe via the stack - // ghost entry, not an interactive panel. + // CR 107.3a: X is chosen and announced by the spell's controller, so only that player gets + // this panel; opponents observe via the stack ghost entry, not an interactive panel. if (!isChooseX || !canAct || !hasValidBounds) return null; return ( @@ -107,74 +115,34 @@ export function ChooseXValueUI() { )} -
- -
- - handleValueChange(Number(e.target.value))} - onBlur={() => setValue(clampedValue)} - aria-label={t("mana.chooseXInputAria")} - className="h-9 w-20 rounded-lg border border-cyan-400/30 bg-gray-950/80 px-2 text-center font-mono text-base font-semibold text-cyan-100 shadow-inner outline-none transition focus:border-cyan-300 focus:ring-2 focus:ring-cyan-400/30" - /> - -
-
+
diff --git a/client/src/components/mana/__tests__/AmountInput.test.tsx b/client/src/components/mana/__tests__/AmountInput.test.tsx new file mode 100644 index 0000000000..1caf1eed7b --- /dev/null +++ b/client/src/components/mana/__tests__/AmountInput.test.tsx @@ -0,0 +1,329 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; + +import { AmountInput, parseAmount } from "../AmountInput.tsx"; + +const LABELS = { + input: "Enter amount", + decrease: "Decrease amount", + increase: "Increase amount", +}; + +// Building-block level (CLAUDE.md "test the building block, not the special case"): every row +// below exercises `parseAmount`/`AmountInput` across their input range, not one card's prompt. +describe("parseAmount", () => { + // DISCRIMINATING rows — each is rejected by exactly one conjunct, named per row. + // `/^\d+$/` guard rows: `Number()` alone lands every one of these INSIDE the window. + it.each([ + ["", 0, 1000, 'Number("") === 0'], + [" 7 ", 0, 1000, 'Number(" 7 ") === 7'], + ["1.5", 0, 1000, 'Number("1.5") === 1.5'], + ["1e3", 0, 1000, 'Number("1e3") === 1000'], + ["+2", 0, 1000, 'Number("+2") === 2 — discriminates at min <= 2 ONLY'], + ["0x10", 0, 100, 'Number("0x10") === 16'], + ])( + "rejects %j in [%i, %i] via the digit guard (%s)", + (raw, min, max) => { + expect(parseAmount(raw as string, min as number, max as number)).toBeNull(); + }, + ); + + // DOMINATED rows — labelled so no later reader mistakes them for probes. `Number()` already + // yields NaN (or a negative that `min >= 0` rejects), so deleting `/^\d+$/` leaves them green. + it.each([ + ["12a", 0, 10, "NaN — DOMINATED"], + ["abc", 0, 10, "NaN — DOMINATED"], + ["Infinity", 0, 10, "Infinity > max — DOMINATED"], + ["1_0", 0, 10, "NaN — DOMINATED"], + ["-1", 0, 10, "min is u32-sourced so min >= 0 always — DOMINATED"], + ["+2", 5, 1000, "2 < min=5, so the range conjunct already rejects — DOMINATED at min=5"], + ])("rejects %j in [%i, %i] (%s)", (raw, min, max) => { + expect(parseAmount(raw as string, min as number, max as number)).toBeNull(); + }); + + // Range-conjunct rows. + it("rejects a value above max", () => { + expect(parseAmount("1001", 0, 1000)).toBeNull(); + }); + + it("rejects a value below min", () => { + expect(parseAmount("3", 5, 1000)).toBeNull(); + }); + + // ACCEPTING rows — the positive controls that make every rejection above non-vacuous. + it("accepts leading zeros as legal digits", () => { + expect(parseAmount("007", 0, 10)).toBe(7); + }); + + it("accepts max itself (the window is upper-INCLUSIVE)", () => { + expect(parseAmount("1000", 0, 1000)).toBe(1000); + }); + + it("accepts min itself (the window is lower-INCLUSIVE)", () => { + expect(parseAmount("5", 5, 1000)).toBe(5); + }); +}); + +describe("AmountInput — rendered contract", () => { + afterEach(() => { + cleanup(); + }); + + // No store reset: AmountInput is prop-driven and reads no store. `cleanup` IS mandatory — + // there is no global RTL auto-cleanup in this repo, so a second mount would make + // getByLabelText throw "Found multiple elements". + + it("A11Y/associate: announces the window and the validation message", () => { + const onRawChange = vi.fn(); + const { rerender } = render( + , + ); + + const box = screen.getByLabelText("Enter amount"); + expect(box).toHaveAttribute("aria-invalid", "true"); + + // Both ids are associated while invalid, and both RESOLVE to the elements that carry the + // hint and the validation copy — an id string alone would be a dangling reference. + const describedBy = box.getAttribute("aria-describedby")?.split(" ") ?? []; + expect(describedBy).toHaveLength(2); + const described = describedBy.map((id) => document.getElementById(id)); + expect(described.map((el) => el?.textContent)).toEqual([ + "max 5", + "Enter a whole number between 0 and 5", + ]); + + expect(screen.getByLabelText("Decrease amount")).toBeInTheDocument(); + expect(screen.getByLabelText("Increase amount")).toBeInTheDocument(); + + // A11Y/valid-state: the window stays ANNOUNCED once the entry is valid. This is the half + // that reverting to `amount === null ? errorId : undefined` would red. + rerender( + , + ); + const validBox = screen.getByLabelText("Enter amount"); + expect(validBox).toHaveAttribute("aria-invalid", "false"); + const validIds = validBox.getAttribute("aria-describedby")?.split(" ") ?? []; + expect(validIds).toHaveLength(1); + expect(document.getElementById(validIds[0])?.textContent).toBe("max 5"); + }); + + // A11Y/value: the three controls this box replaced announced their value natively + // (`type="range"` ⇒ slider, `type="number"` ⇒ spinbutton). A bare `type="text"` announces + // nothing, so stepping with +/− or the arrow keys would mutate a value exposed by NOTHING — + // the live region below carries only the refusal, never the accepted amount. + it("A11Y/value: the accepted amount is exposed, and withheld while out of range", () => { + const { rerender } = render( + , + ); + + const box = screen.getByLabelText("Enter amount"); + expect(box).toHaveAttribute("role", "spinbutton"); + expect(box).toHaveAttribute("aria-valuenow", "3"); + expect(box).toHaveAttribute("aria-valuemin", "0"); + expect(box).toHaveAttribute("aria-valuemax", "5"); + + // Out of range: the value is WITHHELD rather than reported, so a screen reader is never told + // a valuenow that contradicts valuemax. `aria-invalid` carries that state instead. + rerender( + , + ); + const invalidBox = screen.getByLabelText("Enter amount"); + expect(invalidBox).not.toHaveAttribute("aria-valuenow"); + expect(invalidBox).toHaveAttribute("aria-invalid", "true"); + }); + + // A11Y/live-region: the refusal must be ANNOUNCED, not merely associated. `aria-describedby` + // is resolved by a screen reader when focus ARRIVES at the box, but the message appears while + // focus is already inside it — so association alone is silent, and the player would have to tab + // away and back to learn the entry was refused. The region must therefore PRE-EXIST its message + // and only mutate its text; inserting the region (or inserting a node into one) is not + // reliably announced. + it("A11Y/live-region: the validation copy lives in a polite region that pre-exists it", () => { + const { rerender } = render( + , + ); + + // VALID first. This is the half that reds under a conditional mount, where the node does not + // exist until the entry goes invalid. + const region = document.querySelector('[role="status"]'); + expect(region).not.toBeNull(); + expect(region).toHaveAttribute("aria-live", "polite"); + expect(region).toBeEmptyDOMElement(); + + rerender( + , + ); + + // The SAME node, with mutated text — that identity is the property that makes it audible. + // A different node here would mean the message was inserted rather than announced. + expect(document.querySelector('[role="status"]')).toBe(region); + expect(region).toHaveTextContent("Enter a whole number between 0 and 5"); + }); + + it("A11Y/arrows: ArrowUp/ArrowDown step and clamp to the window", () => { + const onRawChange = vi.fn(); + const { rerender } = render( + , + ); + + const box = screen.getByLabelText("Enter amount"); + fireEvent.keyDown(box, { key: "ArrowUp" }); + expect(onRawChange).toHaveBeenLastCalledWith("5"); + fireEvent.keyDown(box, { key: "ArrowDown" }); + expect(onRawChange).toHaveBeenLastCalledWith("3"); + + // Clamped, not 6 — the distinct probe for `Math.min(…, max)`. + rerender( + , + ); + fireEvent.keyDown(screen.getByLabelText("Enter amount"), { key: "ArrowUp" }); + expect(onRawChange).toHaveBeenLastCalledWith("5"); + }); + + it("N5/recover-junk: steppers stay LIVE on an unreadable entry and re-enter the window", () => { + const onRawChange = vi.fn(); + render( + , + ); + + // The recovery anchor: with the slider deleted these are the only non-typing way out. + expect(screen.getByLabelText("Decrease amount")).not.toBeDisabled(); + expect(screen.getByLabelText("Increase amount")).not.toBeDisabled(); + + fireEvent.click(screen.getByLabelText("Increase amount")); + expect(onRawChange).toHaveBeenLastCalledWith("1"); + fireEvent.click(screen.getByLabelText("Decrease amount")); + expect(onRawChange).toHaveBeenLastCalledWith("0"); + }); + + it("N5/recover-range: a READABLE out-of-range entry steps back to the bound, not to min", () => { + const onRawChange = vi.fn(); + render( + , + ); + + // THE N5 discriminator. `step` reads the DIGITS of raw, so 1001 recovers to 1000. Under the + // `(amount ?? min) + delta` implementation both of these would be "0"/"1" — and the "abc" + // case above stays green under BOTH, which is why the two cases are separate tests. + fireEvent.click(screen.getByLabelText("Decrease amount")); + expect(onRawChange).toHaveBeenLastCalledWith("1000"); + fireEvent.click(screen.getByLabelText("Increase amount")); + expect(onRawChange).toHaveBeenLastCalledWith("1000"); + }); + + // The Enter route at the BUILDING-BLOCK level. The caller suites (T6b, AP/enter) assert only + // that an invalid entry never reaches the engine — and that is satisfied EQUALLY by "onSubmit + // was never called" and by "onSubmit was called and the caller's guard rejected it". Nothing + // separated those two until this row, which is what turns the `onSubmit` prop comment ("MUST + // itself reject an invalid amount — AmountInput deliberately does not re-guard") from an + // assertion in prose into one the suite checks. The second half is the discriminating half: + // re-adding a guard here is exactly the change the comment forbids, and it reds only this row. + it("enter: Enter calls onSubmit, and calls it EVEN when the entry is out of range", () => { + const onSubmit = vi.fn(); + const { rerender } = render( + , + ); + fireEvent.keyDown(screen.getByLabelText("Enter amount"), { key: "Enter" }); + expect(onSubmit).toHaveBeenCalledTimes(1); + + rerender( + , + ); + fireEvent.keyDown(screen.getByLabelText("Enter amount"), { key: "Enter" }); + expect(onSubmit).toHaveBeenCalledTimes(2); + }); + + // ponytail: no `min > 0` hint row here. A review asked for one on the premise that the branch + // was uncovered; MEASURED, both legs are already dominated, so the row would detect nothing NEW. + // Collapsing the ternary to the max-only string reds `ChooseXValueUI`'s "min 1 / max 10"; + // collapsing it to the two-sided string reds three rows (this file's A11Y/associate equality on + // "max 5", the assist suite's "max 4", and a ChooseXValue row) because each asserts the hint's + // FULL text, not a substring. A candidate row asserting both legs does red alongside them — it + // is the two mutants' set of DETECTIONS it fails to grow, not their failure count. That is the + // definition of dominated, and it is why the row is absent: it would report coverage it is not + // supplying. Both mutants are one-line and re-runnable if you want to check the claim. +}); diff --git a/client/src/components/mana/__tests__/AssistPaymentUI.test.tsx b/client/src/components/mana/__tests__/AssistPaymentUI.test.tsx index 0d2e1531e8..8eec43675f 100644 --- a/client/src/components/mana/__tests__/AssistPaymentUI.test.tsx +++ b/client/src/components/mana/__tests__/AssistPaymentUI.test.tsx @@ -51,7 +51,9 @@ describe("AssistPaymentUI", () => { expect(container).toBeEmptyDOMElement(); }); - it("clamps the slider to [0, max] and defaults to 0", () => { + // REWRITE: the `getByRole("slider")` subject no longer exists. The engine's [0, max] + // window is now shown by the shared hint rather than by the control's native min/max. + it("defaults to 0 and shows the engine [0, max] window", () => { const waitingFor = assistPaymentWaitingFor(4); setGameStoreForTest({ gameState: createGameState({ waiting_for: waitingFor }), @@ -60,10 +62,75 @@ describe("AssistPaymentUI", () => { render(); - const slider = screen.getByRole("slider") as HTMLInputElement; - expect(slider.min).toBe("0"); - expect(slider.max).toBe("4"); - expect(slider.value).toBe("0"); + // The box keeps the slider's accessible name byte-for-byte. + expect(screen.getByLabelText("Assist: Pay Generic Mana")).toHaveValue("0"); + expect(screen.getByText("max 4")).toBeInTheDocument(); + // At the 0 default the lower bound is already reached. + expect(screen.getByLabelText("Decrease amount")).toBeDisabled(); + }); + + // `max_generic` is held CONSTANT so the caster is the only changing dep: keyed on the bound + // alone the effect would not fire, and the amount chosen for the previous assist prompt would + // carry into this one. The PAYER axis is covered separately by AP/successor-seat below — an + // earlier version of this note claimed a same-caster successor was indistinguishable, which + // `chosen` disproves. + it("AP/successor: a same-bound prompt for a different caster resets the entry", () => { + const promptFor = (caster: number) => + buildAssistPaymentWaitingFor({ data: { caster, chosen: 0, max_generic: 4 } }); + + const first = promptFor(1); + setGameStoreForTest({ + gameState: createGameState({ waiting_for: first }), + waitingFor: first, + }); + + const { rerender } = render(); + const box = screen.getByLabelText("Assist: Pay Generic Mana"); + fireEvent.change(box, { target: { value: "3" } }); + expect(box).toHaveValue("3"); + + const successor = promptFor(2); + setGameStoreForTest({ + gameState: createGameState({ waiting_for: successor }), + waitingFor: successor, + }); + rerender(); + + expect(screen.getByLabelText("Assist: Pay Generic Mana")).toHaveValue("0"); + }); + + // The seat axis for assist: same caster, same bound, different payer. Uses the CR 723.5 + + // CR 723.3 control shape (the local seat stays `turn_decision_controller` while the semantic + // player moves) so one client renders both prompts in a row — otherwise the panel hides and + // the row would be vacuous, which is how the shape was pinned. + it("AP/successor-seat: a same-caster prompt for a different PAYER resets the entry", () => { + const promptFor = (chosen: number) => + buildAssistPaymentWaitingFor({ data: { caster: 1, chosen, max_generic: 4 } }); + + const seat = (waitingFor: WaitingFor, active: number) => { + setGameStoreForTest({ + gameState: createGameState({ + waiting_for: waitingFor, + active_player: active, + turn_decision_controller: 0, + }), + waitingFor, + }); + }; + + const first = promptFor(0); + seat(first, 0); + const { rerender } = render(); + fireEvent.change(screen.getByLabelText("Assist: Pay Generic Mana"), { + target: { value: "3" }, + }); + expect(screen.getByLabelText("Assist: Pay Generic Mana")).toHaveValue("3"); + + const second = promptFor(1); + seat(second, 1); + rerender(); + + expect(screen.getByLabelText("Assist: Pay Generic Mana")).toHaveValue("0"); }); it("dispatches CommitAssistPayment with the selected value", () => { @@ -77,8 +144,12 @@ describe("AssistPaymentUI", () => { render(); - fireEvent.change(screen.getByRole("slider"), { target: { value: "3" } }); - fireEvent.click(screen.getByRole("button")); + // Name-filtered: the panel now has 3 buttons, so a bare getByRole("button") throws + // "Found multiple elements". + fireEvent.change(screen.getByLabelText("Assist: Pay Generic Mana"), { + target: { value: "3" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Pay 3" })); expect(dispatch).toHaveBeenCalledWith({ type: "CommitAssistPayment", @@ -97,11 +168,66 @@ describe("AssistPaymentUI", () => { render(); - fireEvent.click(screen.getByRole("button")); + fireEvent.click(screen.getByRole("button", { name: "Pay nothing" })); expect(dispatch).toHaveBeenCalledWith({ type: "CommitAssistPayment", data: { generic: 0 }, }); }); + + // Enter bypasses the button's `disabled` attribute entirely, so this is the ONLY route on + // which `handleCommit`'s null-guard is observable. Matched pair: the negative alone would + // be satisfied by a component that never dispatches at all. + it("AP/enter: Enter submits a valid amount and refuses to coerce one above max_generic", () => { + const dispatch = vi.fn().mockResolvedValue([]); + const waitingFor = assistPaymentWaitingFor(4); + setGameStoreForTest({ + gameState: createGameState({ waiting_for: waitingFor }), + waitingFor, + dispatch, + }); + + render(); + + const box = screen.getByLabelText("Assist: Pay Generic Mana"); + fireEvent.change(box, { target: { value: "5" } }); + fireEvent.keyDown(box, { key: "Enter" }); + expect(dispatch).not.toHaveBeenCalled(); + + fireEvent.change(box, { target: { value: "3" } }); + fireEvent.keyDown(box, { key: "Enter" }); + expect(dispatch).toHaveBeenCalledWith({ + type: "CommitAssistPayment", + data: { generic: 3 }, + }); + }); + + // The assist domain is 0..max_generic (CR 702.132a), and the engine's own + // `number_projection` synthesizes `min: 0`. An entry above max must not reach the engine. + it("AP/above-max: an entry over max_generic cannot be committed", () => { + const dispatch = vi.fn().mockResolvedValue([]); + const waitingFor = assistPaymentWaitingFor(4); + setGameStoreForTest({ + gameState: createGameState({ waiting_for: waitingFor }), + waitingFor, + dispatch, + }); + + render(); + + fireEvent.change(screen.getByLabelText("Assist: Pay Generic Mana"), { + target: { value: "5" }, + }); + + // "Pay nothing" here was the OPPOSITE of the pending intent: the player typed 5, and + // `(amount ?? 0) === 0` collapsed the invalid entry into the decline label. Declining is + // amount === 0 exactly (pinned separately above); an invalid entry names no amount at all. + expect(screen.getByRole("button", { name: "Confirm" })).toBeDisabled(); + // DOMINATED by the assertion above, labelled rather than deleted. MEASURED on the sibling + // prompt: removing the null-guard reds only the Enter row, and a click here cannot help + // because React does not dispatch `onClick` to a disabled `button` (a React-level + // suppression, not a property of the test environment). AP/enter discriminates. + expect(dispatch).not.toHaveBeenCalled(); + }); }); diff --git a/client/src/components/mana/__tests__/ChooseXValueUI.test.tsx b/client/src/components/mana/__tests__/ChooseXValueUI.test.tsx index ff89b6a0db..2bfce9aa9f 100644 --- a/client/src/components/mana/__tests__/ChooseXValueUI.test.tsx +++ b/client/src/components/mana/__tests__/ChooseXValueUI.test.tsx @@ -85,11 +85,12 @@ describe("ChooseXValueUI", () => { expect(screen.getByText(/Choose a value for X/)).toBeInTheDocument(); expect(screen.getByText(/Nature's Rhythm/)).toBeInTheDocument(); - const slider = screen.getByLabelText("Choose X value") as HTMLInputElement; - expect(slider.min).toBe("0"); - expect(slider.max).toBe("5"); + const input = screen.getByLabelText("Enter X value") as HTMLInputElement; + // min 0 renders the one-sided hint; the engine window is shown, not the control's + // native min/max, which a text box does not carry. + expect(screen.getByText("max 5")).toBeInTheDocument(); - fireEvent.change(slider, { target: { value: "3" } }); + fireEvent.change(input, { target: { value: "3" } }); fireEvent.click(screen.getByRole("button", { name: "Confirm X = 3" })); expect(dispatch).toHaveBeenCalledWith({ type: "ChooseX", data: { value: 3 } }); @@ -123,7 +124,12 @@ describe("ChooseXValueUI", () => { expect(dispatch).toHaveBeenCalledWith({ type: "ChooseX", data: { value: 3 } }); }); - it("clamps manual numeric input upper bound immediately and lower bound on blur", () => { + // DELIBERATE BEHAVIOUR CHANGE (binding ruling D2). This test previously asserted the + // COERCION this change removes: "99" under max=5 committed 5, and "0" under min=2 + // committed 2 — values the caster never chose. Three shipped assertions flip here: the + // upper-bound clamp, the lower-bound blur reset, and the stepper gating (the steppers are + // now the recovery anchor and must stay LIVE while the entry is invalid). + it("rejects out-of-range manual X input instead of coercing it", () => { const waitingFor = chooseXWaitingFor(5, 2); setGameStoreForTest({ @@ -135,18 +141,57 @@ describe("ChooseXValueUI", () => { const input = screen.getByLabelText("Enter X value") as HTMLInputElement; fireEvent.change(input, { target: { value: "99" } }); - expect(input.value).toBe("5"); - expect(screen.getByRole("button", { name: "Confirm X = 5" })).toBeInTheDocument(); + expect(input.value).toBe("99"); + // The label carries NO value while the entry is invalid. `/^Confirm X/` would also match + // "Confirm X = 2" — an X the caster never typed — so the neutral verb is asserted exactly. + expect(screen.getByRole("button", { name: "Confirm" })).toBeDisabled(); + expect( + screen.getByText("Enter a whole number between 2 and 5"), + ).toBeInTheDocument(); fireEvent.change(input, { target: { value: "0" } }); expect(input.value).toBe("0"); - expect(screen.getByRole("button", { name: "Confirm X = 2" })).toBeInTheDocument(); - expect(screen.getByRole("button", { name: "Decrease X value" })).toBeDisabled(); - + expect(screen.getByRole("button", { name: "Confirm" })).toBeDisabled(); + expect( + screen.getByText("Enter a whole number between 2 and 5"), + ).toBeInTheDocument(); + // The recovery anchor, asserted exactly where the old gating lived. + expect( + screen.getByRole("button", { name: "Decrease X value" }), + ).not.toBeDisabled(); + + // Pins that the deleted onBlur reset does not mutate the entry. fireEvent.blur(input); - expect(input.value).toBe("2"); + expect(input.value).toBe("0"); + }); + + // The cost preview displays the CHOSEN X. While the entry is invalid there is no chosen X, so + // the previous `amount ?? min` fallback rendered the mana cost of a value the caster never + // typed — the display-side twin of the commit guard. + it("hides the pending cost preview while the entry is invalid", () => { + const waitingFor = chooseXWaitingFor(5, 2); + + setGameStoreForTest({ + gameState: createGameState({ waiting_for: waitingFor }), + waitingFor, + }); + + render(); + + // Paired positive: without this half, a component that never rendered pips at all would + // satisfy the negative below. + expect(screen.getAllByAltText("G").length).toBeGreaterThan(0); + + fireEvent.change(screen.getByLabelText("Enter X value"), { + target: { value: "99" }, + }); + + expect(screen.queryByAltText("G")).not.toBeInTheDocument(); }); + // DELIBERATE BEHAVIOUR CHANGE (binding ruling D2): the intermediate "1" no longer reads as + // a committed 10. The multi-digit-typing capability it guards SURVIVES — the final dispatch + // assertion is unchanged. it("allows typing a multi-digit X value that starts below the minimum", () => { const dispatch = vi.fn().mockResolvedValue([]); const waitingFor = chooseXWaitingFor(20, 10); @@ -162,7 +207,7 @@ describe("ChooseXValueUI", () => { const input = screen.getByLabelText("Enter X value") as HTMLInputElement; fireEvent.change(input, { target: { value: "1" } }); expect(input.value).toBe("1"); - expect(screen.getByRole("button", { name: "Confirm X = 10" })).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Confirm" })).toBeDisabled(); fireEvent.change(input, { target: { value: "15" } }); expect(input.value).toBe("15"); @@ -170,6 +215,31 @@ describe("ChooseXValueUI", () => { expect(dispatch).toHaveBeenCalledWith({ type: "ChooseX", data: { value: 15 } }); }); + // Enter bypasses the button's `disabled` attribute entirely, so this is the ONLY route on + // which `handleCommit`'s null-guard is observable. Matched pair: the negative alone would + // be satisfied by a component that never dispatches at all. + it("CX/enter: Enter submits a valid X and refuses to coerce an out-of-range one", () => { + const dispatch = vi.fn().mockResolvedValue([]); + const waitingFor = chooseXWaitingFor(5, 2); + + setGameStoreForTest({ + gameState: createGameState({ waiting_for: waitingFor }), + waitingFor, + dispatch, + }); + + render(); + + const input = screen.getByLabelText("Enter X value"); + fireEvent.change(input, { target: { value: "99" } }); + fireEvent.keyDown(input, { key: "Enter" }); + expect(dispatch).not.toHaveBeenCalled(); + + fireEvent.change(input, { target: { value: "4" } }); + fireEvent.keyDown(input, { key: "Enter" }); + expect(dispatch).toHaveBeenCalledWith({ type: "ChooseX", data: { value: 4 } }); + }); + it("dispatches CancelCast when cancel is clicked", () => { const dispatch = vi.fn().mockResolvedValue([]); const waitingFor = chooseXWaitingFor(3); @@ -199,10 +269,11 @@ describe("ChooseXValueUI", () => { const { rerender } = render(); - const slider = screen.getByLabelText("Choose X value") as HTMLInputElement; - expect(slider.min).toBe("1"); + const input = screen.getByLabelText("Enter X value") as HTMLInputElement; + // min > 0 renders the two-sided hint; the window is shown, not the control's native min. + expect(screen.getByText("min 1 / max 10")).toBeInTheDocument(); expect(screen.getByRole("button", { name: "Confirm X = 1" })).toBeInTheDocument(); - fireEvent.change(slider, { target: { value: "7" } }); + fireEvent.change(input, { target: { value: "7" } }); expect(screen.getByRole("button", { name: "Confirm X = 7" })).toBeInTheDocument(); // Simulate re-entering ChooseXValue (e.g., after cost reduction changes max) @@ -218,6 +289,91 @@ describe("ChooseXValueUI", () => { expect(screen.getByRole("button", { name: "Confirm X = 2" })).toBeInTheDocument(); }); + // The test ABOVE cannot discriminate the reset's `max` dependency: it narrows min 1 → 2 as + // well, so `defaultValue` changes and the effect fires either way. This row holds min CONSTANT, + // which is the only shape in which a reset keyed on `defaultValue` alone is observably wrong. + // Both sibling prompts already key on their full window plus their own identity fields — this + // closes that asymmetry. (This named the two dep arrays until the commit that added the seat + // fields made the enumeration wrong; it names the shape now, for the same reason the component's + // copy of this note does.) + it("resets an entry stranded above a narrowed max when min is unchanged", () => { + const dispatch = vi.fn().mockResolvedValue([]); + const waitingFor = chooseXWaitingFor(10, 0); + + setGameStoreForTest({ + gameState: createGameState({ waiting_for: waitingFor }), + waitingFor, + dispatch, + }); + + const { rerender } = render(); + + fireEvent.change(screen.getByLabelText("Enter X value"), { + target: { value: "7" }, + }); + expect( + screen.getByRole("button", { name: "Confirm X = 7" }), + ).toBeInTheDocument(); + + // Same min (0), narrower max (10 → 4). Without `max` in the dep array the entry stays "7", + // which is now permanently refused with no reset to recover it. + const nextWaitingFor = chooseXWaitingFor(4, 0); + setGameStoreForTest({ + gameState: createGameState({ waiting_for: nextWaitingFor }), + waitingFor: nextWaitingFor, + dispatch, + }); + + rerender(); + + expect(screen.getByLabelText("Enter X value")).toHaveValue("0"); + expect( + screen.getByRole("button", { name: "Confirm X = 0" }), + ).toBeInTheDocument(); + }); + + // Companion to the `max` row above, for the other identity axis: the window is held CONSTANT + // and only the SPELL changes, so `pending_cast.object_id` is the sole changing dep. Keyed on + // bounds alone, the X chosen for the previous spell would carry into the next one. + it("resets when a same-window ChooseXValue arrives for a different spell", () => { + const promptFor = (objectId: number) => + buildChooseXValueWaitingFor({ + data: { + player: 0, + max: 10, + min: 0, + pending_cast: buildPendingCast({ + object_id: objectId, + card_id: 1, + cost: { type: "Cost", shards: ["X", "G"], generic: 0 }, + }), + }, + }); + + const first = promptFor(42); + setGameStoreForTest({ + gameState: createGameState({ waiting_for: first }), + waitingFor: first, + }); + + const { rerender } = render(); + fireEvent.change(screen.getByLabelText("Enter X value"), { + target: { value: "7" }, + }); + expect( + screen.getByRole("button", { name: "Confirm X = 7" }), + ).toBeInTheDocument(); + + const successor = promptFor(43); + setGameStoreForTest({ + gameState: createGameState({ waiting_for: successor }), + waitingFor: successor, + }); + rerender(); + + expect(screen.getByLabelText("Enter X value")).toHaveValue("0"); + }); + it("renders nothing for impossible min greater than max bounds", () => { const waitingFor = chooseXWaitingFor(0, 1); @@ -230,7 +386,14 @@ describe("ChooseXValueUI", () => { expect(container).toBeEmptyDOMElement(); }); - it("range slider accepts input when mounted inside DialogHost (#2427)", () => { + // RETARGETED, not deleted. #2427's fix is DialogHost.tsx's residual-`transform` handling, + // which broke `` hit-testing in bottom-anchored panels. This test never + // asserted hit-testing — `fireEvent.change` dispatches on the node and happy-dom performs no + // layout — so it is, and always was, a MOUNT-INTEGRATION guard: the control renders and + // stays operable under DialogHost's motion wrapper and pointerEvents logic. Retargeting to + // the box preserves exactly that strength; the stepper half is added because the steppers + // are now the drag-replacement surface a #2427-class regression would break next. + it("amount box and steppers accept input when mounted inside DialogHost (#2427)", () => { const dispatch = vi.fn().mockResolvedValue([]); const waitingFor = chooseXWaitingFor(5); @@ -250,8 +413,12 @@ describe("ChooseXValueUI", () => { , ); - const slider = screen.getByLabelText("Choose X value") as HTMLInputElement; - fireEvent.change(slider, { target: { value: "4" } }); + fireEvent.change(screen.getByLabelText("Enter X value"), { + target: { value: "4" }, + }); expect(screen.getByRole("button", { name: "Confirm X = 4" })).toBeInTheDocument(); + + fireEvent.click(screen.getByRole("button", { name: "Increase X value" })); + expect(screen.getByRole("button", { name: "Confirm X = 5" })).toBeInTheDocument(); }); }); diff --git a/client/src/components/mana/__tests__/PayAmountChoiceUI.test.tsx b/client/src/components/mana/__tests__/PayAmountChoiceUI.test.tsx index 98e2d245d7..e7aa470307 100644 --- a/client/src/components/mana/__tests__/PayAmountChoiceUI.test.tsx +++ b/client/src/components/mana/__tests__/PayAmountChoiceUI.test.tsx @@ -1,8 +1,11 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { cleanup, render, screen } from "@testing-library/react"; +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; import type { LoopCollapseAxis, WaitingFor } from "../../../adapter/types"; -import { buildGameState } from "../../../test/factories/gameStateFactory.ts"; +import { + buildGameState, + buildPayAmountChoiceWaitingFor, +} from "../../../test/factories/gameStateFactory.ts"; import { setGameStoreForTest } from "../../../test/helpers/gameStoreHelpers.ts"; import { useGameStore } from "../../../stores/gameStore"; import { PayAmountChoiceUI } from "../PayAmountChoiceUI.tsx"; @@ -10,17 +13,16 @@ import { PayAmountChoiceUI } from "../PayAmountChoiceUI.tsx"; // CR 732.2a: the LoopCollapse prompt must name the axis the loop collapses. The // counter/life labels are iteration-framed (×N), never a raw token count. function loopCollapseWaitingFor(axis: LoopCollapseAxis, min = 0): WaitingFor { - return { - type: "PayAmountChoice", + return buildPayAmountChoiceWaitingFor({ data: { player: 0, resource: { type: "LoopCollapse", data: { axis } }, - // The stepper initializes `value` to `min`, so `min` pins the rendered count. + // The box initializes `raw` to `String(min)`, so `min` pins the rendered count. min, max: 1000, source_id: 0, }, - }; + }); } // player 0 == local PLAYER_ID, and turn_decision_controller/active_player are the @@ -87,3 +89,292 @@ describe("PayAmountChoiceUI — LoopCollapse axis label", () => { ).toBeInTheDocument(); }); }); + +const BOX = "Choose amount to pay"; + +/** + * Seed exactly as `renderWithAxis` does — player 0 == the local seat, with + * `active_player`/`turn_decision_controller` on that seat — so + * `useCanActForWaitingState` is true and no query below passes VACUOUSLY. + */ +function renderPrompt(waitingFor: WaitingFor) { + const seeded = setGameStoreForTest({ + gameState: buildGameState({ + waiting_for: waitingFor, + active_player: 0, + turn_decision_controller: 0, + }), + waitingFor, + }); + return { ...seeded, ...render() }; +} + +function type(value: string) { + fireEvent.change(screen.getByLabelText(BOX), { target: { value } }); +} + +describe("PayAmountChoiceUI — sanitized amount entry", () => { + beforeEach(() => { + useGameStore.getState().reset(); + }); + + afterEach(() => { + cleanup(); + }); + + it("T1/reach: a valid entry enables commit and dispatches exactly what was typed", () => { + const { dispatch } = renderPrompt(loopCollapseWaitingFor("Tokens")); + + // Positive control for every negative below: the harness reaches a rendered, + // enabled, dispatch-firing control. + expect(screen.getByLabelText(BOX)).toBeInTheDocument(); + type("377"); + + const commit = screen.getByRole("button", { name: /create 377 tokens/i }); + expect(commit).not.toBeDisabled(); + fireEvent.click(commit); + expect(dispatch).toHaveBeenCalledWith({ + type: "SubmitPayAmount", + data: { amount: 377 }, + }); + }); + + it("T2/above-max: an entry over the engine max cannot be committed", () => { + const { dispatch } = renderPrompt(loopCollapseWaitingFor("Tokens")); + type("1001"); + + expect(screen.getByLabelText(BOX)).toHaveValue("1001"); + // Name pinned to the neutral verb: "Create 0 tokens" here would state an amount the player + // never typed, and `/create/i` would match it. + expect(screen.getByRole("button", { name: "Confirm" })).toBeDisabled(); + // DOMINATED by the assertion above — labelled, per this file's convention, so it is not + // mistaken for a probe of `handleCommit`'s null-guard. MEASURED: deleting + // `if (amount === null) return;` reds ONLY T6b, and adding a click here does not change + // that, because React does not dispatch `onClick` to a disabled `button` at all. That + // suppression is React-level, not environment-level (this suite runs happy-dom), so + // switching the test environment would not change the calculus. Enter (T6b) bypasses + // `disabled` and is the only route on which that guard is observable. + expect(dispatch).not.toHaveBeenCalled(); + }); + + it("T3/below-min: an entry under the engine min cannot be committed", () => { + // Provenance honesty: `min > 0` is NOT currently reachable in production — every + // PayAmountChoice mint site hardcodes `min: 0`. This is a prop-contract test on the + // wire type, not a live-state repro. + const { dispatch } = renderPrompt( + buildPayAmountChoiceWaitingFor({ + data: { + player: 0, + resource: { type: "Counters" }, + min: 5, + max: 1000, + source_id: 0, + }, + }), + ); + type("3"); + + expect(screen.getByRole("button", { name: "Confirm" })).toBeDisabled(); + expect( + screen.getByText("Enter a whole number between 5 and 1000"), + ).toBeInTheDocument(); + // DOMINATED — see T2. The discriminating row for the null-guard is T6b. + expect(dispatch).not.toHaveBeenCalled(); + }); + + // `min` is PINNED at 0 here: at min=5 the "+2" member becomes DOMINATED by the range + // conjunct and stops discriminating the digit guard. + it.each(["", " 7 ", "1.5", "1e3", "+2", "0x10"])( + "T4/non-numeric: %j never reaches the engine", + (raw) => { + const { dispatch } = renderPrompt(loopCollapseWaitingFor("Tokens")); + type(raw); + + expect(screen.getByRole("button", { name: "Confirm" })).toBeDisabled(); + // DOMINATED — see T2. The discriminating row for the null-guard is T6b. + expect(dispatch).not.toHaveBeenCalled(); + }, + ); + + it("T5/cleared: clearing a valid entry cannot submit the 0 the player never typed", () => { + const { dispatch } = renderPrompt(loopCollapseWaitingFor("Tokens")); + type("377"); + type(""); + + expect(screen.getByRole("button", { name: "Confirm" })).toBeDisabled(); + // The click that used to sit here was inert — React does not dispatch `onClick` to a + // disabled `button` — and implied coverage this row does not have. DOMINATED; T6b + // discriminates. See T2. + expect(dispatch).not.toHaveBeenCalled(); + }); + + // Reset is keyed on prompt IDENTITY, not just its window. Keyed on [min, max] alone the effect + // would not fire for a successor prompt with the same bounds, and the previous decision would + // carry into it. `min`/`max` are held CONSTANT here so `source_id` is the only changing dep — + // the one shape in which the identity dependency is observable. + it("T10/successor: a same-window prompt from a DIFFERENT source resets the entry", () => { + const promptFrom = (sourceId: number) => + buildPayAmountChoiceWaitingFor({ + data: { + player: 0, + resource: { type: "Counters" }, + min: 0, + max: 10, + source_id: sourceId, + }, + }); + + const { rerender } = renderPrompt(promptFrom(1)); + type("7"); + expect(screen.getByLabelText(BOX)).toHaveValue("7"); + + const successor = promptFrom(2); + setGameStoreForTest({ + gameState: buildGameState({ + waiting_for: successor, + active_player: 0, + turn_decision_controller: 0, + }), + waitingFor: successor, + }); + rerender(); + + expect(screen.getByLabelText(BOX)).toHaveValue("0"); + }); + + // The seat axis, which `source_id` alone does NOT cover. The reachable case is the LoopCollapse + // arm in `game/turns.rs` — the same prompt `loopCollapseWaitingFor` models here — which mints one + // prompt per controller in APNAP order with `min: 0`, `accumulated: 0`, `source_id: ObjectId(0)` + // and `pending_mana_ability: None` written as LITERALS. Only `max` and the collapse axis derive + // from state, so two controllers with equal counts on the same axis differ in `player` alone. + // (An earlier version cited an equal-life case on `effects/pay.rs` instead. That test does show + // consecutive prompts with a constant source and `player` 0 then 1 — but `max` and `accumulated` + // move there too, so `[min, max]` alone would already have fired and it proves nothing about + // `source_id` being insufficient. Same correction as in the component.) + it("T11/successor-seat: a same-source prompt for a DIFFERENT seat resets the entry", () => { + const promptFor = (player: number) => + buildPayAmountChoiceWaitingFor({ + data: { + player, + resource: { type: "Counters" }, + min: 0, + max: 10, + source_id: 9, + }, + }); + + // CR 723.5 ("while controlling another player, a player makes all choices and decisions the + // controlled player is allowed to make") + CR 723.3 ("a player who's being controlled during + // their turn is still the active player") — which together are why the local seat (0) stays + // `turn_decision_controller` while the prompt's semantic player moves to seat 1. That is what + // keeps `useCanActForWaitingState` (usePlayerId.ts:95) true, so ONE client answers both + // prompts in a row. Setting the controller to 1 as well hides the panel and makes this row + // vacuous — it fails that way, which is how the shape was pinned rather than guessed. + const seat = (waitingFor: WaitingFor, player: number) => { + setGameStoreForTest({ + gameState: buildGameState({ + waiting_for: waitingFor, + active_player: player, + turn_decision_controller: 0, + }), + waitingFor, + }); + }; + + const first = promptFor(0); + seat(first, 0); + const { rerender } = render(); + type("7"); + expect(screen.getByLabelText(BOX)).toHaveValue("7"); + + const second = promptFor(1); + seat(second, 1); + rerender(); + + expect(screen.getByLabelText(BOX)).toHaveValue("0"); + }); + + it("T6a/enter-valid: Enter submits a valid entry", () => { + const { dispatch } = renderPrompt(loopCollapseWaitingFor("Tokens")); + type("42"); + fireEvent.keyDown(screen.getByLabelText(BOX), { key: "Enter" }); + + expect(dispatch).toHaveBeenCalledWith({ + type: "SubmitPayAmount", + data: { amount: 42 }, + }); + }); + + it("T6b/enter-invalid: Enter does NOT coerce an out-of-range entry", () => { + const { dispatch } = renderPrompt(loopCollapseWaitingFor("Tokens")); + type("1001"); + fireEvent.keyDown(screen.getByLabelText(BOX), { key: "Enter" }); + + expect(dispatch).not.toHaveBeenCalled(); + }); + + it("T7/engine-bound: the window comes from the engine, not a client constant", () => { + // Hostile multi-authority fixture: max=7, not the LoopCollapse 1000. Runnable proof + // that no client-side ceiling exists. + const { dispatch } = renderPrompt( + buildPayAmountChoiceWaitingFor({ + data: { + player: 0, + resource: { type: "LoopCollapse", data: { axis: "Tokens" } }, + min: 0, + max: 7, + source_id: 0, + }, + }), + ); + + type("8"); + expect(screen.getByRole("button", { name: "Confirm" })).toBeDisabled(); + + type("7"); + const commit = screen.getByRole("button", { name: /create 7 tokens/i }); + expect(commit).not.toBeDisabled(); + fireEvent.click(commit); + expect(dispatch).toHaveBeenCalledWith({ + type: "SubmitPayAmount", + data: { amount: 7 }, + }); + }); + + it("T8/seat-gate: the prompt renders only for the seat the engine addressed", () => { + // Rendering the component directly bypasses GamePage's outer gate, so the component's + // own `!data || !canAct` guard is genuinely the expression under test. + const { container } = renderPrompt( + buildPayAmountChoiceWaitingFor({ + data: { + player: 1, + resource: { type: "LoopCollapse", data: { axis: "Tokens" } }, + min: 0, + max: 1000, + source_id: 0, + }, + }), + ); + expect(container).toBeEmptyDOMElement(); + + // PINNED POSITIVE, same test: the emptiness above is satisfiable by a constant + // `return null`, so the identical prompt on the LOCAL seat must render the box. + cleanup(); + renderPrompt(loopCollapseWaitingFor("Tokens")); + expect(screen.getByLabelText(BOX)).toBeInTheDocument(); + }); + + it("T9/zero: 0 is a legal amount and commits (the truthiness-bug class)", () => { + const { dispatch } = renderPrompt(loopCollapseWaitingFor("Tokens")); + + // `raw` is "0" at mount (min = 0), so this asserts the mounted state directly. + expect(screen.getByLabelText(BOX)).toHaveValue("0"); + const commit = screen.getByRole("button", { name: /create 0 tokens/i }); + expect(commit).not.toBeDisabled(); + fireEvent.click(commit); + expect(dispatch).toHaveBeenCalledWith({ + type: "SubmitPayAmount", + data: { amount: 0 }, + }); + }); +}); diff --git a/client/src/components/modal/DialogHost.tsx b/client/src/components/modal/DialogHost.tsx index adeb15b012..cf3664470e 100644 --- a/client/src/components/modal/DialogHost.tsx +++ b/client/src/components/modal/DialogHost.tsx @@ -176,9 +176,13 @@ export function DialogHost({ children }: { children: ReactNode }) { const isNarrow = useIsNarrowViewport(); // Only apply the peek slide transform while peeked. Framer-motion keeps a // residual `transform` (even at `{ x: 0, y: 0 }`) whenever `animate` is set, - // which breaks `` hit-testing in bottom-anchored panels - // such as ChooseXValueUI — the slider looks fine but ignores drags until - // something else reflows the tree (issue #2427). + // which breaks pointer hit-testing in bottom-anchored panels — the control + // looks fine but ignores input until something else reflows the tree + // (issue #2427). Originally hit `` in ChooseXValueUI; + // that slider no longer exists, and the live subjects are now the amount box + // and its ± steppers (AmountInput), which ChooseXValueUI's mount-integration + // test drives for exactly this reason. The guard is NOT obsolete just because + // the control it was written for is gone. const slideTransform = peeked ? isNarrow ? { x: 0, y: "calc(100vh - 64px)" } diff --git a/client/src/i18n/locales/de/game.json b/client/src/i18n/locales/de/game.json index aab92ec833..ba9858a5d3 100644 --- a/client/src/i18n/locales/de/game.json +++ b/client/src/i18n/locales/de/game.json @@ -570,13 +570,15 @@ "mana": { "free": "Gratis", "chooseXTitle": "Wähle einen Wert für X", - "chooseXAria": "X-Wert wählen", "chooseXInputAria": "X-Wert eingeben", "decreaseX": "X-Wert verringern", "increaseX": "X-Wert erhöhen", - "xEquals": "X = {{value}}", "minMax": "min {{min}} / max {{max}}", "maxOnly": "max {{max}}", + "amountOutOfRange": "Gib eine ganze Zahl zwischen {{min}} und {{max}} ein", + "decreaseAmount": "Menge verringern", + "increaseAmount": "Menge erhöhen", + "confirmAmount": "Bestätigen", "confirmX": "X = {{value}} bestätigen", "payMana": "Manakosten bezahlen", "convokeHint": "Tappe Kreaturen, um die Bezahlung zu unterstützen.", diff --git a/client/src/i18n/locales/en/game.json b/client/src/i18n/locales/en/game.json index c89f3f9f02..0ecc33951c 100644 --- a/client/src/i18n/locales/en/game.json +++ b/client/src/i18n/locales/en/game.json @@ -609,13 +609,15 @@ "mana": { "free": "Free", "chooseXTitle": "Choose a value for X", - "chooseXAria": "Choose X value", "chooseXInputAria": "Enter X value", "decreaseX": "Decrease X value", "increaseX": "Increase X value", - "xEquals": "X = {{value}}", "minMax": "min {{min}} / max {{max}}", "maxOnly": "max {{max}}", + "amountOutOfRange": "Enter a whole number between {{min}} and {{max}}", + "decreaseAmount": "Decrease amount", + "increaseAmount": "Increase amount", + "confirmAmount": "Confirm", "confirmX": "Confirm X = {{value}}", "payMana": "Pay Mana Cost", "convokeHint": "Tap creatures to help pay.", diff --git a/client/src/i18n/locales/es/game.json b/client/src/i18n/locales/es/game.json index 15eec7374d..471c4aace7 100644 --- a/client/src/i18n/locales/es/game.json +++ b/client/src/i18n/locales/es/game.json @@ -570,13 +570,15 @@ "mana": { "free": "Gratis", "chooseXTitle": "Elige un valor para X", - "chooseXAria": "Elegir el valor de X", "chooseXInputAria": "Introducir el valor de X", "decreaseX": "Disminuir el valor de X", "increaseX": "Aumentar el valor de X", - "xEquals": "X = {{value}}", "minMax": "mín. {{min}} / máx. {{max}}", "maxOnly": "máx. {{max}}", + "amountOutOfRange": "Introduce un número entero entre {{min}} y {{max}}", + "decreaseAmount": "Disminuir la cantidad", + "increaseAmount": "Aumentar la cantidad", + "confirmAmount": "Confirmar", "confirmX": "Confirmar X = {{value}}", "payMana": "Pagar el coste de maná", "convokeHint": "Gira criaturas para ayudar a pagar.", diff --git a/client/src/i18n/locales/fr/game.json b/client/src/i18n/locales/fr/game.json index 36f73a2fd0..6054e17c24 100644 --- a/client/src/i18n/locales/fr/game.json +++ b/client/src/i18n/locales/fr/game.json @@ -570,13 +570,15 @@ "mana": { "free": "Gratuit", "chooseXTitle": "Choisissez une valeur pour X", - "chooseXAria": "Choisir la valeur de X", "chooseXInputAria": "Saisir la valeur de X", "decreaseX": "Diminuer la valeur de X", "increaseX": "Augmenter la valeur de X", - "xEquals": "X = {{value}}", "minMax": "min {{min}} / max {{max}}", "maxOnly": "max {{max}}", + "amountOutOfRange": "Entrez un nombre entier entre {{min}} et {{max}}", + "decreaseAmount": "Diminuer la quantité", + "increaseAmount": "Augmenter la quantité", + "confirmAmount": "Confirmer", "confirmX": "Confirmer X = {{value}}", "payMana": "Payer le coût de mana", "convokeHint": "Engagez des créatures pour aider à payer.", diff --git a/client/src/i18n/locales/it/game.json b/client/src/i18n/locales/it/game.json index 1024e7baec..54435a0cc3 100644 --- a/client/src/i18n/locales/it/game.json +++ b/client/src/i18n/locales/it/game.json @@ -570,13 +570,15 @@ "mana": { "free": "Gratis", "chooseXTitle": "Scegli un valore per X", - "chooseXAria": "Scegli il valore di X", "chooseXInputAria": "Inserisci il valore di X", "decreaseX": "Diminuisci il valore di X", "increaseX": "Aumenta il valore di X", - "xEquals": "X = {{value}}", "minMax": "min {{min}} / max {{max}}", "maxOnly": "max {{max}}", + "amountOutOfRange": "Inserisci un numero intero tra {{min}} e {{max}}", + "decreaseAmount": "Diminuisci la quantità", + "increaseAmount": "Aumenta la quantità", + "confirmAmount": "Conferma", "confirmX": "Conferma X = {{value}}", "payMana": "Paga il costo di mana", "convokeHint": "TAPpa le creature per aiutare a pagare.", diff --git a/client/src/i18n/locales/pl/game.json b/client/src/i18n/locales/pl/game.json index c7e3139e9b..adbabae652 100644 --- a/client/src/i18n/locales/pl/game.json +++ b/client/src/i18n/locales/pl/game.json @@ -570,13 +570,15 @@ "mana": { "free": "Za darmo", "chooseXTitle": "Wybierz wartość dla X", - "chooseXAria": "Wybierz wartość X", "chooseXInputAria": "Wpisz wartość X", "decreaseX": "Zmniejsz wartość X", "increaseX": "Zwiększ wartość X", - "xEquals": "X = {{value}}", "minMax": "min {{min}} / maks {{max}}", "maxOnly": "maks {{max}}", + "amountOutOfRange": "Wpisz liczbę całkowitą od {{min}} do {{max}}", + "decreaseAmount": "Zmniejsz kwotę", + "increaseAmount": "Zwiększ kwotę", + "confirmAmount": "Potwierdź", "confirmX": "Potwierdź X = {{value}}", "payMana": "Zapłać koszt many", "convokeHint": "Obróć stwory, aby pomóc zapłacić.", diff --git a/client/src/i18n/locales/pt/game.json b/client/src/i18n/locales/pt/game.json index 3dfe39ebaf..10f18eb93e 100644 --- a/client/src/i18n/locales/pt/game.json +++ b/client/src/i18n/locales/pt/game.json @@ -570,13 +570,15 @@ "mana": { "free": "Grátis", "chooseXTitle": "Escolha um valor para X", - "chooseXAria": "Escolher valor de X", "chooseXInputAria": "Inserir valor de X", "decreaseX": "Diminuir valor de X", "increaseX": "Aumentar valor de X", - "xEquals": "X = {{value}}", "minMax": "mín {{min}} / máx {{max}}", "maxOnly": "máx {{max}}", + "amountOutOfRange": "Digite um número inteiro entre {{min}} e {{max}}", + "decreaseAmount": "Diminuir a quantia", + "increaseAmount": "Aumentar a quantia", + "confirmAmount": "Confirmar", "confirmX": "Confirmar X = {{value}}", "payMana": "Pagar Custo de Mana", "convokeHint": "Vire criaturas para ajudar a pagar.", diff --git a/client/src/test/factories/gameStateFactory.ts b/client/src/test/factories/gameStateFactory.ts index 88fec89e54..4d4d0f9164 100644 --- a/client/src/test/factories/gameStateFactory.ts +++ b/client/src/test/factories/gameStateFactory.ts @@ -29,6 +29,7 @@ type TriggerTargetSelectionWaitingFor = Extract< { type: "TriggerTargetSelection" } >; type ChooseXValueWaitingFor = Extract; +type PayAmountChoiceWaitingFor = Extract; type UntapChoiceWaitingFor = Extract; type AssistPaymentWaitingFor = Extract; type CastOfferWaitingFor = Extract; @@ -284,6 +285,26 @@ export const buildChooseXValueWaitingFor = ( return chooseXValueWaitingForFactory.withData(overrides.data ?? {}).build(); }; +export class PayAmountChoiceWaitingForFactory extends PlayerWaitingForFactory {} + +export const payAmountChoiceWaitingForFactory = + PayAmountChoiceWaitingForFactory.define((): PayAmountChoiceWaitingFor => ({ + type: "PayAmountChoice", + data: { + player: 0, + resource: { type: "Energy" }, + min: 0, + max: 0, + source_id: 0, + }, + })); + +export const buildPayAmountChoiceWaitingFor = ( + overrides: Partial = {}, +): PayAmountChoiceWaitingFor => { + return payAmountChoiceWaitingForFactory.withData(overrides.data ?? {}).build(); +}; + export class AssistPaymentWaitingForFactory extends WaitingForFactory { withCaster(caster: PlayerId) { return this.withData({ caster }); @@ -392,6 +413,10 @@ export class WaitingForVariantFactory extends Factory = {}) { + return this.variant(payAmountChoiceWaitingForFactory.withData(data).build()); + } + assistPayment(data: Partial = {}) { return this.variant(assistPaymentWaitingForFactory.withData(data).build()); } @@ -635,6 +660,10 @@ export class GameStateFactory extends Factory { return this.waitingFor(waitingForFactory.chooseXValue(data).build()); } + payAmountChoice(data: Partial = {}) { + return this.waitingFor(waitingForFactory.payAmountChoice(data).build()); + } + assistPayment(data: Partial = {}) { return this.waitingFor(waitingForFactory.assistPayment(data).build()); }