Merge pull request 'Hold'em: give the seat holding the dealer button its own middle back' (#121) from fix/holdem-dealer-chip-eats-its-seat into master

Reviewed-on: #121

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.
+/* There is no invisible pad around this chip any more, and that is the point.
 
-       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.
+   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".
 
-       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;
-}
+   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 "<name> 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;

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

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);
             });

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");
+    });
+});

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) : ""}`
             );
         });