diff --git a/JoshHeaps.Net/wwwroot/css/casino/holdem.css b/JoshHeaps.Net/wwwroot/css/casino/holdem.css index 5d866b1..9569337 100644 --- a/JoshHeaps.Net/wwwroot/css/casino/holdem.css +++ b/JoshHeaps.Net/wwwroot/css/casino/holdem.css @@ -1040,6 +1040,13 @@ button, border-radius: 16px 16px 0 0; padding: 0.35rem 0.9rem calc(1rem + env(safe-area-inset-bottom, 0px)); box-shadow: 0 -14px 40px rgba(0, 0, 0, 0.65); + /* This sheet had no height limit and didn't scroll, so on a phone held sideways + its first two actions — Fold and All-in, the two that matter — sat below the + bottom of the screen with no way to reach them. A dealer in landscape could + long-press a seat and then not fold it. */ + max-height: 85vh; + overflow-y: auto; + overscroll-behavior: contain; } .seatMenuHeader { @@ -2207,7 +2214,12 @@ body.hasActionBar #toastHost { display: flex; align-items: center; justify-content: center; - z-index: 50; + /* Every other sheet in here sits at 90-98; this one was at 50, underneath both + the action bar (55) and the toast (80). So the bar drew on top of the dialog + that pays people — in landscape one of its inputs was completely covered by + the Fold button — and a toast could sit over "Confirm Winner(s)". It's the odd + one out rather than a deliberate choice, and it joins the others. */ + z-index: 95; padding: 1.5rem; } @@ -2221,8 +2233,13 @@ body.hasActionBar #toastHost { border-radius: 10px; width: 100%; max-width: 480px; - max-height: 85vh; + /* 85vh is most of a 390px-tall landscape window, and the action bar is fixed over + the bottom of it — so the last of the wizard sat underneath the bar and its + controls couldn't be tapped. Leaving the bar's own height out of the budget + keeps the whole dialog above it. */ + max-height: calc(85vh - var(--actionBarHeight, 0px)); overflow-y: auto; + overscroll-behavior: contain; padding: 1.2rem 1.4rem 1.6rem; } @@ -2816,3 +2833,33 @@ textarea:focus-visible { [role="alertdialog"]:focus { outline: none; } + +/* ---------- Fields on a tablet ---------- + The 16px minimum and the 44px touch targets were both written into the phone + media query, so every iPad size missed them — measured at 768, 834 and 1024 wide, + where all ten setup fields sat at 13.3px. iOS zooms the page whenever a field + under 16px takes focus, so a dealer on an iPad was getting the table zoomed every + time they touched the blind amount, then pinching back out. The reason for the + rule was never "narrow screen", it was "iOS", and an iPad is iOS. + + Font size applies everywhere now — 13.3px was small for a settings field on a + desktop anyway. The touch sizing keys off `hover: none`, which is what actually + distinguishes a finger from a mouse; a tablet gets thumb-sized controls and a + desktop keeps its compact sidebar. */ +.seatEditorForm input:not([type="checkbox"]):not([type="color"]), +.panelBody input:not([type="checkbox"]):not([type="color"]), +.subPanelBody input:not([type="checkbox"]):not([type="color"]), +.panelBody select, +.panelBody textarea { + font-size: 16px; +} + +@media (hover: none) { + .seatEditorForm input:not([type="checkbox"]):not([type="color"]), + .panelBody input:not([type="checkbox"]):not([type="color"]), + .subPanelBody input:not([type="checkbox"]):not([type="color"]), + .panelBody select { + min-height: 44px; + padding: 0.4rem 0.6rem; + } +} diff --git a/tests/holdem/README.md b/tests/holdem/README.md index 34582a9..7c865b9 100644 --- a/tests/holdem/README.md +++ b/tests/holdem/README.md @@ -22,6 +22,32 @@ other in the page, in the load order from `TexasHoldem.cshtml`. | `splitCalculator.test.js` | Chopped pots, odd chips, unmakeable denominations | | `invariants.test.js` | 1000 randomised hands per property (see below) | +## The layout audit + +The unit tests above cover the money. `uiAudit.js` covers the thing you can only see +in a browser: whether a dealer can actually reach every control. + +```bash +node tests/holdem/runUiAudit.js # needs Playwright and the site running +``` + +It checks touch-action and tap highlights, that fields stay selectable and are large +enough not to trigger iOS zoom, that nothing important is covered or off-screen with +no way to scroll to it, that no grid column has collapsed to nothing, and that no +text is painted outside its box. It runs over the table, all five overlays and Quick +Split, at thirteen viewports from a 360px phone to a 1600px desktop and the same +phones on their side. + +Deliberately not in CI: the unit suite is dependency-free and runs in a second, while +this one needs a browser and a running site. Run it after any change that adds or +resizes a control. With no Playwright to hand, paste `uiAudit.js` into devtools and +call `LayoutAudit.runAll()`. + +It exists because these checks were being re-typed into a console every time +something changed, and each time they only covered whatever had just been touched. +That is how the standings table shipped with a player-name column zero pixels wide +and kept it for two rounds. + ## Why the randomised suite exists Every correctness bug found in this tool so far has been shape-dependent: blinds posted diff --git a/tests/holdem/runUiAudit.js b/tests/holdem/runUiAudit.js new file mode 100644 index 0000000..33ba18f --- /dev/null +++ b/tests/holdem/runUiAudit.js @@ -0,0 +1,134 @@ +// Drives uiAudit.js over the dealer's surfaces at every viewport worth caring about. +// +// node tests/holdem/runUiAudit.js [baseUrl] +// +// Needs Playwright available (`npx playwright install chromium` once). It is +// deliberately NOT wired into the unit-test suite or CI, because that suite is +// dependency-free and runs in a second — this one needs a browser and a running +// site. Run it after any change that adds or resizes a control. +// +// No Playwright to hand? Open the page, paste tests/holdem/uiAudit.js into devtools, +// and call LayoutAudit.runAll(). Same checks, one viewport. +const path = require("path"); + +const BASE = process.argv[2] || "http://localhost:5299"; +const AUDIT = path.join(__dirname, "uiAudit.js"); + +// Portrait phones, tablets, desktops, then the same phones on their side — landscape +// is where this layout has broken most often, because the table is sized from width +// and the action bar is fixed to a very short viewport. +const VIEWPORTS = [ + [360, 640, "Galaxy S8"], [375, 667, "iPhone SE"], [390, 844, "iPhone 14"], + [412, 915, "Pixel 7"], [430, 932, "iPhone Pro Max"], + [768, 1024, "iPad"], [834, 1112, "iPad Air"], [1024, 1366, "iPad Pro"], + [1280, 900, "desktop"], [1600, 1000, "wide desktop"], + [640, 360, "S8 landscape"], [667, 375, "SE landscape"], [844, 390, "iPhone landscape"], +]; + +const OVERLAYS = [ + ["rulings", () => document.getElementById("openRulingsBtn").click()], + ["seat menu", () => SeatMenu.open(HoldemState.load(), {}, 0)], + ["card picker", () => { document.getElementById("cardPickerOverlay").hidden = false; }], + ["add chips", () => AddChips.open(HoldemState.load(), 0, () => {})], + ["pot wizard", () => document.getElementById("resolvePotBtn").click()], +]; + +// A table part-way through a real night: long names, uneven stacks, a hand log. +function seedTable() { + const state = HoldemState.createDefault(); + HoldemState.resizeSeats(state, 9); + [["Christopher", 1200], ["Mike from work", 800], ["Alexandra", 1500], ["Bob", 250], ["Dee", 600]] + .forEach(([name, cash], i) => { + state.seats[i].name = name; + state.seats[i].inHand = true; + state.seats[i].buyIns = cash; + state.seats[i].stack = cash; + }); + state.smallBlind = 5; + state.bigBlind = 10; + for (let hand = 1; hand <= 4; hand++) { + state.handHistory.push({ + handNumber: hand, timestamp: Date.now(), pot: 120, + payouts: [{ id: state.seats[hand % 5].id, label: "P", amount: 120 }], + ledger: state.seats.slice(0, 5).map(p => ({ id: p.id, name: p.name, in: 30, won: 0 })), + }); + } + state.handNumber = 5; + HoldemState.save(state); +} + +async function main() { + let chromium; + try { + ({ chromium } = require("playwright")); + } catch { + console.error("This script needs Playwright: npm i -D playwright && npx playwright install chromium"); + console.error("Or paste tests/holdem/uiAudit.js into devtools and call LayoutAudit.runAll()."); + process.exit(2); + } + + const browser = await chromium.launch(); + const page = await browser.newPage(); + const failures = []; + const warnings = []; + let checks = 0; + + for (const [width, height, label] of VIEWPORTS) { + await page.setViewportSize({ width, height }); + + await page.goto(`${BASE}/Casino/TexasHoldem`); + await page.evaluate(seedTable); + await page.goto(`${BASE}/Casino/TexasHoldem`); + await page.click("#nextHandBtn"); + await page.waitForTimeout(300); + await page.evaluate(() => document.querySelectorAll("details.sidebarPanel").forEach(d => { d.open = true; })); + await page.addScriptTag({ path: AUDIT }); + + const table = await page.evaluate(() => LayoutAudit.runAll()); + checks++; + if (!table.clean) failures.push({ where: `table · ${label} ${width}x${height}`, offenders: table.failed.flatMap(f => f.offenders) }); + table.warnings.forEach(w => warnings.push(`${label}: ${w}`)); + + for (const [name, open] of OVERLAYS) { + await page.evaluate(open); + await page.waitForTimeout(180); + checks++; + const result = await page.evaluate(() => { + const overlay = [...document.querySelectorAll("[id$='Overlay']")].find(o => !o.hidden); + if (!overlay) return { offenders: [] }; + return LayoutAudit.occlusion(`#${overlay.id} button, #${overlay.id} a, #${overlay.id} input`); + }); + await page.evaluate(() => document.querySelectorAll("[id$='Overlay']").forEach(o => { o.hidden = true; })); + if (result.offenders.length) failures.push({ where: `${name} · ${label} ${width}x${height}`, offenders: result.offenders }); + } + } + + for (const [width, height, label] of VIEWPORTS.filter((_, i) => i % 4 === 0)) { + 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) }); + } + + await browser.close(); + + console.log(`${checks} audits across ${VIEWPORTS.length} viewports`); + warnings.forEach(w => console.log(` note: ${w}`)); + if (failures.length === 0) { + console.log("clean"); + return; + } + failures.forEach(f => { + console.log(`\nFAIL ${f.where}`); + f.offenders.forEach(o => console.log(` ${o}`)); + }); + process.exit(1); +} + +main(); diff --git a/tests/holdem/uiAudit.js b/tests/holdem/uiAudit.js new file mode 100644 index 0000000..7768ddd --- /dev/null +++ b/tests/holdem/uiAudit.js @@ -0,0 +1,278 @@ +// Layout and touch audits for the dealer, written once so they stop being written +// again every round. +// +// These checks have been re-typed into a browser console at least six times over +// the course of building this thing, and each time they only covered whatever had +// just been changed. That is exactly how the standings table shipped with a +// zero-pixel player-name column and survived two rounds: the audit that would have +// caught it existed only in a previous session's scrollback. +// +// This file is deliberately dependency-free and browser-side. It is injected into +// the page (see runUiAudit.js) rather than imported, so it needs no build step, no +// package.json, and no browser automation of its own — anyone can also paste it +// into devtools and call LayoutAudit.runAll(). +const LayoutAudit = { + // Everything a dealer has to be able to hit. + CONTROL_SELECTOR: "button, a, input, select, textarea, [role='button'], .seat", + + describe(el) { + if (!el) return "(nothing)"; + const id = el.id ? `#${el.id}` : ""; + const cls = (el.className || "").toString().trim().split(/\s+/).filter(Boolean).slice(0, 2).join("."); + const text = (el.textContent || "").replace(/\s+/g, " ").trim().slice(0, 24); + return `${el.tagName.toLowerCase()}${id}${cls ? "." + cls : ""}${text ? ` "${text}"` : ""}`; + }, + + visibleControls() { + return [...document.querySelectorAll(this.CONTROL_SELECTOR)].filter(el => { + const rect = el.getBoundingClientRect(); + if (rect.width === 0 || rect.height === 0) return false; + const style = getComputedStyle(el); + return style.visibility !== "hidden" && style.display !== "none"; + }); + }, + + // ---- Touch (see the mobile round) ---- + + // Without touch-action: manipulation a mobile browser holds every tap for ~300ms + // deciding whether it was a double-tap-to-zoom. This tool uses double-tap on a + // seat as its fold shortcut, so that wait is on the critical path. + touchAction() { + const offenders = this.visibleControls() + .filter(el => getComputedStyle(el).touchAction === "auto") + .map(el => this.describe(el)); + return { name: "touch-action", offenders, checked: this.visibleControls().length }; + }, + + tapHighlight() { + const offenders = this.visibleControls() + .filter(el => { + const colour = getComputedStyle(el).webkitTapHighlightColor; + return colour && !/rgba\(0,\s*0,\s*0,\s*0\)|transparent/.test(colour); + }) + .map(el => this.describe(el)); + return { name: "tap-highlight", offenders, checked: this.visibleControls().length }; + }, + + // Text selection must stay ON for anything a dealer types into. + textSelectable() { + const offenders = [...document.querySelectorAll("input[type='text'], input[type='number'], textarea")] + .filter(el => el.offsetParent !== null) + .filter(el => { + const style = getComputedStyle(el); + return (style.webkitUserSelect || style.userSelect) === "none"; + }) + .map(el => this.describe(el)); + return { name: "fields stay selectable", offenders }; + }, + + // ---- Reachability (see the occlusion round) ---- + + // A control covered by something else is unreachable however good it looks. + // Sampling several points matters: a button you can only hit in one corner is + // covered for practical purposes. + // Can this element be brought into view by scrolling? Something below the fold on + // a page that scrolls is perfectly reachable; something below the fold in a panel + // that *doesn't* scroll is lost. The first version of this check didn't make the + // distinction and reported every link under the fold as a fault. + reachableByScrolling(el) { + const rect = el.getBoundingClientRect(); + if (getComputedStyle(el).position === "fixed") return false; + + for (let node = el.parentElement; node; node = node.parentElement) { + const style = getComputedStyle(node); + const scrolls = /auto|scroll|overlay/.test(style.overflowY) || node === document.body; + if (!scrolls) continue; + const box = node === document.body ? { top: 0, bottom: innerHeight } : node.getBoundingClientRect(); + const room = node === document.body + ? document.documentElement.scrollHeight - innerHeight - scrollY + : node.scrollHeight - node.clientHeight - node.scrollTop; + if (rect.top >= box.bottom && room > 0) return true; + if (rect.bottom <= box.top && (node === document.body ? scrollY > 0 : node.scrollTop > 0)) return true; + } + return false; + }, + + occlusion(selector = "#actionBar button, #holdemHeader button, .seat") { + const offenders = []; + const warnings = []; + [...document.querySelectorAll(selector)].forEach(el => { + const rect = el.getBoundingClientRect(); + if (rect.width === 0 || rect.height === 0) return; + + const outsideViewport = rect.bottom <= 0 || rect.top >= innerHeight || + rect.right <= 0 || rect.left >= innerWidth; + if (outsideViewport) { + if (!this.reachableByScrolling(el)) { + offenders.push(`${this.describe(el)} :: off-screen and cannot be scrolled to`); + } + return; + } + + const points = [ + [rect.left + rect.width / 2, rect.top + rect.height / 2], + [rect.left + rect.width * 0.25, rect.top + rect.height * 0.3], + [rect.left + rect.width * 0.75, rect.top + rect.height * 0.3], + [rect.left + rect.width * 0.25, rect.top + rect.height * 0.7], + [rect.left + rect.width * 0.75, rect.top + rect.height * 0.7], + ].filter(([x, y]) => x >= 0 && y >= 0 && x < innerWidth && y < innerHeight); + if (points.length === 0) return; + + let blocked = 0; + let testable = 0; + let blockedBy = null; + points.forEach(([x, y]) => { + const top = document.elementFromPoint(x, y); + // A null result means the point isn't hit-testable — typically it sits + // inside the element but outside a scrollable ancestor's clip, i.e. the + // control is scrolled out of view rather than covered by anything. + // Counting that as "covered by null" was reporting a perfectly + // reachable button in a sheet that scrolls. + if (!top) return; + testable++; + if (el.contains(top) || top === el || top.contains(el)) return; + // Seats ring a round table and overlap each other at the rim by + // design; that is crowding, not an overlay hiding a control. + if (top.closest && top.closest(".seat")) return; + blocked++; + if (!blockedBy) blockedBy = this.describe(top); + }); + + if (testable === 0) { + if (!this.reachableByScrolling(el)) { + offenders.push(`${this.describe(el)} :: not hit-testable anywhere and cannot be scrolled to`); + } + return; + } + if (blocked === 0) return; + + const note = `${this.describe(el)} :: ${blocked === testable ? "fully" : `${blocked}/${testable}`} covered by ${blockedBy}`; + + // A toast is transient by design: it dismisses itself and carries a close + // button, and on a desktop it necessarily lands over a table that fills the + // content column. Partial overlap from one is a fact of the layout, not a + // fault. Being *fully* covered by a toast is still a failure though — that + // was a real bug once, where a seat sat exactly under the Undo button and a + // tap meant for a player took back the last action instead. + const transient = blockedBy && /toast/i.test(blockedBy); + if (transient && blocked < testable) warnings.push(note); + else offenders.push(note); + }); + return { name: "nothing important is covered", offenders, warnings }; + }, + + // ---- Layout (the check that would have caught the standings regression) ---- + + // A grid column squeezed to nothing doesn't truncate its text, it erases it. + // This is the one that was missing: the player-name column sat at 0px through + // two rounds because nothing ever asked whether a column had survived. + collapsedColumns() { + const offenders = []; + [...document.querySelectorAll("[class*='Row'], [class*='row']")].forEach(row => { + if (getComputedStyle(row).display !== "grid") return; + [...row.children].forEach(cell => { + if (getComputedStyle(cell).display === "none") return; + const hasText = (cell.textContent || "").trim().length > 0; + if (hasText && cell.getBoundingClientRect().width < 4) { + offenders.push(`${this.describe(row)} > ${this.describe(cell)} :: ${Math.round(cell.getBoundingClientRect().width)}px wide`); + } + }); + }); + return { name: "no column collapsed to nothing", offenders }; + }, + + // Text actually painted outside the box meant to contain it — which on a table of + // overlapping seats means painted on top of a neighbour. + // + // The clipping check is the whole subtlety, and the first version of this got it + // wrong. A Range's bounding rect measures the *laid out* text, which happily runs + // past the element when `overflow: hidden` is quietly clipping it — so an + // ellipsised name was reported as spilling when nothing was drawn outside at all. + // Only overflow that is actually visible can paint over a neighbour. + textSpill(selector = ".seatPuckStack, .seatPuckTotal, .seatPuckName") { + const offenders = []; + [...document.querySelectorAll(selector)].forEach(el => { + const text = (el.textContent || "").trim(); + if (!text) return; + + const style = getComputedStyle(el); + const clipsItself = style.overflow !== "visible" || style.overflowX !== "visible"; + if (clipsItself) return; + + const container = el.closest(".seat") || el.parentElement; + if (!container) return; + + const range = document.createRange(); + range.selectNodeContents(el); + const drawn = range.getBoundingClientRect(); + const bounds = container.getBoundingClientRect(); + if (drawn.left < bounds.left - 1 || drawn.right > bounds.right + 1) { + offenders.push(`${this.describe(el)} "${text}" paints outside ${this.describe(container)}`); + } + }); + return { name: "no text painted outside its box", offenders }; + }, + + horizontalOverflow() { + const offenders = document.documentElement.scrollWidth > innerWidth + 1 + ? [`page scrolls sideways: ${document.documentElement.scrollWidth}px in a ${innerWidth}px viewport`] + : []; + return { name: "page never scrolls sideways", offenders }; + }, + + // iOS zooms the whole page when a field under 16px takes focus, and the dealer + // is then pinching back out mid-hand. + fieldFontSize() { + const offenders = [...document.querySelectorAll("input, textarea, select")] + .filter(el => el.offsetParent !== null) + .filter(el => ["text", "number", "search", "tel", "url", "email", "password", ""].includes(el.type || "")) + .filter(el => parseFloat(getComputedStyle(el).fontSize) < 16) + .map(el => `${this.describe(el)} at ${getComputedStyle(el).fontSize}`); + return { name: "no field small enough to trigger iOS zoom", offenders }; + }, + + // ---- Reach (touch target sizes) ---- + + touchTargets(minimum = 44) { + const offenders = this.visibleControls() + .filter(el => el.type !== "color") + .filter(el => { + const rect = el.getBoundingClientRect(); + return rect.height < minimum - 0.5 || rect.width < minimum - 0.5; + }) + .map(el => { + const rect = el.getBoundingClientRect(); + return `${this.describe(el)} :: ${Math.round(rect.width)}x${Math.round(rect.height)}`; + }); + return { name: `touch targets at least ${minimum}px`, offenders }; + }, + + // ---- Runner ---- + + runAll(options = {}) { + const checks = [ + this.touchAction(), + this.tapHighlight(), + this.textSelectable(), + this.occlusion(options.occlusionSelector), + this.collapsedColumns(), + this.textSpill(), + this.horizontalOverflow(), + this.fieldFontSize(), + ]; + if (options.includeTouchTargets) checks.push(this.touchTargets(options.minimumTarget)); + + return { + viewport: `${innerWidth}x${innerHeight}`, + failed: checks.filter(c => c.offenders.length > 0), + // Things worth knowing that aren't faults — see occlusion() on toasts. + warnings: checks.flatMap(c => c.warnings || []), + passed: checks.filter(c => c.offenders.length === 0).map(c => c.name), + clean: checks.every(c => c.offenders.length === 0), + }; + }, +}; + +if (typeof module !== "undefined" && module.exports) { + module.exports = LayoutAudit; +}