diff --git a/JoshHeaps.Net/wwwroot/css/casino/holdem.css b/JoshHeaps.Net/wwwroot/css/casino/holdem.css index e0ca5b0..bc405a4 100644 --- a/JoshHeaps.Net/wwwroot/css/casino/holdem.css +++ b/JoshHeaps.Net/wwwroot/css/casino/holdem.css @@ -1213,34 +1213,28 @@ button, cursor: pointer; z-index: 2; transition: opacity 0.15s ease; - /* A dealer puck should look like a puck — small, sitting on the rim of its seat. - But tapping it hands the button to that seat and re-posts the blinds, and 22px - is half the 44px a fingertip needs. The hit area grows without the chip doing - so: the visual stays 22px and an invisible pad around it catches the tap. - Deliberately not centred on the chip — it's pushed down and left, away from the - neighbouring seat the chip juts towards, so the bigger target can't start - stealing taps meant for the seat next door. */ - isolation: isolate; } -.dealerChip::after { - content: ""; - position: absolute; - /* Grows inward, over its own seat, not outward onto the neighbours. - - The chip already sits outside its seat by design — top:-10px, right:-10px — so - a pad that also reaches up and right lands on whatever seat is next around the - oval. It did: three of nine seats had taps taken by a neighbour's chip, seat 1 - losing 10 of 49 sampled points including its own centre, which meant tapping - the middle of seat 1 gave the button to seat 9. - - Down and left is its own seat, which is a large target that can spare the - corner. The two pixels of outward reach are just to cover the chip's border. */ - top: -3px; - right: -3px; - bottom: -18px; - left: -18px; -} +/* There is no invisible pad around this chip any more, and that is the point. + + It was here to make a 22px disc meet the 44px a fingertip needs. It was moved twice + trying to find somewhere to grow that wasn't already spoken for — outward first, + where it took taps from the seat next door; then inward, over "its own seat, which + is a large target that can spare the corner". + + The seat could not spare it. Measured on a 390px phone, a 72x46 puck: the pad ran + from the puck's own centre to its right edge and from its top to 3px off its bottom, + and the chip's click handler calls stopPropagation. The seat holding the button kept + 52% of its face — 130 of 276 sampled points went to the chip, its exact middle among + them. Tapping the button seat to open its editor did nothing at all, and on a fresh + table the button is on seat 1, so the first thing a dealer touches is the one dead + puck on the felt. + + The pad had no user left to serve. A finger never reaches this chip: on touch the + ghost chips are pointer-events: none (below), and the one live chip is the seat that + already has the button — see .dealerChip.active. Giving the button on touch has + always been the seat's long-press menu. What remains is a mouse on a revealed ghost, + and 22px is a mouse target with room to spare. */ /* A poker table has one dealer button. This one drew nine. @@ -1276,11 +1270,25 @@ button, pointer-events: auto; } +/* The one that has the button is a marker, not a control. + + Every other chip on the felt answers "give the button to this seat". This one would + answer "give the button to the seat that has it" — which is nothing, except mid-hand, + where makeDealer takes the blinds back and re-posts them. So it was a puck that either + did nothing or did something unasked-for, sitting on top of the seat a dealer was + reaching for. The seat menu already knew this and hides "Give the button" on the seat + that holds it; the chip now agrees. + + Inert to the pointer, still there to read. Its label says " has the dealer + button" — a statement, and the only place on the page that says it — so it keeps its + place in the tree and under the keyboard. */ .dealerChip.active { background: radial-gradient(circle at 35% 30%, #ffe9a8, #f5d547 55%, #cba428 100%); border-color: var(--gold-deep); color: #3a2e00; opacity: 1; + pointer-events: none; + cursor: default; } .dealerChip.hidden { @@ -4793,23 +4801,10 @@ body.hasActionBar #toastHost { padding: 0 var(--sp-2); } - /* The dealer button has to stay a 22px disc — it's a chip sitting on a seat, and - drawing it thumb-sized would swallow the puck. So the disc keeps its size and - an invisible pad around it takes the tap. It reaches outward, away from the - puck's own tap target, so pressing the seat still opens the seat. */ - .dealerChip::after { - content: ""; - position: absolute; - top: 50%; - left: 50%; - width: 44px; - height: 44px; - transform: translate(-50%, -50%); - } - - .dealerChip { - overflow: visible; - } + /* The dealer chip's 44px tap pad used to be redeclared here, centred on the disc. + On a 72x46 puck that square started at the puck's own middle, and the seat that + had the button could not be tapped where a thumb lands. It is gone; see + .dealerChip near the top of this file for why nothing needs to replace it. */ .toastAction { min-height: 44px; diff --git a/JoshHeaps.Net/wwwroot/js/casino/holdem/main.js b/JoshHeaps.Net/wwwroot/js/casino/holdem/main.js index cdba3f3..82224e3 100644 --- a/JoshHeaps.Net/wwwroot/js/casino/holdem/main.js +++ b/JoshHeaps.Net/wwwroot/js/casino/holdem/main.js @@ -246,6 +246,11 @@ document.addEventListener("DOMContentLoaded", () => { }, makeDealer: index => { + // Moving the button to where it already is is not a move. Saying so costs a + // toast nobody asked for, and mid-hand it would take the blinds back off the + // felt and re-post the same chips to say nothing happened. + if (index === state.dealerSeatIndex) return; + // Before the deal there's nothing posted, so this is just moving a marker. // After it, the blinds have to follow the button or the badges and the // money disagree — but that's a real thing happening to real chips, and diff --git a/JoshHeaps.Net/wwwroot/js/casino/holdem/render.js b/JoshHeaps.Net/wwwroot/js/casino/holdem/render.js index fe0ecda..c6ad5c8 100644 --- a/JoshHeaps.Net/wwwroot/js/casino/holdem/render.js +++ b/JoshHeaps.Net/wwwroot/js/casino/holdem/render.js @@ -1370,6 +1370,11 @@ const Render = { // so it goes through the same hook — otherwise this one path would move // the button without moving the blinds that follow it. node.querySelector(".dealerChip").addEventListener("click", e => { + // The seat that already has the button has nothing to give itself, and + // this handler swallows the event — so a chip that answered here would + // be a hole in its own seat. CSS keeps the pointer off it; the keyboard + // can still reach it, and this is what it finds. + if (index === this.state.dealerSeatIndex) return; e.stopPropagation(); this.hooks.onGiveButton(index); }); diff --git a/tests/holdem/seatTapTargets.test.js b/tests/holdem/seatTapTargets.test.js new file mode 100644 index 0000000..207ed6e --- /dev/null +++ b/tests/holdem/seatTapTargets.test.js @@ -0,0 +1,119 @@ +// The seat holding the dealer button has to still be a seat you can tap. +// +// On a fresh table the button is on seat 1, and seat 1 is where the first player goes. +// Measured on a 390px phone, a 72x46 puck: the dealer chip's invisible tap pad ran from +// the puck's own centre to its right edge and from its top to 3px off its bottom. The +// chip's click handler calls stopPropagation, so every tap that landed there reached +// nothing at all — the seat holding the button kept 52% of its own face, 130 of 276 +// sampled points going to a chip whose only answer was "give the button to the seat +// that has it". Seating the first player, on the seat a dealer reaches for first, did +// nothing. Every other seat kept 97%, and the button moves every hand, so the dead puck +// moved around the table with it. +// +// Three things had to agree for that to happen and they are the three checked here: the +// pad exists, the chip under it is live, and its handler eats the event. The pad is gone +// (nothing needs it — no finger can reach any chip), the chip that has the button is +// inert to the pointer, and the handler bails rather than swallowing a tap it has no +// answer for. +const test = require("node:test"); +const { describe } = require("node:test"); +const assert = require("node:assert"); +const fs = require("fs"); +const path = require("path"); + +const src = (...parts) => fs.readFileSync(path.join(__dirname, "..", "..", "JoshHeaps.Net", ...parts), "utf8"); + +const CSS = src("wwwroot", "css", "casino", "holdem.css"); +const RENDER = src("wwwroot", "js", "casino", "holdem", "render.js"); +const MAIN = src("wwwroot", "js", "casino", "holdem", "main.js"); +const AUDIT = fs.readFileSync(path.join(__dirname, "uiAudit.js"), "utf8"); + +// The body of every rule written at exactly this selector, concatenated. Every one of +// them, because the phone block redeclared the chip's pad under the same name and it +// was the redeclared copy — 44px centred on the disc — that reached furthest in. +const rulesFor = selector => { + const escaped = selector.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const re = new RegExp(`(^|[},])\\s*${escaped}\\s*\\{([^}]*)\\}`, "gm"); + const bodies = []; + let found; + while ((found = re.exec(CSS)) !== null) bodies.push(found[2]); + return bodies; +}; + +// --------------------------------------------------------------------------- + +describe("the seat under the dealer button is still a seat", () => { + test("the chip has no invisible pad reaching back over its own puck", () => { + // Not "a smaller pad". A fixed pad cannot be safe at every puck size: the seat + // search hands out pucks from 48px wide to 100-odd depending on the screen and + // the number of chairs, and any pad big enough to matter on the large one + // covers the middle of the small one. + assert.equal(rulesFor(".dealerChip::after").length, 0, + "the dealer chip grew a tap pad again — see the note above .dealerChip in holdem.css"); + }); + + test("the chip that has the button takes no taps", () => { + const bodies = rulesFor(".dealerChip.active"); + assert.ok(bodies.length > 0, ".dealerChip.active has no rule at all"); + assert.ok(bodies.some(body => /pointer-events:\s*none/.test(body)), + "the seat holding the button has a live control sitting on it"); + }); + + test("the other chips stay inert until a pointer that can hover arrives", () => { + // This is what makes the pad unnecessary rather than merely unwanted. On touch + // there is no hover, so no ghost chip is reachable by a finger and the only live + // chip is the inert one above. Nothing a thumb can press is 22px. + const ghost = rulesFor(".dealerChip:not(.active)"); + assert.ok(ghost.some(body => /pointer-events:\s*none/.test(body)), + "a chip nobody can see is taking taps"); + + const hover = CSS.slice(CSS.indexOf("@media (hover: hover)")); + const revealed = hover.match(/\.seat:hover\s+\.dealerChip:not\(\.active\)\s*\{([^}]*)\}/); + assert.ok(revealed, "hovering a seat no longer offers to put the button there"); + assert.match(revealed[1], /pointer-events:\s*auto/, + "the mouse path to moving the button is gone"); + }); + + test("and the handler does not swallow a tap it has no answer for", () => { + // stopPropagation is what turned a pointless control into a hole. It still runs + // for the eight seats the button could move to; the ninth returns first. + const handler = RENDER.slice(RENDER.indexOf('querySelector(".dealerChip").addEventListener')); + const body = handler.slice(0, handler.indexOf("});")); + const guard = body.indexOf("dealerSeatIndex"); + const stop = body.indexOf("stopPropagation"); + assert.ok(guard >= 0, "the chip's click handler does not check whose button it is"); + assert.ok(stop >= 0, "the chip stopped guarding the seat behind it from the other chips"); + assert.ok(guard < stop, + "the guard has to come first, or the tap is eaten before anyone decides it meant nothing"); + }); + + test("moving the button to where it already is is not a move", () => { + // The keyboard can still reach the chip, and the same no-op would take the + // blinds off the felt and re-post the same chips to announce that nothing + // happened. The seat menu has always hidden "Give the button" on the seat that + // holds it; this is the same rule where the money is. + const from = MAIN.indexOf("makeDealer: index =>"); + assert.ok(from >= 0, "makeDealer is gone or renamed"); + const body = MAIN.slice(from, MAIN.indexOf("blindsArePosted", from)); + assert.match(body, /if\s*\(\s*index\s*===\s*state\.dealerSeatIndex\s*\)\s*return/, + "makeDealer will re-post the blinds to move the button nowhere"); + }); +}); + +describe("the sweep can see this kind of failure now", () => { + test("a control inside a seat counts as a rival for the seat's tap", () => { + // The layout sweep already had a check named "a seat's centre belongs to that + // seat", written after the same chip's pad grew *outward* and started taking + // taps from the seat next door. It ran clean through all of this: it waved + // through anything the seat contained, and the chip is inside the seat. The + // check that existed for this exact bug could not see the second half of it. + const from = AUDIT.indexOf("seatOwnership()"); + assert.ok(from >= 0, "the seat-ownership check is gone"); + const body = AUDIT.slice(from, AUDIT.indexOf("oneDealerButton()", from)); + + assert.match(body, /seat\.contains\(rival\)/, + "a seat's own control is being waved through again"); + assert.doesNotMatch(body, /if\s*\(\s*!hit\s*\|\|\s*seat\.contains\(hit\)/, + "the early return that hid this is back: it exempts every descendant, chip included"); + }); +}); diff --git a/tests/holdem/uiAudit.js b/tests/holdem/uiAudit.js index 0a62434..c3e5c8e 100644 --- a/tests/holdem/uiAudit.js +++ b/tests/holdem/uiAudit.js @@ -897,6 +897,14 @@ const LayoutAudit = { // // Reported separately from occlusion because the cause is different: not an // overlay on top of the table, but two pieces of the table fighting over a tap. + // + // It then made the same mistake one level down. `seat.contains(hit)` waved through + // anything inside the seat, and the dealer chip is inside the seat: when its hit + // pad was moved *inward* to stop it reaching the neighbours, it covered its own + // seat's middle instead and this check called that fine. The chip's handler calls + // stopPropagation, so those taps reached nothing — on a fresh table the button is + // on seat 1, and seating the first player was the one thing on the felt that did + // not work. A control inside the seat is a rival for the tap, not part of the seat. seatOwnership() { // Seats behind an open sheet aren't fighting each other for taps — nothing is // meant to reach them at all. Asking the question there produced six offenders @@ -916,13 +924,29 @@ const LayoutAudit = { if (x < 0 || y < 0 || x >= innerWidth || y >= innerHeight) return; const hit = document.elementFromPoint(x, y); - if (!hit || seat.contains(hit) || hit === seat) return; + if (!hit) return; + + // The nearest control at that point, if any. A control is a rival for the + // tap wherever it lives — the seat's own chip swallows the event as surely + // as the seat next door's does. + const CONTROLS = "button, a, input, select, textarea, [role='button']"; + const rival = hit.closest ? hit.closest(CONTROLS) : null; + + if (rival && seat.contains(rival)) { + offenders.push( + `seat ${index + 1}'s centre goes to its own ${this.describe(rival)}` + ); + return; + } + + // Its own name, stack or felt. That is the seat, and a tap there is a tap + // on the seat. + if (!rival && (hit === seat || seat.contains(hit))) return; const other = hit.closest(".seat"); const who = other ? `seat ${seats.indexOf(other) + 1}` : this.describe(hit); - const control = hit.matches && hit.matches("button, a, input, select, [role='button']"); offenders.push( - `seat ${index + 1}'s centre goes to ${who}${control ? "'s " + this.describe(hit) : ""}` + `seat ${index + 1}'s centre goes to ${who}${rival ? "'s " + this.describe(rival) : ""}` ); });