Hold'em: every seat on the ring is the same seat, and the ring is measured before it is believed #122

Open
jheaps wants to merge 27 commits from polish/holdem-seats-one-size into master
6 changed files with 377 additions and 14 deletions
Showing only changes of commit 2e372d4677 - Show all commits
@@ -799,7 +799,12 @@
<div id="potWizardModal" role="dialog" aria-labelledby="potWizardTitle">
<div class="potWizardHeader">
<h2 id="potWizardTitle" tabindex="-1" data-modal-focus>Resolve the Pot</h2>
<button id="closePotWizardBtn" data-modal-close class="holdemBtn holdemBtnGhost">✕</button>
@* Named, and named for what leaving does rather than "Close". The glyph is
the whole of this control, so unnamed it announced as "button" — on the
one dialog in the tool that divides money, where the difference between
backing out and confirming is somebody's stack. *@
<button id="closePotWizardBtn" type="button" data-modal-close class="holdemBtn holdemBtnGhost"
aria-label="Close without paying the pot">✕</button>
</div>
<div id="potWizardBody"></div>
@* The buttons live outside the scrolling body on purpose. This is the one
@@ -853,9 +853,27 @@ button,
color: #fff;
}
/* A transparent sheet over the whole felt — and until now it took every tap that landed
on it.
The seats are pucks scattered around a ring; the space between them is the middle of
the table, and the middle of the table is where the board and the pot badge live. But
this layer is `inset: 0` and a later sibling of #pokerTable, so it covers all of it,
and an element with no background still hit-tests. Measured at 1280x900 with the board
turned on and five cards out: all five community cards — buttons, each one the way the
dealer enters or corrects that card — hit-tested to `div#seatsLayer`. Not one of them
was pressable, on any screen wide enough to keep the board in the middle.
Nothing scripted could see it, because a probe that calls `.click()` on the node skips
hit-testing entirely; the sweep could not see it either, because its occlusion check
only ever asked about the bar, the header and the seats. #betsLayer, the other sheet
over the felt, has had `pointer-events: none` all along.
The pucks take their own taps back below. Everything else in here is inside one. */
#seatsLayer {
position: absolute;
inset: 0;
pointer-events: none;
}
/* ---------- Bets on the felt ---------- */
@@ -949,6 +967,11 @@ button,
did not: a 64px puck measured 74 across once its padding and border were counted,
and the search that sets this compares it against distances on the felt. */
box-sizing: border-box;
/* The layer above is inert so the middle of the felt is reachable; a puck is the
thing in it that is not. Its own children inherit this, and the two that want to
be untouchable — the ghost dealer chips, the one that is only a marker — say so
for themselves further down. */
pointer-events: auto;
width: var(--seatPuckW, 96px);
/* Every min-height on a seat is a drawn height now, border and padding included,
because of the box-sizing above. They were content heights before, so each one
@@ -1843,11 +1843,27 @@ const Render = {
if (onStrip) {
if (strip && row.parentElement !== strip) strip.appendChild(row);
if (strip) strip.hidden = false;
if (centre) { centre.style.maxWidth = ""; centre.style.maxHeight = ""; }
if (centre) { centre.style.width = ""; centre.style.maxWidth = ""; centre.style.maxHeight = ""; }
} else {
if (centre && row.parentElement !== centre) centre.insertBefore(row, centre.firstChild);
if (strip) strip.hidden = true;
if (centre) centre.style.maxWidth = `${Math.round(width)}px`;
if (centre) {
// Both, and the width is the half that was missing.
//
// `max-width` caps a box; it does not claim anything. The centre is a
// flex column that sizes to its content, so with 463px of measured room
// written into its max-width it went on being 136px wide — and the board
// row inside it is `width: 100%` of that, which is a loop that comes to
// rest on the cards' own `min-width`. Measured with the board on: the
// middle had 294px on an iPad, 463px on a desktop and 542px on a wide
// one, and in all three the five community cards drew 24px across. On
// the strip, where the row simply gets the width of the page, the same
// cards draw 44. They are buttons — a tap on one opens the picker for
// that card — so 24px was a little over half the floor this sweep holds
// every other control on the felt to.
centre.style.width = `${Math.round(width)}px`;
centre.style.maxWidth = `${Math.round(width)}px`;
}
}
// The chips were laid out against whatever was in the middle of the felt a
@@ -2530,8 +2546,21 @@ const Render = {
const valueInput = row.querySelector(".customFieldValue");
labelInput.value = field.label;
valueInput.value = field.value;
// Same as the chip legend's remove button, and for the same reason: "×" is
// the whole of this control, so unnamed it announces as a button and nothing
// more, in a column of others exactly like it. The label the dealer typed is
// what tells them apart, and it changes as it is typed.
const removeBtn = row.querySelector(".removeFieldBtn");
const nameRemove = () => {
const typed = labelInput.value.trim();
removeBtn.setAttribute("aria-label", typed ? `Remove "${typed}"` : "Remove this note");
};
nameRemove();
labelInput.addEventListener("input", () => {
field.label = labelInput.value;
nameRemove();
this.hooks.save(this.state);
this.renderSeatDisclosures(form, player);
});
@@ -2540,7 +2569,7 @@ const Render = {
this.hooks.save(this.state);
this.renderSeatDisclosures(form, player);
});
row.querySelector(".removeFieldBtn").addEventListener("click", () => {
removeBtn.addEventListener("click", () => {
player.customFields.splice(fieldIdx, 1);
this.renderCustomFields(form, player, index);
this.hooks.save(this.state);
+17 -2
View File
@@ -322,17 +322,32 @@ async function main() {
// spent already.
if (shape !== TABLES[0][0]) continue;
// The whole audit, not the one check.
//
// This ran `occlusion()` and nothing else — one rule of twenty-five — so
// for the sweep's entire life the five sheets a dealer spends half the
// night in had never been measured for touch targets, contrast, control
// names, text spill or anything else. The three rules that made that a
// reasonable thing to do are modal-aware now: two of them ask about the
// fixed bar, which is not a question while a scrim is covering it, and
// the third called every card below the fold of a sheet that scrolls an
// unreachable button.
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`);
const sheet = LayoutAudit.runAll({
occlusionSelector: overlay
? `#${overlay.id} button, #${overlay.id} a, #${overlay.id} input`
: undefined,
});
return { offenders: sheet.failed.flatMap(f => f.offenders), warnings: sheet.warnings };
});
await page.evaluate(() => document.querySelectorAll("[id$='Overlay']").forEach(o => { o.hidden = true; }));
record(`${name} · ${label} ${width}x${height}`, result.offenders);
result.warnings.forEach(w => warnings.push(`${name} · ${label}: ${w}`));
}
}
}
+209
View File
@@ -0,0 +1,209 @@
// The five cards in the middle of the felt, and the sheet of glass over them.
//
// Two faults, one on top of the other, and the sweep could see neither.
//
// #seatsLayer is a transparent `inset: 0` sibling of the table that holds the seat
// pucks. The pucks ring the felt; the middle of it is empty layer — and an element with
// no background still hit-tests. So every tap aimed at the community cards landed on the
// layer instead. Measured at 1280x900 with the board on and five cards out: all five
// hit-tested to `div#seatsLayer`, on every screen wide enough to keep the board in the
// middle rather than on the strip. Each one is a button that opens the picker for that
// card, and not one of them could be pressed. #betsLayer, the other sheet over the felt,
// has been `pointer-events: none` since it was written.
//
// Underneath that, the cards were a little over half the size they were meant to be.
// `placeBoard` measures how much middle the felt has and writes it to the centre's
// `max-width` — but a max-width caps a box, it does not claim anything, and the centre
// is a flex column that sizes to its content. So with 463px of measured room it went on
// being 136px wide, the board row inside it is `width: 100%` of that, and five cards
// sharing 136px come to rest on their own `min-width`: 24px, against a 44px floor this
// sweep holds every other control on the felt to. On the strip, where the row simply
// gets the width of the page, the same cards have always drawn 44.
//
// Neither was visible to a scripted probe — `.click()` on a node skips hit-testing
// entirely — and the sweep's occlusion check had only ever been shown three selectors'
// worth of the page: the action bar, the header, and the seats.
const test = require("node:test");
const { describe } = require("node:test");
const assert = require("node:assert");
const fs = require("fs");
const path = require("path");
const ROOT = path.join(__dirname, "..", "..", "JoshHeaps.Net");
const CSS = fs.readFileSync(path.join(ROOT, "wwwroot", "css", "casino", "holdem.css"), "utf8");
const AUDIT = fs.readFileSync(path.join(__dirname, "uiAudit.js"), "utf8");
const DRIVER = fs.readFileSync(path.join(__dirname, "runUiAudit.js"), "utf8");
const src = name => fs.readFileSync(path.join(ROOT, "wwwroot", "js", "casino", "holdem", `${name}.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("the sheet over the felt", () => {
test("it does not take taps meant for what is under it", () => {
assert.match(ruleFor("#seatsLayer"), /pointer-events:\s*none/);
});
// Which is only safe because the thing in it that *is* a control says so itself.
test("but a seat still does", () => {
assert.match(ruleFor(".seat"), /pointer-events:\s*auto/);
});
// The two chips that want to stay untouchable set their own value and are inside a
// seat, so they override what they inherit rather than depending on the layer.
test("and the two pucks that are markers rather than controls still are", () => {
assert.match(ruleFor(".dealerChip:not(.active)"), /pointer-events:\s*none/);
assert.match(ruleFor(".dealerChip.active"), /pointer-events:\s*none/);
});
test("the other sheet is unchanged, and was always right", () => {
assert.match(ruleFor("#betsLayer"), /pointer-events:\s*none/);
});
test("what it measured is written beside the rule", () => {
const at = CSS.indexOf("/* A transparent sheet over the whole felt");
assert.ok(at >= 0, "the rule carries no note saying why");
const note = CSS.slice(at, CSS.indexOf("*/", at));
assert.match(note, /1280x900/);
assert.match(note, /seatsLayer/);
});
});
describe("how wide the middle actually is", () => {
test("the room it measured is claimed, not only capped", () => {
const render = src("render");
const at = render.indexOf("placeBoard(row, strip) {");
assert.ok(at >= 0, "there is no placeBoard");
const body = render.slice(at, render.indexOf("\n },", at));
assert.match(body, /centre\.style\.width = `\$\{Math\.round\(width\)\}px`/,
"the centre is still given a ceiling and no floor, so it shrink-wraps");
assert.match(body, /centre\.style\.maxWidth = `\$\{Math\.round\(width\)\}px`/);
});
// Both come off together when the board leaves the felt, or the centre would keep a
// width measured for a board that is no longer in it.
test("and both are given back when the board moves to the strip", () => {
const render = src("render");
const at = render.indexOf("placeBoard(row, strip) {");
const body = render.slice(at, render.indexOf("} else {", at));
assert.match(body, /centre\.style\.width = ""/);
assert.match(body, /centre\.style\.maxWidth = ""/);
});
// The floor is what it always said it was; what changed is that the cards can now
// reach past it to the ceiling.
test("a card is still allowed to grow to a thumb", () => {
const rule = ruleFor(".boardCard");
assert.match(rule, /max-width:\s*44px/);
assert.match(rule, /min-height:\s*44px/);
assert.match(rule, /flex:\s*1 1 0/);
});
test("what it measured is written beside the code", () => {
const render = src("render");
const at = render.indexOf("// Both, and the width is the half that was missing.");
assert.ok(at >= 0, "the fix carries no note saying why");
const note = render.slice(at, at + 1400);
assert.match(note, /463px/);
assert.match(note, /24px/);
});
});
describe("why the sweep could not see either of them", () => {
// Three selectors: the bar, the header and the seats. Nothing else on the page had
// ever been asked whether something was sitting on top of it.
test("the occlusion check looks at every control now, not three groups", () => {
const at = AUDIT.indexOf("occlusion(selector = null) {");
assert.ok(at >= 0, "occlusion still hard-codes its own list");
assert.match(AUDIT.slice(at, at + 200), /selector = selector \|\| this\.CONTROL_SELECTOR/);
});
// Widening it put three kinds of non-fault in front of the rule for the first time.
test("a control that opted out of the pointer is not reported as covered", () => {
const at = AUDIT.indexOf("occlusion(selector = null) {");
const body = AUDIT.slice(at, AUDIT.indexOf("\n },", at));
assert.match(body, /getComputedStyle\(el\)\.pointerEvents === "none"/);
});
test("nor is one inside a disclosure nobody has opened", () => {
const at = AUDIT.indexOf("occlusion(selector = null) {");
const body = AUDIT.slice(at, AUDIT.indexOf("\n },", at));
assert.match(body, /el\.checkVisibility/,
"a shut <details> still hands out a real rect, and a size check lets it through");
});
test("and flow content under a fixed layer is said rather than failed", () => {
const at = AUDIT.indexOf("occlusion(selector = null) {");
const body = AUDIT.slice(at, AUDIT.indexOf("\n },", at));
assert.match(body, /scrolls clear of it/);
});
});
describe("the rest of what the sweep had never run", () => {
// It opened five sheets and asked one question of them.
test("an overlay gets the whole audit, not the occlusion check alone", () => {
const at = DRIVER.indexOf("for (const [name, open] of OVERLAYS) {");
assert.ok(at >= 0);
const body = DRIVER.slice(at, DRIVER.indexOf("record(`${name}", at));
assert.match(body, /LayoutAudit\.runAll\(/);
assert.ok(!/return LayoutAudit\.occlusion\(/.test(body),
"the overlay pass still runs one rule");
});
// Two of the rules that made running only one of them reasonable ask about the
// fixed bar, which is not a question while a scrim is covering the page.
test("the two questions about the fixed bar stand down while a sheet is open", () => {
["focusHasRoom() {", "furnitureBudget() {"].forEach(signature => {
const at = AUDIT.indexOf(signature);
assert.ok(at >= 0, `there is no \`${signature}\``);
const body = AUDIT.slice(at, AUDIT.indexOf("\n },", at));
assert.match(body, /if \(this\.modalScope\(\)\) \{/,
`${signature} still reads a full-screen scrim as furniture`);
});
});
// And the third called the bottom four rows of a sheet that scrolls unreachable.
test("a dialog's buttons below the fold are only a fault if nothing scrolls to them", () => {
const at = AUDIT.indexOf("dialogActionsReachable() {");
const body = AUDIT.slice(at, AUDIT.indexOf("\n },", at));
assert.match(body, /if \(this\.reachableByScrolling\(button\)\) return;/);
});
// The helper all three of those lean on had a blind spot of its own: it asked
// whether the element had left the box entirely, which is true of something well
// below the fold and false of something hanging eight pixels over the edge.
test("and reachability counts a control that is only part-way out", () => {
const at = AUDIT.indexOf("reachableByScrolling(el) {");
const body = AUDIT.slice(at, AUDIT.indexOf("\n },", at));
assert.match(body, /const below = rect\.bottom - box\.bottom/);
assert.match(body, /const above = box\.top - rect\.top/);
// The comment above the fix quotes the old test, so the code is read on its own.
const code = body.split("\n").filter(line => !/^\s*\/\//.test(line)).join("\n");
assert.ok(!/rect\.top >= box\.bottom/.test(code),
"it still asks whether the element has left the box entirely");
});
});
describe("two controls that were a glyph and nothing else", () => {
test("the pot wizard's close is named for what leaving does", () => {
const markup = fs.readFileSync(path.join(ROOT, "Pages", "Casino", "TexasHoldem.cshtml"), "utf8");
const at = markup.indexOf('id="closePotWizardBtn"');
assert.ok(at >= 0);
assert.match(markup.slice(at, markup.indexOf(">", at)), /aria-label="Close without paying the pot"/);
});
// Named from the label the dealer typed, and kept up with it — the same shape the
// chip legend's remove button already used, in a column of identical crosses.
test("and a custom field's remove says which field", () => {
const render = src("render");
const at = render.indexOf("renderCustomFields(form, player, index) {");
const body = render.slice(at, render.indexOf("\n },", at));
assert.match(body, /removeBtn\.setAttribute\("aria-label"/);
assert.match(body, /labelInput\.value\.trim\(\)/);
assert.match(body, /nameRemove\(\);/);
});
});
+90 -8
View File
@@ -143,6 +143,15 @@ const LayoutAudit = {
// and this file has no automation of its own: scroll-padding-bottom on the scroll
// container has to reserve at least as much room as the furniture standing there.
focusHasRoom() {
// Not a question that means anything while a sheet is open. The page behind is
// inert, and the sheet's own scrim is `position: fixed; inset: 0` — bottom-anchored
// and full height — so this read it as furniture standing 100% of the window tall
// and demanded scroll padding for all of it. Every overlay failed, every viewport.
if (this.modalScope()) {
return { name: "focus has room under the fixed bar", offenders: [],
skipped: "a sheet is open — the page behind it is not being tabbed" };
}
const furniture = this.bottomFurniture();
if (furniture.length === 0) {
return { name: "focus has room under the fixed bar", offenders: [],
@@ -182,6 +191,13 @@ const LayoutAudit = {
// stops being read.
furnitureBudget() {
const name = "the fixed bar leaves the table its room";
// Same reason as focusHasRoom above: a scrim is not furniture, it is the whole
// point of the thing on top of it, and "the felt has less room than the furniture"
// is not a complaint anyone can act on while a sheet is deliberately covering it.
if (this.modalScope()) {
return { name, offenders: [], skipped: "a sheet is open — it is meant to cover the table" };
}
const furniture = this.bottomFurniture();
if (furniture.length === 0) {
return { name, offenders: [], skipped: "nothing is fixed to the bottom here" };
@@ -322,11 +338,25 @@ const LayoutAudit = {
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
const down = 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;
const up = node === document.body ? scrollY : node.scrollTop;
// How far past each edge the element sits, and whether there is that much
// scrolling left in this direction. Partly past counts.
//
// This used to ask whether the element had left the box *entirely* —
// `rect.top >= box.bottom` — which is true of something well below the fold
// and false of something hanging eight pixels over the edge of it. So a card
// half-visible at the bottom of a sheet with 254px of scroll still in it came
// back "cannot be scrolled to", and the row above it, fully out, came back
// fine. Verified in a browser before believing either: the sheet scrolls, the
// two of clubs comes fully into view and hit-tests as itself.
const below = rect.bottom - box.bottom;
const above = box.top - rect.top;
if (below > 0.5 && down >= below - 0.5) return true;
if (above > 0.5 && up >= above - 0.5) return true;
}
return false;
},
@@ -374,7 +404,15 @@ const LayoutAudit = {
return null;
},
occlusion(selector = "#actionBar button, #holdemHeader button, .seat") {
// Everything a dealer can press, not three selectors' worth.
//
// This asked about the action bar, the header and the seats, and nothing else on the
// page had ever been checked for being covered — which is how the toast came to sit
// on 98% of "Common rulings" and 98% of "Quick Split" on every phone, every hand,
// for eight seconds. Both are controls, both sit below the felt, and neither was in
// any of the three groups, so the sweep never looked.
occlusion(selector = null) {
selector = selector || this.CONTROL_SELECTOR;
const offenders = [];
const warnings = [];
// While something is open on top, only what is inside it can be judged. The
@@ -386,6 +424,21 @@ const LayoutAudit = {
const rect = el.getBoundingClientRect();
if (rect.width === 0 || rect.height === 0) return;
// Inside a disclosure nobody has opened. A shut `<details>` still hands out
// a real rect for everything in it — the rake fields measure 54x28 at a
// sensible place on the page while House take is closed — so a size check
// alone lets them through and they hit-test to the next summary along.
// `checkVisibility` is the browser's own answer to the same question.
if (typeof el.checkVisibility === "function" &&
!el.checkVisibility({ checkOpacity: false, checkVisibilityCSS: true })) return;
// A control that has opted out of the pointer is not being covered — it is
// a marker that happens to be a button, and the hit test lands on whatever
// is behind it because that is what it asked for. The dealer button on the
// seat that holds it is exactly this: `pointer-events: none`, deliberately,
// because giving the button to the seat that has it is not a thing to do.
if (getComputedStyle(el).pointerEvents === "none") return;
const outsideViewport = rect.bottom <= 0 || rect.top >= innerHeight ||
rect.right <= 0 || rect.left >= innerWidth;
if (outsideViewport) {
@@ -423,6 +476,7 @@ const LayoutAudit = {
let blocked = 0;
let testable = 0;
let blockedBy = null;
let blocker = null;
points.forEach(([x, y]) => {
const top = document.elementFromPoint(x, y);
// A null result means the point isn't hit-testable — typically it sits
@@ -448,7 +502,7 @@ const LayoutAudit = {
if (!isControl) return;
}
blocked++;
if (!blockedBy) blockedBy = this.describe(top);
if (!blockedBy) { blockedBy = this.describe(top); blocker = top; }
});
if (testable === 0) {
@@ -468,8 +522,29 @@ const LayoutAudit = {
// 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);
if (transient && blocked < testable) { warnings.push(note); return; }
// Content in the flow, under something fixed, on a page that scrolls.
//
// The fixed thing stays where it is and the content moves, so this is a
// position rather than a state: scroll, and the control comes out from
// under. Worth saying — a dealer reaching for Common rulings while a toast
// is up finds a toast — but it is not the same fault as a control that no
// amount of scrolling will free, which is what the rest of this reports.
// Widening the check from three selectors to every control put forty of
// these in front of a rule that had only ever been shown the felt.
const pinned = node => {
for (let up = node; up && up !== document.body; up = up.parentElement) {
if (getComputedStyle(up).position === "fixed") return true;
}
return false;
};
const canScroll = document.documentElement.scrollHeight > innerHeight + 1;
if (blockedBy && !pinned(el) && pinned(blocker) && canScroll) {
warnings.push(`${note} — scrolls clear of it`);
return;
}
offenders.push(note);
});
return { name: "nothing important is covered", offenders, warnings };
},
@@ -1576,10 +1651,17 @@ const LayoutAudit = {
const r = button.getBoundingClientRect();
const label = this.describe(button);
if (r.bottom > innerHeight || r.top < 0) {
// Below the fold is only a fault if it is also out of reach. A sheet
// that scrolls is allowed to be taller than the window — the card
// picker is fifty-two cards and always will be, and this reported the
// bottom four rows of it as unreachable buttons on every phone there
// is. What the rule is actually for is a dialog whose Confirm has
// been pushed off a short screen with no way to get to it.
if (this.reachableByScrolling(button)) return;
const off = r.bottom > innerHeight
? Math.round(r.bottom - innerHeight) + "px below the fold"
: Math.round(-r.top) + "px above it";
offenders.push(`${label} is ${off}`);
offenders.push(`${label} is ${off}, and nothing scrolls to it`);
return;
}
const hit = document.elementFromPoint(r.left + r.width / 2, r.top + r.height / 2);