diff --git a/JoshHeaps.Net/wwwroot/css/casino/quicksplit.css b/JoshHeaps.Net/wwwroot/css/casino/quicksplit.css index a41684d..35927d1 100644 --- a/JoshHeaps.Net/wwwroot/css/casino/quicksplit.css +++ b/JoshHeaps.Net/wwwroot/css/casino/quicksplit.css @@ -526,20 +526,30 @@ button, font-weight: 600; } +/* As wide as the three things in it, and no wider. + This sits in the same wrapping flex row as the chips, where it has never once fitted + on the first line of a phone. `flex: 1 1 auto` then handed it the whole of the second + line: measured on an iPhone, a 358px bar with 252px of nothing in the middle and the + − and + pushed 304px apart — the two ends of a control a thumb is meant to work with + one hand. 136px now, its two ends 121px apart. */ .waysStepper { display: flex; align-items: center; gap: 0.35rem; - flex: 1 1 auto; - min-width: 150px; + flex: 0 0 auto; background: #191919; border: 1px solid #333; border-radius: 11px; padding: 0.2rem 0.4rem; } +/* 44 both ways, not 40x44. It had been 40px wide since the steppers were written, and + the sweep never said so because the only two steppers on the site are on this screen + and this screen was only ever audited in its all-in mode, where neither is on display + until a side pot has more than two contenders. The first sweep that opened the even + mode reported `hittable 39x43` on both ends, on every touch viewport. */ .stepBtn { - width: 40px; + width: 44px; height: 44px; border: none; border-radius: 8px; @@ -551,17 +561,26 @@ button, cursor: pointer; } +/* The floor and the ceiling look like the floor and the ceiling. */ +.stepBtn:disabled { + opacity: 0.32; + cursor: default; +} + @media (hover: hover) { - .stepBtn:hover { + .stepBtn:not(:disabled):hover { background: #333; } } +/* The count itself, now that no chip beside it is also holding one — so it is painted + the way a selected chip is painted, because that is what it is standing in for. */ .waysReadout { - flex: 1; + min-width: 2.5ch; text-align: center; - color: #999; - font-size: 0.8rem; + color: #f0d9a8; + font-size: 1rem; + font-weight: 600; font-variant-numeric: tabular-nums; } diff --git a/JoshHeaps.Net/wwwroot/js/casino/holdem/quickSplit.js b/JoshHeaps.Net/wwwroot/js/casino/holdem/quickSplit.js index 73a4c03..95255b7 100644 --- a/JoshHeaps.Net/wwwroot/js/casino/holdem/quickSplit.js +++ b/JoshHeaps.Net/wwwroot/js/casino/holdem/quickSplit.js @@ -21,6 +21,27 @@ const QuickSplit = { mode: "even", entries: [], + // The first count the preset chips don't cover. Below it the chips are the control; + // at it and above, one chip opens a stepper and the stepper is the control. The + // number is never stated by two of them at once — it used to be stated by three, + // with chip "2" lit gold, "2 ways" in a bar under it, and "Each of 2 players gets" + // in the panel above. + overflowFrom() { + return this.WAY_CHOICES[this.WAY_CHOICES.length - 1] + 1; + }, + + clampWays(count) { + return Math.max(this.WAY_CHOICES[0], Math.min(this.MAX_WAYS, count)); + }, + + // A pot is chopped between the players who can win it, and there is no such thing + // as chopping it between more of them than there are. This used to read `Math.max`, + // which let a side pot only two players were eligible for be split ten ways at $12 + // each — an answer to a question nobody at the table could have asked. + chopCeiling(contenderCount) { + return Math.max(2, Math.min(contenderCount, this.MAX_WAYS)); + }, + mount() { this.potInput = document.getElementById("potInput"); this.clearPotBtn = document.getElementById("clearPotBtn"); @@ -86,10 +107,11 @@ const QuickSplit = { renderWayChoices() { const row = document.getElementById("waysChoices"); row.innerHTML = ""; + const beyond = this.ways >= this.overflowFrom(); this.WAY_CHOICES.forEach(count => { row.appendChild( - this.makeChoiceButton(String(count), this.ways === count, () => { + this.makeChoiceButton(String(count), !beyond && this.ways === count, () => { this.ways = count; this.renderWayChoices(); this.update(); @@ -97,34 +119,65 @@ const QuickSplit = { ); }); - // Beyond a five-way chop it's rare enough to hide behind a stepper rather than - // spend a row of buttons on it. - const more = document.createElement("div"); - more.className = "waysStepper"; + // Beyond a five-way chop it's rare enough not to be worth a chip each. One more + // chip is the door to the rest; it reads "6+" whatever the count behind it is, + // because the stepper is where the count is written. Tapping it while it is + // already open does nothing — it is the fifth option of a group, not a toggle. + const door = this.makeChoiceButton(`${this.overflowFrom()}+`, beyond, () => { + if (beyond) return; + this.ways = this.overflowFrom(); + this.renderWayChoices(); + this.update(); + }); + door.setAttribute("aria-label", `${this.overflowFrom()} or more`); + row.appendChild(door); - const minus = this.makeStepButton("−", () => this.stepWays(-1)); - const plus = this.makeStepButton("+", () => this.stepWays(1)); - const readout = document.createElement("span"); - readout.className = "waysReadout"; - readout.textContent = `${this.ways} ways`; - - more.appendChild(minus); - more.appendChild(readout); - more.appendChild(plus); - row.appendChild(more); + if (beyond) row.appendChild(this.buildWaysStepper()); }, - makeStepButton(label, onPick) { + // Six to twelve, which is exactly what the door promised. The floor is 6 rather + // than 2 because the chips below it are one tap away and already on screen — and + // because stepping down out of the stepper would pull the stepper out from under + // the finger that was pressing it. + buildWaysStepper() { + const stepper = document.createElement("div"); + stepper.className = "waysStepper"; + stepper.setAttribute("role", "group"); + stepper.setAttribute("aria-label", "How many ways"); + + stepper.appendChild(this.makeStepButton("−", () => this.stepWays(-1), { + label: "One fewer way", + disabled: this.ways <= this.overflowFrom(), + })); + + const readout = document.createElement("span"); + readout.className = "waysReadout"; + readout.textContent = String(this.ways); + stepper.appendChild(readout); + + stepper.appendChild(this.makeStepButton("+", () => this.stepWays(1), { + label: "One more way", + disabled: this.ways >= this.MAX_WAYS, + })); + + return stepper; + }, + + makeStepButton(label, onPick, options) { const btn = document.createElement("button"); btn.type = "button"; btn.className = "stepBtn"; btn.textContent = label; + if (options && options.label) btn.setAttribute("aria-label", options.label); + // A step that can't happen says so. Both ends of both steppers on this screen + // used to look exactly like the middle and swallow the tap in silence. + btn.disabled = !!(options && options.disabled); btn.addEventListener("click", onPick); return btn; }, stepWays(delta) { - this.ways = Math.max(2, Math.min(this.MAX_WAYS, this.ways + delta)); + this.ways = this.clampWays(this.ways + delta); this.renderWayChoices(); this.update(); }, @@ -205,7 +258,8 @@ const QuickSplit = { // The per-pot ways live on the row rather than in `this.ways`, so opening a chop on // the main pot doesn't change what the side pot below it is saying. addChopControl(row, amount, contenderCount) { - let ways = contenderCount; + const ceiling = this.chopCeiling(contenderCount); + let ways = ceiling; let open = false; const toggle = document.createElement("button"); @@ -225,19 +279,24 @@ const QuickSplit = { const result = SplitCalculator.split(amount, ways, this.chip); detail.innerHTML = ""; - const stepper = document.createElement("div"); - stepper.className = "unevenChopWays"; - stepper.appendChild(this.makeStepButton("−", () => { - if (ways > 2) { ways -= 1; draw(); } - })); - const label = document.createElement("span"); - label.className = "unevenChopWaysLabel"; - label.textContent = `${ways} ways`; - stepper.appendChild(label); - stepper.appendChild(this.makeStepButton("+", () => { - if (ways < Math.max(contenderCount, this.MAX_WAYS)) { ways += 1; draw(); } - })); - detail.appendChild(stepper); + // Two eligible players is two ways and nothing else, so the stepper is left + // out rather than shown with both ends dead — which is the common shape, a + // side pot usually coming down to the two who could still call. + if (ceiling > 2) { + const stepper = document.createElement("div"); + stepper.className = "unevenChopWays"; + stepper.appendChild(this.makeStepButton("−", () => { + if (ways > 2) { ways -= 1; draw(); } + }, { label: "One fewer way", disabled: ways <= 2 })); + const label = document.createElement("span"); + label.className = "unevenChopWaysLabel"; + label.textContent = `${ways} ways`; + stepper.appendChild(label); + stepper.appendChild(this.makeStepButton("+", () => { + if (ways < ceiling) { ways += 1; draw(); } + }, { label: "One more way", disabled: ways >= ceiling })); + detail.appendChild(stepper); + } const each = document.createElement("p"); each.className = "unevenChopEach"; @@ -368,4 +427,10 @@ const QuickSplit = { }, }; -document.addEventListener("DOMContentLoaded", () => QuickSplit.mount()); +if (typeof document !== "undefined") { + document.addEventListener("DOMContentLoaded", () => QuickSplit.mount()); +} + +if (typeof module !== "undefined" && module.exports) { + module.exports = QuickSplit; +} diff --git a/tests/holdem/load.js b/tests/holdem/load.js index f11cea5..cb543fe 100644 --- a/tests/holdem/load.js +++ b/tests/holdem/load.js @@ -48,6 +48,11 @@ const WakeLock = load("wakeLock"); // so little of a document that a stand-in can answer it. See theKeyboardInASheet. const Modal = load("modal"); +// Quick Split draws itself entirely from the DOM, but the arithmetic that decides what +// its two steppers are allowed to reach — and how many ways a pot may be chopped — is +// plain and answers here. See theWaysControl. +const QuickSplit = load("quickSplit"); + // The seat menu's list — which actions a seat is offered and which group each one // lands in — is decided before anything is drawn, so actionsFor() answers here. Only // render() and open() touch the DOM. @@ -111,6 +116,7 @@ module.exports = { WakeLock, SeatMenu, Modal, + QuickSplit, makeTable, chipsOnTable, contributions, diff --git a/tests/holdem/runUiAudit.js b/tests/holdem/runUiAudit.js index 1c7b062..6b1b22a 100644 --- a/tests/holdem/runUiAudit.js +++ b/tests/holdem/runUiAudit.js @@ -323,15 +323,33 @@ async function main() { const label = `${rawLabel} · ${hand}`; await page.setViewportSize({ width, height }); await page.goto(`${BASE}/Casino/TexasHoldem/QuickSplit`); - await page.evaluate(() => { - const button = [...document.querySelectorAll("button")].find(b => /all.?in/i.test(b.textContent)); - if (button) button.click(); - }); await page.addScriptTag({ path: AUDIT }); - checks++; - const result = await page.evaluate(() => LayoutAudit.runAll({ occlusionSelector: "main button, main a, main input" })); - if (!result.clean) failures.push({ where: `quick split · ${label} ${width}x${height}`, offenders: result.failed.flatMap(f => f.offenders) }); - result.warnings.forEach(w => warnings.push(`quick split · ${label}: ${w}`)); + + // Both halves of the screen. The sweep used to switch straight to the all-in + // mode and audit only that, so the mode the screen opens in — the common one, + // a pot and a number of ways — was never looked at once, which is how a bar + // with 252px of nothing in it lasted this long. The ways stepper only exists + // past five, so the even pass opens it before measuring. + const modes = [ + ["even", () => { + const beyond = [...document.querySelectorAll("#waysChoices .choiceChip")] + .find(b => /\+$/.test(b.textContent.trim())); + if (beyond) beyond.click(); + }], + ["all-in", () => { + const button = [...document.querySelectorAll("button")].find(b => /all.?in/i.test(b.textContent)); + if (button) button.click(); + }], + ]; + + for (const [mode, arrange] of modes) { + await page.evaluate(arrange); + await page.waitForTimeout(120); + checks++; + const result = await page.evaluate(() => LayoutAudit.runAll({ occlusionSelector: "main button, main a, main input" })); + if (!result.clean) failures.push({ where: `quick split · ${mode} · ${label} ${width}x${height}`, offenders: result.failed.flatMap(f => f.offenders) }); + result.warnings.forEach(w => warnings.push(`quick split · ${mode} · ${label}: ${w}`)); + } } await context.close(); diff --git a/tests/holdem/theWaysControl.test.js b/tests/holdem/theWaysControl.test.js new file mode 100644 index 0000000..b684b36 --- /dev/null +++ b/tests/holdem/theWaysControl.test.js @@ -0,0 +1,224 @@ +// One number, one control, and a ceiling that matches the table. +// +// "Split between" on Quick Split was two controls for one value sitting on top of each +// other: a row of chips 2 3 4 5, and under it a bar reading "2 ways" with a − and a +. +// Measured on an iPhone: 134px with its label, against 74px for "Smallest chip in play" +// right below it, which offers five choices in one row. And the number was written down +// three times at once — chip "2" lit gold, "2 ways" in the bar, and "Each of 2 players +// gets" in the result panel above. Two of those three were controls. +// +// The bar was never meant to be a bar. `.waysStepper` asked for `min-width: 150px` in a +// wrapping flex row, so on any phone it wrapped to a line of its own, and `flex: 1 1 +// auto` then gave it all 358px of that line: 252px of empty readout with the − and the +// + pushed 304px apart. One-handed, on a screen 390px wide, they cannot both be reached. +// +// So the chips get a fifth chip. It reads "6+" whatever is behind it, because behind it +// is where the count is written, and the stepper only exists once you are through the +// door. Under six ways — every chop a home game will ever run — the field is one row. +// +// Two more things went with it. Neither stepper on this screen ever disabled an end, so +// − at the floor and + at the ceiling looked live and swallowed the tap; NN/G's stepper +// guidelines ask for exactly the opposite. And the per-pot chop capped its + with +// `Math.max(contenderCount, MAX_WAYS)`, which is always 12: a side pot that only P1 and +// P2 were eligible for would happily be chopped ten ways at $12 each. +const test = require("node:test"); +const { describe } = require("node:test"); +const assert = require("node:assert"); +const fs = require("fs"); +const path = require("path"); +const { QuickSplit } = require("./load"); + +const ROOT = path.join(__dirname, "..", "..", "JoshHeaps.Net"); +const CSS = fs.readFileSync(path.join(ROOT, "wwwroot", "css", "casino", "quicksplit.css"), "utf8"); +const SRC = fs.readFileSync(path.join(ROOT, "wwwroot", "js", "casino", "holdem", "quickSplit.js"), "utf8"); + +const ruleFor = selector => { + const at = CSS.indexOf(`\n${selector} {`); + assert.ok(at >= 0, `there is no \`${selector}\` rule`); + return CSS.slice(at, CSS.indexOf("}", at)); +}; + +// --------------------------------------------------------------------------- + +describe("where the chips stop and the stepper starts", () => { + test("the door is the first count the chips don't cover", () => { + assert.deepEqual(QuickSplit.WAY_CHOICES, [2, 3, 4, 5]); + assert.equal(QuickSplit.overflowFrom(), 6); + }); + + // Derived rather than written down twice: adding a chip has to move the door. + test("and it moves if a chip is added or taken away", () => { + const was = QuickSplit.WAY_CHOICES; + try { + QuickSplit.WAY_CHOICES = [2, 3]; + assert.equal(QuickSplit.overflowFrom(), 4); + } finally { + QuickSplit.WAY_CHOICES = was; + } + }); + + test("the stepper's own range is what the door promised", () => { + assert.equal(QuickSplit.clampWays(QuickSplit.overflowFrom() - 1), 5); + assert.equal(QuickSplit.clampWays(QuickSplit.MAX_WAYS + 4), QuickSplit.MAX_WAYS); + assert.equal(QuickSplit.clampWays(0), QuickSplit.WAY_CHOICES[0]); + assert.equal(QuickSplit.clampWays(-3), 2); + }); + + test("the door is drawn once, from the same number", () => { + const at = SRC.indexOf("renderWayChoices() {"); + const body = SRC.slice(at, SRC.indexOf("\n },", at)); + assert.match(body, /`\$\{this\.overflowFrom\(\)\}\+`/, + "the chip's label is typed in rather than taken from where the chips stop"); + assert.match(body, /aria-label/, "\"6+\" is not a name a screen reader can read out"); + }); + + // The chips and the stepper are one group: when the count is past the chips, no + // chip is lit, and the door is. + test("nothing is lit in two places at once", () => { + const at = SRC.indexOf("renderWayChoices() {"); + const body = SRC.slice(at, SRC.indexOf("\n },", at)); + assert.match(body, /const beyond = this\.ways >= this\.overflowFrom\(\)/); + assert.match(body, /!beyond && this\.ways === count/, + "a preset chip can still light up while the stepper holds a different number"); + assert.match(body, /if \(beyond\) row\.appendChild\(this\.buildWaysStepper\(\)\)/, + "the stepper is drawn even when the chips are the control"); + }); + + test("and the count is written in one place, without a unit the label already gave", () => { + const at = SRC.indexOf("buildWaysStepper() {"); + assert.ok(at >= 0); + const body = SRC.slice(at, SRC.indexOf("\n },", at)); + assert.match(body, /readout\.textContent = String\(this\.ways\)/); + assert.ok(!/\$\{this\.ways\} ways/.test(body), "the stepper is spelling the count out again"); + }); +}); + +describe("both ends of both steppers", () => { + test("a step that cannot happen says so", () => { + const at = SRC.indexOf("makeStepButton(label, onPick, options) {"); + assert.ok(at >= 0, "the step buttons still take no disabled state"); + assert.match(SRC.slice(at, SRC.indexOf("\n },", at)), /btn\.disabled = /); + }); + + test("the ways stepper disables its floor and its ceiling", () => { + const at = SRC.indexOf("buildWaysStepper() {"); + const body = SRC.slice(at, SRC.indexOf("\n },", at)); + assert.match(body, /disabled: this\.ways <= this\.overflowFrom\(\)/); + assert.match(body, /disabled: this\.ways >= this\.MAX_WAYS/); + }); + + test("so does the per-pot chop", () => { + const at = SRC.indexOf("addChopControl(row, amount, contenderCount) {"); + const body = SRC.slice(at, SRC.indexOf("\n updateUneven()", at)); + assert.match(body, /disabled: ways <= 2/); + assert.match(body, /disabled: ways >= ceiling/); + }); + + // 40x44 until the sweep first opened the even mode and read back `hittable 39x43` + // on every touch viewport there is. + test("and both ends are a fingertip wide, not just a fingertip tall", () => { + const rule = ruleFor(".stepBtn"); + const px = axis => Number((rule.match(new RegExp(`${axis}:\\s*(\\d+)px`)) || [])[1]); + assert.ok(px("width") >= 44, `a step button is ${px("width")}px wide`); + assert.ok(px("height") >= 44, `a step button is ${px("height")}px tall`); + }); + + // Disabled has to look disabled, or it is the same silent tap with extra steps. + test("and a disabled end is painted as one", () => { + assert.match(ruleFor(".stepBtn:disabled"), /opacity:\s*0\.\d+/); + assert.match(CSS, /\.stepBtn:not\(:disabled\):hover/, + "a dead button still lights up under the mouse"); + }); +}); + +describe("how many ways a pot can go", () => { + // The whole of the bug: `Math.max` where `Math.min` was meant. + test("never more ways than there are players who can win it", () => { + assert.equal(QuickSplit.chopCeiling(2), 2); + assert.equal(QuickSplit.chopCeiling(3), 3); + assert.equal(QuickSplit.chopCeiling(9), 9); + }); + + test("and never past the screen's own ceiling", () => { + assert.equal(QuickSplit.chopCeiling(20), QuickSplit.MAX_WAYS); + }); + + // A pot is only offered a chop at two contenders or more, but the floor is stated + // here too so that a caller that gets it wrong can't produce a one-way split. + test("a chop is at least two ways", () => { + assert.equal(QuickSplit.chopCeiling(1), 2); + assert.equal(QuickSplit.chopCeiling(0), 2); + }); + + test("the chop's + is held to it", () => { + const at = SRC.indexOf("addChopControl(row, amount, contenderCount) {"); + const body = SRC.slice(at, SRC.indexOf("\n updateUneven()", at)); + assert.match(body, /const ceiling = this\.chopCeiling\(contenderCount\)/); + assert.match(body, /if \(ways < ceiling\)/); + assert.ok(!/Math\.max\(contenderCount/.test(body), "the old ceiling is still there"); + }); + + // Two eligible players is two ways and nothing else. A stepper with both ends dead + // is furniture. + test("a heads-up pot is offered no stepper at all", () => { + const at = SRC.indexOf("addChopControl(row, amount, contenderCount) {"); + const body = SRC.slice(at, SRC.indexOf("\n updateUneven()", at)); + assert.match(body, /if \(ceiling > 2\) \{/); + assert.ok(body.indexOf("if (ceiling > 2) {") < body.indexOf('className = "unevenChopWays"')); + }); +}); + +describe("the bar that was never meant to be a bar", () => { + test("the stepper is as wide as what is in it", () => { + const rule = ruleFor(".waysStepper"); + assert.match(rule, /flex:\s*0 0 auto/); + assert.ok(!/min-width/.test(rule), + "a min-width in a wrapping row is what handed it the whole second line"); + }); + + test("and the readout no longer pushes the two buttons apart", () => { + const rule = ruleFor(".waysReadout"); + assert.ok(!/flex:\s*1(\s|;)/.test(rule)); + assert.match(rule, /min-width:\s*[\d.]+ch/); + assert.match(rule, /tabular-nums/); + }); + + // It is standing in for a selected chip, so it is painted like one. + test("the count is painted like the chip it replaces", () => { + const chip = ruleFor(".choiceChip.selected"); + // Not `border-color`, which comes first in the rule and is a different gold. + const colour = (chip.match(/(?:^|[^-\w])color:\s*(#[0-9a-f]{3,6})/i) || [])[1]; + assert.ok(colour, "the selected chip has no colour to match"); + assert.match(ruleFor(".waysReadout"), new RegExp(`color:\\s*${colour}`, "i")); + }); + + // The measurement that found it, kept where the next round will read it. + test("what it measured is written beside the rule", () => { + const at = CSS.indexOf("/* As wide as the three things in it"); + assert.ok(at >= 0, "the rule carries no note saying why"); + const note = CSS.slice(at, CSS.indexOf("*/", at)); + assert.match(note, /304px/); + assert.match(note, /252px/); + }); +}); + +describe("what the sweep looks at", () => { + const DRIVER = fs.readFileSync(path.join(__dirname, "runUiAudit.js"), "utf8"); + + // It went straight to the all-in mode, so the mode the screen opens in was audited + // zero times in fourteen rounds. + test("both modes of the screen, not just the second one", () => { + const at = DRIVER.indexOf("const modes = ["); + assert.ok(at >= 0, "the quick split pass still audits one mode"); + const body = DRIVER.slice(at, DRIVER.indexOf("];", at)); + assert.match(body, /"even"/); + assert.match(body, /"all-in"/); + assert.match(DRIVER, /quick split · \$\{mode\}/, "a failure won't say which mode it was in"); + }); + + test("with the stepper open, since it isn't there otherwise", () => { + const at = DRIVER.indexOf('["even", () => {'); + assert.ok(at >= 0); + assert.match(DRIVER.slice(at, DRIVER.indexOf("}]", at)), /#waysChoices \.choiceChip/); + }); +});