diff --git a/JoshHeaps.Net/Pages/Casino/TexasHoldem.cshtml b/JoshHeaps.Net/Pages/Casino/TexasHoldem.cshtml index 751f20b..0d8445f 100644 --- a/JoshHeaps.Net/Pages/Casino/TexasHoldem.cshtml +++ b/JoshHeaps.Net/Pages/Casino/TexasHoldem.cshtml @@ -101,24 +101,48 @@ "While you deal" below, with the paragraphs that explain them, which is where House take and Without live betting already were: the panel had the vocabulary for this and was only using it for money. *@ + @* Every field below takes digits and none of them is type="number". + That is deliberate, and it is the second half of a fix — the first + half is Render.setField, which stopped a re-render writing over the + text somebody was still typing. + + type="number" was costing more than it paid here. It silently drops + characters it dislikes with no feedback to the person or to assistive + tech; it reports a half-typed "0." as "0", which is what let the + decimal point be written away; it does not expose selectionStart, so + nothing can put a caret back where it was; it draws a spin button + into a 54px-wide blind field, where five digits already need 45 of + the 52 available; and it is the control GOV.UK dropped from their + number patterns after user testing, for the same reasons plus Dragon + and NVDA. + + inputmode is what actually summons the keypad on a phone, and it is + the better keypad: "decimal" carries the separator for the reader's + locale, which is the key a dealer needs to type 0.25 and the one + type="number" leaves them hunting for on iOS. + + min/max/step go with it. Nothing relied on them: the seat count is + clamped to 2..24 in main.js before it is used, and the money fields + are read through a finite-number check that a spinner never provided + anyway. *@

Blinds post themselves each hand. Seats without their own stack start here.

