Stop the table being drawn from a measurement of a bar that isn't there
The felt is sized from --feltRoomPx, which actionBar.js publishes from the bar's real height, because the bar is fixed to the bottom and grows with the hand so nothing else can know how much window is left. That measurement is right. What it was measuring was not. ActionBar.mount() called watchHeight() before render(), and the bar ships `<div id="actionBar" hidden>`. So the first publish measured a bar that was not there yet and published 0px — not "I don't know" but "the bar takes none of the window", which is the one answer that makes the felt draw itself too big. Traced on a cold load, nine-handed: t=181ms --actionBarHeight 0px bar hidden, 0 tall t=186ms --feltRoomPx 721px t=1056ms --actionBarHeight 151px bar visible, 151 tall — ResizeObserver t=1059ms --feltRoomPx 570px The observer does come round on its own once `hidden` comes off, but a frame later at best, and with nine seats to lay out it was 875ms behind. What a dealer saw at 360x640 was a table drawn 336x504 and collapsing to 336x374 after it was on screen and readable: 130px of felt with every seat on it. 1280x900 lost 13px the same way. Two rules, separate on purpose. publish() now declines rather than guessing while the bar is hidden and not a rail — a rail still publishes its honest zero, because it is a real bar that has moved out of the way. And mount() publishes once render() has given it something to measure. That leaves the frame before any of it, which is real: the scripts are at the end of the body and the table is laid out inside DOMContentLoaded, so the browser has painted by the time --feltRoomPx exists. The fallback there was `100vh` — the worst guess available, since it says the whole window is free. It is now the same guess two other rules in this file already make, 9rem for the bar plus 7rem for what sits above and below, which lands 10px over the truth at 360x640; and it is a ceiling, so over is a small shrink rather than a seat behind the bar. Measured after, on both phones: one draw, no jump, and a layout-shift score of 0 where the felt used to move 130px. 1362 tests pass, 8 of them new — 5 fail against the old code. One existing test pinned the literal fallback string; its claim is about the cap being the measured room, and it now says that. The sweep is clean across 428 audits; contrast unchanged. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_015bnp7xKUPFFSpwkYNFaPgb
This commit is contained in:
@@ -5199,10 +5199,23 @@ body.hasActionBar #toastHost {
|
|||||||
the worst any of these phones actually publishes is 142, which lays out fine.
|
the worst any of these phones actually publishes is 142, which lays out fine.
|
||||||
|
|
||||||
Landscape overrides all of this outright, further down the file. */
|
Landscape overrides all of this outright, further down the file. */
|
||||||
|
/* The fallback is what the felt is drawn at before actionBar.js has measured
|
||||||
|
anything, which is a real frame a dealer sees — the scripts run at the end of the
|
||||||
|
body and the table is laid out inside main.js's DOMContentLoaded, so the browser
|
||||||
|
has already painted by the time --feltRoomPx exists.
|
||||||
|
|
||||||
|
`100vh` was the worst guess available: it says the whole window is free. Measured
|
||||||
|
at 360x640 nine-handed, the felt came up 336x504 and dropped to 336x374 as soon as
|
||||||
|
the bar was measured — 130px of table, with every seat on it, moving after it was
|
||||||
|
readable. This is a guess too, but a near one: 9rem is what two other rules in
|
||||||
|
this file already assume a bar costs before it can be asked, and 7rem is what sits
|
||||||
|
above and below the felt. It lands 10px over the truth at 360x640, and since this
|
||||||
|
is a ceiling rather than a height, over is a small shrink rather than a seat
|
||||||
|
behind the bar. Anything measured replaces it on the next frame. */
|
||||||
#pokerTableWrap {
|
#pokerTableWrap {
|
||||||
--feltAspect: 1.5;
|
--feltAspect: 1.5;
|
||||||
max-width: none;
|
max-width: none;
|
||||||
max-height: var(--feltRoomPx, 100vh);
|
max-height: var(--feltRoomPx, calc(100vh - var(--actionBarHeight, 9rem) - 7rem));
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Stacked, with the action that ends the hand furthest from a stray thumb. */
|
/* Stacked, with the action that ends the hand furthest from a stray thumb. */
|
||||||
|
|||||||
@@ -16,8 +16,14 @@ const ActionBar = {
|
|||||||
state.actingSeatIndex !== null && state.actingSeatIndex !== undefined
|
state.actingSeatIndex !== null && state.actingSeatIndex !== undefined
|
||||||
? state.actingSeatIndex
|
? state.actingSeatIndex
|
||||||
: null;
|
: null;
|
||||||
|
// watchHeight first, because render() may reach for publishHeight; then publish
|
||||||
|
// once there is a bar to measure. The ResizeObserver does come round on its own
|
||||||
|
// when the markup's `hidden` comes off — but it arrives a frame later at best,
|
||||||
|
// and measured on a cold load with nine seats to lay out it was 875ms behind.
|
||||||
|
// The felt is drawn inside that gap.
|
||||||
this.watchHeight();
|
this.watchHeight();
|
||||||
this.render();
|
this.render();
|
||||||
|
this.publishHeight();
|
||||||
},
|
},
|
||||||
|
|
||||||
// The bar is fixed to the bottom of the screen, so everything else has to know how
|
// The bar is fixed to the bottom of the screen, so everything else has to know how
|
||||||
@@ -31,13 +37,29 @@ const ActionBar = {
|
|||||||
if (!host || this.heightWatcher) return;
|
if (!host || this.heightWatcher) return;
|
||||||
|
|
||||||
const publish = () => {
|
const publish = () => {
|
||||||
const visible = !host.hidden;
|
|
||||||
const rail = this.isRail(host);
|
const rail = this.isRail(host);
|
||||||
|
// A bar nobody has drawn yet is not a bar of zero height. It is a bar whose
|
||||||
|
// height nobody knows, and saying zero is answering the question wrongly
|
||||||
|
// rather than declining it: the felt reads this to decide how tall to be,
|
||||||
|
// and zero tells it the whole window is free.
|
||||||
|
//
|
||||||
|
// It is the state this bar starts in — `<div id="actionBar" hidden>` in the
|
||||||
|
// markup, taken out by the first render() — and watchHeight() used to
|
||||||
|
// publish from inside that window, because mount() called it before render.
|
||||||
|
// Measured at 360x640 nine-handed: the table came up 336x504 against a
|
||||||
|
// published 525px of room, then collapsed to 336x374 sixty milliseconds
|
||||||
|
// later when the real 151px bar was finally measured. 1280x900 lost 13px
|
||||||
|
// the same way. A table that is drawn, read, and then redrawn at a
|
||||||
|
// different size is the layout shift this measurement exists to prevent.
|
||||||
|
//
|
||||||
|
// A rail is different, and still publishes: it is a real bar that genuinely
|
||||||
|
// takes none of the bottom, because it is off to the side.
|
||||||
|
if (host.hidden && !rail) return;
|
||||||
// On a phone held sideways the bar is a rail down the right-hand side, so it
|
// On a phone held sideways the bar is a rail down the right-hand side, so it
|
||||||
// takes no height at all — and `--actionBarHeight` is what four other rules
|
// takes no height at all — and `--actionBarHeight` is what four other rules
|
||||||
// and the felt measurement all mean by "how much of the bottom is spoken
|
// and the felt measurement all mean by "how much of the bottom is spoken
|
||||||
// for". Publishing the rail's height there would reserve the whole window.
|
// for". Publishing the rail's height there would reserve the whole window.
|
||||||
const height = visible && !rail ? host.getBoundingClientRect().height : 0;
|
const height = rail ? 0 : host.getBoundingClientRect().height;
|
||||||
document.documentElement.style.setProperty("--actionBarHeight", `${Math.round(height)}px`);
|
document.documentElement.style.setProperty("--actionBarHeight", `${Math.round(height)}px`);
|
||||||
// What is under the felt is the bar in one layout and the two links in the
|
// What is under the felt is the bar in one layout and the two links in the
|
||||||
// other. Either way it is the same question — how much of the bottom is
|
// other. Either way it is the same question — how much of the bottom is
|
||||||
|
|||||||
@@ -140,7 +140,18 @@ describe("what stops the oval", () => {
|
|||||||
// The oval is a preference and the room is a fact. Whatever shape is asked for
|
// The oval is a preference and the room is a fact. Whatever shape is asked for
|
||||||
// above, the felt gives way to the measurement rather than the other way round,
|
// above, the felt gives way to the measurement rather than the other way round,
|
||||||
// because the thing on the other side of the cap is a seat behind the action bar.
|
// because the thing on the other side of the cap is a seat behind the action bar.
|
||||||
assert.equal(phoneRule, "var(--feltRoomPx, 100vh)");
|
assert.match(phoneRule, /^var\(--feltRoomPx,/,
|
||||||
|
"something other than the measured room decides how tall the felt may be");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("and what it falls back on is a near guess rather than the whole window", () => {
|
||||||
|
// The fallback is only ever used for the frame before actionBar.js has measured
|
||||||
|
// anything — but that frame is one a dealer sees, because the scripts run at the
|
||||||
|
// end of the body. `100vh` was what stood here, and it says the whole window is
|
||||||
|
// free: measured at 360x640 nine-handed, the table came up 336x504 and dropped to
|
||||||
|
// 336x374 as soon as the bar was measured. See whatTheFeltIsToldFirst.
|
||||||
|
assert.doesNotMatch(phoneRule, /var\(--feltRoomPx,\s*100vh\)/);
|
||||||
|
assert.match(phoneRule, /var\(--actionBarHeight, 9rem\)/);
|
||||||
});
|
});
|
||||||
|
|
||||||
test("and it is not floored, however tempting that is", () => {
|
test("and it is not floored, however tempting that is", () => {
|
||||||
|
|||||||
@@ -0,0 +1,173 @@
|
|||||||
|
// The size the table is drawn at before anything has been measured.
|
||||||
|
//
|
||||||
|
// The felt is not sized by CSS alone. It is sized from `--feltRoomPx`, which actionBar.js
|
||||||
|
// publishes from the bar's real height — because the bar is fixed to the bottom of the
|
||||||
|
// screen and grows with the hand, so nothing else can know how much of the window is
|
||||||
|
// left. That measurement is right. What it was measuring was not.
|
||||||
|
//
|
||||||
|
// `ActionBar.mount()` called `watchHeight()` before `render()`, and the bar ships
|
||||||
|
// `<div id="actionBar" hidden>` in the markup. So the first publish measured a bar that
|
||||||
|
// was not there yet and published `0px` — not "I don't know", but "the bar takes none of
|
||||||
|
// the window", which is the one answer that makes the felt draw itself too big.
|
||||||
|
//
|
||||||
|
// Measured on a cold load, nine-handed:
|
||||||
|
//
|
||||||
|
// 360x640 --feltRoomPx 525px -> felt 336x504, then 336x374 at t=300ms
|
||||||
|
// 1280x900 --feltRoomPx 755px -> felt 857x617, then 839x604
|
||||||
|
//
|
||||||
|
// A hundred and thirty pixels of table, with every seat on it, moving after it was on
|
||||||
|
// screen and readable. The ResizeObserver does come round on its own once the markup's
|
||||||
|
// `hidden` comes off — but a frame later at best, and measured with nine seats to lay
|
||||||
|
// out it was 875ms behind.
|
||||||
|
//
|
||||||
|
// Two rules hold it shut, and they are separate on purpose: one stops the wrong number
|
||||||
|
// being published at all, the other makes sure a right one is published straight away.
|
||||||
|
const test = require("node:test");
|
||||||
|
const { describe } = require("node:test");
|
||||||
|
const assert = require("node:assert");
|
||||||
|
const fs = require("fs");
|
||||||
|
const path = require("path");
|
||||||
|
const { ActionBar } = require("./load");
|
||||||
|
|
||||||
|
const SRC = path.join(__dirname, "..", "..", "JoshHeaps.Net", "wwwroot", "js", "casino", "holdem");
|
||||||
|
const BAR = fs.readFileSync(path.join(SRC, "actionBar.js"), "utf8");
|
||||||
|
const CSS = fs.readFileSync(
|
||||||
|
path.join(__dirname, "..", "..", "JoshHeaps.Net", "wwwroot", "css", "casino", "holdem.css"), "utf8");
|
||||||
|
|
||||||
|
// A document with one action bar in it, and a note of everything published.
|
||||||
|
const pageWith = ({ hidden, height, layout }) => {
|
||||||
|
const published = [];
|
||||||
|
const host = {
|
||||||
|
hidden,
|
||||||
|
getBoundingClientRect: () => ({ height, top: 0 }),
|
||||||
|
};
|
||||||
|
global.document = {
|
||||||
|
getElementById: id => (id === "actionBar" ? host : null),
|
||||||
|
documentElement: {
|
||||||
|
style: {
|
||||||
|
setProperty: (name, value) => published.push([name, value]),
|
||||||
|
getPropertyValue: () => "",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
global.getComputedStyle = () => ({ getPropertyValue: () => layout });
|
||||||
|
global.window = { innerHeight: 844, scrollY: 0, addEventListener: () => {} };
|
||||||
|
return { host, published, barHeight: () => published.filter(p => p[0] === "--actionBarHeight") };
|
||||||
|
};
|
||||||
|
|
||||||
|
// watchHeight wires listeners and publishes once. The felt half of it is stubbed: it
|
||||||
|
// reads a wrap that is not in this document, and what is under test here is the number
|
||||||
|
// the bar decides to hand over.
|
||||||
|
const publishOnce = page => {
|
||||||
|
const realFelt = ActionBar.publishFeltRoom;
|
||||||
|
const roomAsked = [];
|
||||||
|
ActionBar.publishFeltRoom = value => roomAsked.push(value);
|
||||||
|
ActionBar.heightWatcher = null;
|
||||||
|
try {
|
||||||
|
ActionBar.watchHeight();
|
||||||
|
} finally {
|
||||||
|
ActionBar.publishFeltRoom = realFelt;
|
||||||
|
}
|
||||||
|
return roomAsked;
|
||||||
|
};
|
||||||
|
|
||||||
|
test.afterEach(() => {
|
||||||
|
delete global.document;
|
||||||
|
delete global.getComputedStyle;
|
||||||
|
delete global.window;
|
||||||
|
ActionBar.heightWatcher = null;
|
||||||
|
delete ActionBar.publishHeight;
|
||||||
|
});
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
describe("a bar nobody has drawn yet", () => {
|
||||||
|
test("says nothing, rather than saying nothing is there", () => {
|
||||||
|
// The distinction the whole fix rests on. Zero is an answer; "not yet" is not,
|
||||||
|
// and publishing the first as though it were the second is what made the felt
|
||||||
|
// believe it had the whole window.
|
||||||
|
const page = pageWith({ hidden: true, height: 0, layout: "bar" });
|
||||||
|
publishOnce(page);
|
||||||
|
|
||||||
|
assert.deepEqual(page.barHeight(), [],
|
||||||
|
"a hidden bar published a height, and the felt will be drawn against it");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("and the felt is not asked to resize on the strength of it either", () => {
|
||||||
|
const page = pageWith({ hidden: true, height: 0, layout: "bar" });
|
||||||
|
const roomAsked = publishOnce(page);
|
||||||
|
|
||||||
|
assert.deepEqual(roomAsked, [], "the felt was handed a number measured off nothing");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("a bar that has been drawn publishes what it measures", () => {
|
||||||
|
const page = pageWith({ hidden: false, height: 151, layout: "bar" });
|
||||||
|
publishOnce(page);
|
||||||
|
|
||||||
|
assert.deepEqual(page.barHeight(), [["--actionBarHeight", "151px"]]);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("a rail is not an undrawn bar", () => {
|
||||||
|
// A phone held sideways puts the bar down the right-hand side, where it genuinely
|
||||||
|
// takes none of the bottom of the window. That zero is a measurement and has to keep
|
||||||
|
// being published — otherwise the felt goes on reserving room for a bar that has
|
||||||
|
// moved out of its way.
|
||||||
|
test("it publishes zero, and it does so even while hidden", () => {
|
||||||
|
const page = pageWith({ hidden: true, height: 0, layout: "rail" });
|
||||||
|
publishOnce(page);
|
||||||
|
|
||||||
|
assert.deepEqual(page.barHeight(), [["--actionBarHeight", "0px"]],
|
||||||
|
"a rail stopped saying it takes no height");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("and a drawn rail publishes zero rather than its own height", () => {
|
||||||
|
const page = pageWith({ hidden: false, height: 390, layout: "rail" });
|
||||||
|
publishOnce(page);
|
||||||
|
|
||||||
|
assert.deepEqual(page.barHeight(), [["--actionBarHeight", "0px"]],
|
||||||
|
"the rail's height went into the number that means 'how much of the bottom is gone'");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("and something is published as soon as there is a bar to measure", () => {
|
||||||
|
test("mount publishes after it has rendered, not only before", () => {
|
||||||
|
// watchHeight still goes first, because render() may reach for publishHeight.
|
||||||
|
// The publish that matters is the one after.
|
||||||
|
const mount = BAR.slice(BAR.indexOf("mount(state, hooks) {"), BAR.indexOf("watchHeight() {"));
|
||||||
|
const watch = mount.indexOf("this.watchHeight();");
|
||||||
|
const render = mount.indexOf("this.render();");
|
||||||
|
const publish = mount.indexOf("this.publishHeight();");
|
||||||
|
|
||||||
|
assert.ok(watch >= 0 && render >= 0 && publish >= 0,
|
||||||
|
"mount no longer does all three of these");
|
||||||
|
assert.ok(watch < render, "watchHeight has to be in place before render can call it");
|
||||||
|
assert.ok(render < publish,
|
||||||
|
"mount publishes before the bar has been drawn, which is the whole bug");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("what the felt falls back on before any of that", () => {
|
||||||
|
// The scripts are at the end of the body and the table is laid out inside main.js's
|
||||||
|
// DOMContentLoaded, so there is a real frame where --feltRoomPx does not exist yet.
|
||||||
|
// `100vh` was the worst guess available for it: it says the whole window is free.
|
||||||
|
test("a phone does not assume the whole window", () => {
|
||||||
|
// The phone's rule is the one that caps the felt's height; the wide layout caps
|
||||||
|
// its width instead, from the same number.
|
||||||
|
const found = CSS.match(/max-height:\s*var\(--feltRoomPx,([^)]*\)?[^;]*);/);
|
||||||
|
assert.ok(found, "nothing caps the felt's height by the measured room any more");
|
||||||
|
const fallback = found[1].trim();
|
||||||
|
assert.notEqual(fallback, "100vh",
|
||||||
|
"the fallback claims the whole window, so the first frame draws a table that has to shrink");
|
||||||
|
assert.match(fallback, /var\(--actionBarHeight, 9rem\)/,
|
||||||
|
"9rem is what the rest of this file assumes a bar costs before it can be asked");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("and 9rem is a number this file already stands behind", () => {
|
||||||
|
// Two other rules had reached the same conclusion before this one did. If they
|
||||||
|
// ever disagree, the felt's first frame and the toast's first frame disagree.
|
||||||
|
const nines = CSS.match(/var\(--actionBarHeight, 9rem\)/g) || [];
|
||||||
|
assert.ok(nines.length >= 3,
|
||||||
|
`only ${nines.length} rules assume 9rem; the felt's guess should be the same guess as the rest`);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user