@@ -462,7 +486,7 @@
Total: $0
@@ -481,7 +505,7 @@ @@ -494,7 +518,7 @@ diff --git a/JoshHeaps.Net/wwwroot/css/casino/holdem.css b/JoshHeaps.Net/wwwroot/css/casino/holdem.css index 9708bf0..f8db923 100644 --- a/JoshHeaps.Net/wwwroot/css/casino/holdem.css +++ b/JoshHeaps.Net/wwwroot/css/casino/holdem.css @@ -132,7 +132,11 @@ .potLayerAmount, .handHistoryRow, .handHistoryPot, -input[type="number"] { +/* Was input[type="number"]. The numeric fields are type="text" with an inputmode now + — see the comment above the blinds in TexasHoldem.cshtml — and what earns a digit + column here is holding digits, which is exactly what inputmode says. */ +input[inputmode="decimal"], +input[inputmode="numeric"] { font-variant-numeric: tabular-nums; } diff --git a/JoshHeaps.Net/wwwroot/js/casino/holdem/main.js b/JoshHeaps.Net/wwwroot/js/casino/holdem/main.js index f32b5d8..b7db0eb 100644 --- a/JoshHeaps.Net/wwwroot/js/casino/holdem/main.js +++ b/JoshHeaps.Net/wwwroot/js/casino/holdem/main.js @@ -563,7 +563,7 @@ document.addEventListener("DOMContentLoaded", () => { }); document.getElementById("basePotInput").addEventListener("input", e => { - state.basePotAmount = e.target.value === "" ? null : Number(e.target.value); + state.basePotAmount = Money.readAmount(e.target.value); hooks.save(state); Render.renderAll(); }); @@ -828,7 +828,10 @@ document.addEventListener("DOMContentLoaded", () => { ["defaultStackInput", "defaultStack"], ].forEach(([id, key]) => { document.getElementById(id).addEventListener("input", e => { - state[key] = e.target.value === "" ? null : Number(e.target.value); + // Money.readAmount rather than Number(): these boxes are type="text" now, so + // a half-typed "0." or a stray letter reaches here instead of being silently + // eaten by the control, and NaN must not become the blind. + state[key] = Money.readAmount(e.target.value); hooks.save(state); Render.renderAll(); }); diff --git a/JoshHeaps.Net/wwwroot/js/casino/holdem/money.js b/JoshHeaps.Net/wwwroot/js/casino/holdem/money.js index 26da0b5..e412f96 100644 --- a/JoshHeaps.Net/wwwroot/js/casino/holdem/money.js +++ b/JoshHeaps.Net/wwwroot/js/casino/holdem/money.js @@ -17,6 +17,31 @@ const Money = { return Math.round((Number(amount) || 0) * 100); }, + // What a typed-in box is worth, or null if it isn't worth anything yet. + // + // The numeric fields are type="text" now, which is the whole point — the browser + // hands over exactly what was typed instead of silently dropping the characters it + // dislikes. The cost of that is that what was typed might not be a number, and + // `Number("abc")` is NaN, which would go into state, get saved, and come back as + // null on reload having taken the blind with it. + // + // So the reading is done in one place: empty is null, anything non-finite is null, + // and a real number comes back rounded to the cent the rest of the tool works in. + // Null rather than 0 because the fields it reads distinguish them — an unset + // starting stack falls back to the table default, a zero one does not. + // + // Deliberately not clamped to >= 0 here. A negative blind is a different mistake + // from a mistyped one, and the callers that care already clamp (the rake to 0..100, + // the seat count to 2..24); folding it in would silently turn a typed "-5" into + // something the dealer did not ask for. + readAmount(text) { + if (text == null) return null; + const trimmed = String(text).trim(); + if (trimmed === "") return null; + const value = Number(trimmed); + return Number.isFinite(value) ? this.round(value) : null; + }, + fromCents(cents) { return cents / 100; }, @@ -149,11 +174,20 @@ const Money = { // // Only number boxes. A name or a note is a thing you edit, and selecting all of // it on focus would be the same mistake pointing the other way. + // + // What makes a box a number box is now the inputmode rather than the type: nothing + // in the tool is type="number" any more (see the comment above the blinds in + // TexasHoldem.cshtml). Both inputmodes count. "numeric" is the seat count and the + // chip count — whole-number boxes that open pre-filled exactly like the money ones, + // and which this used to catch through their old type. Reading only "decimal" would + // have quietly dropped them, so it reads both. selectsOnFocus(field) { if (!field || field.tagName !== "INPUT") return false; const type = String(field.type || "").toLowerCase(); if (type === "number") return true; - return type === "text" && String(field.inputMode || "").toLowerCase() === "decimal"; + if (type !== "text") return false; + const mode = String(field.inputMode || "").toLowerCase(); + return mode === "decimal" || mode === "numeric"; }, // Would a wheel over this field change what it says? @@ -166,6 +200,12 @@ const Money = { // // A predicate rather than a listener so the rule can be tested without a browser, // the same way selectsOnFocus is. main.js does the blurring. + // + // As of the move off type="number" this answers false for every field in the tool, + // and that is the point rather than a bug: a text box with an inputmode is not a + // spinbutton, so a wheel over it cannot change it and there is nothing left to + // defend. Kept, tests and all, because it is the guard that makes that safe to rely + // on — reintroduce a type="number" anywhere and this starts firing again on its own. wheelWouldEdit(field, activeElement) { // The tag check is belt and braces and known to be so: nothing but an // carries type="number", so the line below already excludes every textarea, diff --git a/JoshHeaps.Net/wwwroot/js/casino/holdem/render.js b/JoshHeaps.Net/wwwroot/js/casino/holdem/render.js index 9f05947..e8c5b25 100644 --- a/JoshHeaps.Net/wwwroot/js/casino/holdem/render.js +++ b/JoshHeaps.Net/wwwroot/js/casino/holdem/render.js @@ -21,6 +21,7 @@ const Render = { this.lastHistorySignature = null; this.handTimeCache.clear(); this.watchSeatSheetHeight(); + this.watchFieldExits(); this.renderAll(); }, @@ -113,21 +114,79 @@ const Render = { }); }, + // ---- Writing a value into a field somebody might be standing in ---- + // + // A render says what the state is. It is not entitled to say what somebody is + // half-way through typing, and when it did, it ate the decimal point. + // + // The six setup fields are wired `input` -> save -> renderAll, and renderAll comes + // back round through renderSetupFields and assigns `.value`. Type "0.25" into the + // small blind and the fourth keystroke is not the one that breaks it; the "." is: + // + // "0" -> reads "0" -> state 0 -> written back "0" (no change, fine) + // "0." -> reads "0" -> state 0 -> written back "0" <- the "." is gone + // then -> caret is at 0, because assigning .value to a number input parks it + // at the start (number inputs do not expose selectionStart at all) + // "2" -> "20" + // "5" -> "250" + // + // A dealer setting $0.25/$0.50 blinds got a table playing 250/500, silently, with + // every pot for the rest of the night computed off it. Measured on all six: + // 0.25->250, 0.50->500, 12.50->5012, 2.5->52 (a 52% rake), 7.50->507, 0.75->750. + // + // So: never write into the control that has focus. It is the one place where the + // person knows better than the state does, and the state is a keystroke behind them + // anyway. Everything they type still lands in state through the `input` handler, so + // skipping the write-back costs nothing and is not a deferral of it. + // + // This guards every control rather than the six that were caught, because the next + // field wired to a re-render is the next one to eat a decimal point, and there is + // nothing about it at the call site to warn whoever adds it. + setField(el, value) { + if (!el) return; + if (typeof document !== "undefined" && el === document.activeElement) return; + const next = value == null ? "" : String(value); + // Assigning an identical value still moves the caret in some engines, so the + // compare is part of the guard rather than an optimisation. + if (el.value !== next) el.value = next; + }, + + setToggle(el, on) { + if (!el) return; + if (typeof document !== "undefined" && el === document.activeElement) return; + if (el.checked !== !!on) el.checked = !!on; + }, + + // The other half of the rule above: a field that was skipped because somebody was + // standing in it has to catch up when they step out, or it keeps showing what they + // typed after the state has moved on (a blind that got clamped, say). Bound once, + // on the document, because the controls it covers are rebuilt from templates. + watchFieldExits() { + if (this.fieldExitWatcher || typeof document === "undefined") return; + this.fieldExitWatcher = event => { + const el = event.target; + if (!el || !el.matches || !el.matches("input, select, textarea")) return; + // After the blur has settled, so activeElement is no longer this field. + setTimeout(() => this.renderSetupFields(), 0); + }; + document.addEventListener("focusout", this.fieldExitWatcher); + }, + renderSetupFields() { - document.getElementById("seatCountInput").value = this.state.seatCount; - document.getElementById("directionSelect").value = this.state.direction; - document.getElementById("basePotInput").value = this.state.basePotAmount ?? ""; - document.getElementById("dedicatedDealerToggle").checked = !!this.state.dedicatedDealer; - document.getElementById("trackBoardToggle").checked = !!this.state.trackBoard; - document.getElementById("keepScreenAwakeToggle").checked = this.state.keepScreenAwake !== false; - document.getElementById("smallBlindInput").value = this.state.smallBlind ?? ""; - document.getElementById("bigBlindInput").value = this.state.bigBlind ?? ""; - document.getElementById("defaultStackInput").value = this.state.defaultStack ?? ""; + this.setField(document.getElementById("seatCountInput"), this.state.seatCount); + this.setField(document.getElementById("directionSelect"), this.state.direction); + this.setField(document.getElementById("basePotInput"), this.state.basePotAmount ?? ""); + this.setToggle(document.getElementById("dedicatedDealerToggle"), this.state.dedicatedDealer); + this.setToggle(document.getElementById("trackBoardToggle"), this.state.trackBoard); + this.setToggle(document.getElementById("keepScreenAwakeToggle"), this.state.keepScreenAwake !== false); + this.setField(document.getElementById("smallBlindInput"), this.state.smallBlind ?? ""); + this.setField(document.getElementById("bigBlindInput"), this.state.bigBlind ?? ""); + this.setField(document.getElementById("defaultStackInput"), this.state.defaultStack ?? ""); const rake = this.state.rake || {}; - document.getElementById("rakePercentInput").value = rake.percent || ""; - document.getElementById("rakeCapInput").value = rake.cap ?? ""; - document.getElementById("rakeNoFlopInput").checked = rake.noFlopNoDrop !== false; + this.setField(document.getElementById("rakePercentInput"), rake.percent || ""); + this.setField(document.getElementById("rakeCapInput"), rake.cap ?? ""); + this.setToggle(document.getElementById("rakeNoFlopInput"), rake.noFlopNoDrop !== false); // What the house has actually taken tonight, said where the rake is set rather // than only in the bank check — a dealer who changes the percentage mid-session @@ -2571,7 +2630,11 @@ const Render = { const stackInput = form.querySelector(".stackInput"); stackInput.value = player.stack ?? ""; stackInput.addEventListener("input", () => { - player.stack = stackInput.value === "" ? null : Number(stackInput.value); + // Null for empty *and* for anything that isn't a number — the box is + // type="text" now, so a stray letter arrives here rather than being eaten + // by the control, and a seat holding NaN chips is worse than one holding + // none. See Money.readAmount. + player.stack = Money.readAmount(stackInput.value); this.commitSeatChange(form, player, index); }); @@ -2718,9 +2781,11 @@ const Render = { chip.color = colorInput.value; this.hooks.save(this.state); // The strip above is the same fact this field is editing, so it keeps up - // with it. Only the strip: renderChipLegend would write this control's - // own value back into it mid-keystroke, which is the same reason the - // list below calls its neighbours one at a time rather than renderAll. + // with it. Only the strip, still — but for cost now rather than for + // safety: setField refuses to write into a focused control, so the + // mid-keystroke clobber this was dodging can no longer happen from any + // direction. Naming the two renders that matter is simply cheaper than + // laying the felt out again on every keystroke. this.renderChipLegendStrip(); }); valueInput.addEventListener("input", () => { @@ -2735,8 +2800,8 @@ const Render = { // legend's own summary is on it: shut, the panel reads "5 colours · // $1–$500", and re-pricing the top chip is exactly the edit that // changes it. Called here for the same reason as the rest of the list: - // renderAll would write this field's own value back into it mid- - // keystroke. + // it is what this edit actually moves, and renderAll would lay the whole + // felt out again on every keystroke to find that out. // // Only that one. The bank check is not downstream of this, which is // worth saying because it looks as though it should be — a seat's chip diff --git a/tests/holdem/theDecimalPoint.test.js b/tests/holdem/theDecimalPoint.test.js new file mode 100644 index 0000000..eb94e06 --- /dev/null +++ b/tests/holdem/theDecimalPoint.test.js @@ -0,0 +1,272 @@ +// Typing 0.25 into a box and getting 0.25. +// +// The six setup fields are wired `input` -> save -> Render.renderAll(), and renderAll +// comes back round through renderSetupFields and assigns `.value` to the very box being +// typed in. The decimal point is where that turns into a wrong number rather than a +// wasted render, because a `type="number"` reports a half-typed "0." as "0" — so the +// write-back erased the point, and assigning `.value` to a number input parks the caret +// at position 0, so every digit after it landed at the front. +// +// Measured in Chrome against the page as it stood, keystroke by keystroke: +// +// small blind "0.25" 0 -> 0 -> 20 -> 250 a table playing 250/500 +// big blind "0.50" 0 -> 0 -> 50 -> 500 +// start stack "12.50" 1 -> 12 -> 12 -> 512 -> 5012 +// rake percent "2.5" 2 -> 2 -> 52 a 52% rake +// rake cap "7.50" 7 -> 7 -> 57 -> 507 +// base pot "0.75" 0 -> 0 -> 70 -> 750 +// +// Nothing warned: every one of those is a valid number, so no validation could have +// caught it, and the dealer's own table is the only place it shows. +// +// Two rules hold it shut, and they are separate on purpose. One stops a render writing +// over somebody's half-finished typing at all. The other takes the fields off +// `type="number"`, which is what made a half-finished number unreadable in the first +// place — and is what GOV.UK's design system concluded after user testing, for this and +// four other reasons. +const test = require("node:test"); +const { describe } = require("node:test"); +const assert = require("node:assert"); +const fs = require("fs"); +const path = require("path"); +const { Render, Money } = require("./load"); + +const MARKUP = fs.readFileSync( + path.join(__dirname, "..", "..", "JoshHeaps.Net", "Pages", "Casino", "TexasHoldem.cshtml"), "utf8"); + +// A stand-in for one form control, and a document that knows which one has focus. +const field = (value = "") => ({ value, checked: false }); +const withFocus = focused => { + global.document = { activeElement: focused || null }; +}; + +test.afterEach(() => { delete global.document; }); + +// --------------------------------------------------------------------------- + +describe("a render does not type for you", () => { + test("the box somebody is standing in is left exactly as they left it", () => { + // The whole fix in one assertion. "0." is not a number and state has read it as + // 0; writing that 0 back is what ate the point. + const box = field("0."); + withFocus(box); + + Render.setField(box, 0); + + assert.equal(box.value, "0.", + "the render wrote over a half-typed number, which is how 0.25 became 250"); + }); + + test("and the box nobody is in is brought up to date", () => { + const box = field("2"); + withFocus(null); + + Render.setField(box, 5); + + assert.equal(box.value, "5"); + }); + + test("a value that has not changed is not written at all", () => { + // Assigning an identical value still moves the caret in some engines, so this + // is part of the guard rather than a saving. + const box = field("5"); + let writes = 0; + Object.defineProperty(box, "value", { + get: () => "5", + set: () => { writes += 1; }, + }); + withFocus(null); + + Render.setField(box, 5); + + assert.equal(writes, 0, "an unchanged value was assigned anyway"); + }); + + test("null and undefined clear the box rather than spelling themselves out", () => { + withFocus(null); + const a = field("9"); + const b = field("9"); + + Render.setField(a, null); + Render.setField(b, undefined); + + assert.equal(a.value, ""); + assert.equal(b.value, "", "an unset blind would have read the word 'undefined'"); + }); + + test("and a box that isn't there is not an error", () => { + withFocus(null); + assert.doesNotThrow(() => Render.setField(null, 5)); + assert.doesNotThrow(() => Render.setToggle(null, true)); + }); +}); + +describe("the same rule for a switch", () => { + test("a checkbox somebody is on is not flipped under them", () => { + const box = field(); + box.checked = true; + withFocus(box); + + Render.setToggle(box, false); + + assert.equal(box.checked, true, + "a render flipped a switch the dealer had their finger on"); + }); + + test("and one nobody is on keeps up with the state", () => { + const box = field(); + box.checked = true; + withFocus(null); + + Render.setToggle(box, false); + + assert.equal(box.checked, false); + }); +}); + +describe("what a typed-in box is worth", () => { + // The fields are type="text" now, so what arrives is what was typed — including + // things that are not numbers. Number("abc") is NaN, and NaN must not become a blind. + test("a number is worth that number", () => { + assert.equal(Money.readAmount("0.25"), 0.25); + assert.equal(Money.readAmount("12.50"), 12.5); + assert.equal(Money.readAmount("200"), 200); + }); + + test("nothing typed yet is not zero", () => { + // Null and 0 mean different things to the fields that use this: an unset + // starting stack falls back to the table default, a zero one does not. + assert.equal(Money.readAmount(""), null); + assert.equal(Money.readAmount(" "), null); + assert.equal(Money.readAmount(null), null); + assert.equal(Money.readAmount(undefined), null); + }); + + test("and neither is something that is not a number", () => { + ["abc", "1.2.3", "$5", "--", "1,5"].forEach(text => { + assert.equal(Money.readAmount(text), null, + `"${text}" came back as something other than null`); + }); + }); + + test("infinity is not a stack", () => { + assert.equal(Money.readAmount("Infinity"), null); + assert.equal(Money.readAmount("1e999"), null); + }); + + test("a half-typed number is read as what it is so far", () => { + // "0." arrives here now instead of being flattened to "0" by the control. It is + // worth 0 at this instant, and the next keystroke will say otherwise — what + // matters is that the box still reads "0." when it does. + assert.equal(Money.readAmount("0."), 0); + assert.equal(Money.readAmount(".5"), 0.5); + }); + + test("and it comes back rounded to the cent the tool works in", () => { + assert.equal(Money.readAmount("0.005"), 0.01); + assert.equal(Money.readAmount("1.004"), 1); + }); + + test("a negative is left alone rather than quietly clamped", () => { + // A typed "-5" is a different mistake from a mistyped one, and the callers that + // care clamp for themselves. Folding it in here would change what the dealer + // asked for without saying so. + assert.equal(Money.readAmount("-5"), -5); + }); +}); + +describe("the boxes themselves", () => { + const inputs = MARKUP.match(/]*>/g) || []; + const attr = (tag, name) => { + const m = tag.match(new RegExp(`${name}="([^"]*)"`)); + return m ? m[1] : null; + }; + const named = tag => attr(tag, "id") || attr(tag, "class") || tag.slice(0, 40); + + test("none of them is a type=number any more", () => { + const spinners = inputs.filter(t => attr(t, "type") === "number"); + assert.deepEqual(spinners.map(named), [], + "a type=number is back: it reports a half-typed '0.' as '0', which is the " + + "read that let a render write the decimal point away"); + }); + + test("every box that takes digits says which keypad it wants", () => { + // The digits fields are the ones this round converted. Identified by name rather + // than by type, because their type is exactly what no longer distinguishes them. + const DIGITS = [ + "seatCountInput", "smallBlindInput", "bigBlindInput", "defaultStackInput", + "rakePercentInput", "rakeCapInput", "basePotInput", + "stackInput", "amountOverrideInput", "chipValueInput", "chipCountInput", + "addChipsInput", + ]; + DIGITS.forEach(name => { + const tag = inputs.find(t => named(t) === name); + assert.ok(tag, `${name} is gone from the markup`); + const mode = attr(tag, "inputmode"); + assert.ok(mode === "decimal" || mode === "numeric", + `${name} has inputmode="${mode}" — without one a phone offers a ` + + `full keyboard for a number, which is the slowest thing on the page`); + }); + }); + + test("the money ones ask for the separator, the counting ones do not", () => { + // "decimal" carries the reader's decimal separator; "numeric" is bare digits. + // A blind needs the point. A seat count would only be a way to type a wrong one. + const modeOf = name => attr(inputs.find(t => named(t) === name), "inputmode"); + ["smallBlindInput", "bigBlindInput", "defaultStackInput", "rakeCapInput", + "basePotInput", "stackInput", "chipValueInput"].forEach(name => { + assert.equal(modeOf(name), "decimal", `${name} cannot be given cents`); + }); + ["seatCountInput", "chipCountInput"].forEach(name => { + assert.equal(modeOf(name), "numeric", `${name} counts things, not money`); + }); + }); + + test("and none of them invites the browser to fill it in", () => { + // A saved address offering itself as a small blind is the browser being helpful + // about the one field on the page it cannot know anything about. + ["smallBlindInput", "bigBlindInput", "defaultStackInput", "seatCountInput", + "rakePercentInput", "rakeCapInput", "basePotInput"].forEach(name => { + const tag = inputs.find(t => named(t) === name); + assert.equal(attr(tag, "autocomplete"), "off", `${name} may be autofilled`); + }); + }); +}); + +describe("what still counts as a number box", () => { + // Money.selectsOnFocus decides whether tapping a pre-filled amount box selects what + // is in it. It used to recognise those boxes by type="number"; there are none left. + test("a text box with a decimal keypad is one", () => { + assert.equal(Money.selectsOnFocus( + { tagName: "INPUT", type: "text", inputMode: "decimal" }), true); + }); + + test("and so is one with a numeric keypad", () => { + // The seat count and the chip count. Both open pre-filled, both were caught by + // the old type check, and reading only "decimal" would have dropped them. + assert.equal(Money.selectsOnFocus( + { tagName: "INPUT", type: "text", inputMode: "numeric" }), true); + }); + + test("a plain text box is not", () => { + assert.equal(Money.selectsOnFocus( + { tagName: "INPUT", type: "text", inputMode: "" }), false, + "a player's name would be wiped by the first keystroke after a tap"); + }); +}); + +describe("a wheel has nothing left to spin", () => { + test("no field in the tool is a spinbutton any more", () => { + // wheelWouldEdit answers false for every field now, which is the point rather + // than a regression: a text box is not a spinbutton, so a wheel over it cannot + // change it. The guard stays because it is what makes that safe to rely on. + assert.equal(Money.wheelWouldEdit( + { tagName: "INPUT", type: "text", inputMode: "decimal" }, + { tagName: "INPUT", type: "text", inputMode: "decimal" }), false); + }); + + test("but it would still catch one if somebody added it back", () => { + const spinner = { tagName: "INPUT", type: "number" }; + assert.equal(Money.wheelWouldEdit(spinner, spinner), true); + }); +});