Stop a scroll being an edit, and a stack changing without being touched
A focused <input type="number"> is a spinbutton, and a browser lets the mouse wheel drive it. Measured on the running page at 1280x900 with the sidebar open: focus the small blind and scroll, $5 becomes $5.50. Focus a seat's stack and scroll, $500 becomes $501. Both go straight through to localStorage, because these fields save on `input` like everything else here. Nothing is announced and nothing looks wrong. The tool has invented a chip, on a table whose whole purpose is that the chip count is right, and the gesture that did it was a dealer reaching for another panel. The rule is narrow on purpose. Every other way of driving a spinbutton is deliberate -- the arrow keys, the stepper arrows, typing -- so scrolling is the only one taken away. Verified after: the value holds, one ArrowUp still moves it, typing still works, and the page still scrolls. Blur rather than preventDefault. Cancelling the event stops the value moving and stops the page moving with it, which trades a silent bug for a sidebar that mysteriously will not scroll -- a problem the dealer can see and cannot explain, instead of one they cannot see at all. The decision lives in Money as a predicate beside selectsOnFocus, so the rule is testable without a browser and main.js only does the blurring. Two things checked rather than assumed. The tagName guard in the predicate is decoration: deleting it passes every test, because nothing but an <input> carries type="number". That is now said in the comment instead of being left to look load-bearing. And the page's one <select> -- which way the blinds move -- is not vulnerable: focused, hovered and scrolled 300px in Chromium it stays where it was, so it is left out of the rule rather than defended against. 916 tests pass, 6 of them new. Four fault injections into the predicate are caught; a fifth was applied, not caught, and turned out not to be a fault. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_015bnp7xKUPFFSpwkYNFaPgb
This commit is contained in:
@@ -5,6 +5,32 @@ document.addEventListener("DOMContentLoaded", () => {
|
||||
// `hidden` attribute they all already toggle.
|
||||
Modal.mount();
|
||||
|
||||
// ---- A scroll is not an edit ----
|
||||
//
|
||||
// A focused `<input type="number">` is a spinbutton, and a browser lets the mouse
|
||||
// wheel drive it. Every other way of driving one is deliberate: the arrow keys, the
|
||||
// little stepper arrows, typing. Scrolling is the one actuation nobody means — the
|
||||
// dealer is reaching for another panel and the cursor happens to be over a field
|
||||
// they touched a moment ago.
|
||||
//
|
||||
// Measured on the running page, at 1280x900 with the sidebar open. Focus the small
|
||||
// blind, scroll: $5 becomes $5.50. Focus a seat's stack, scroll: $500 becomes $501.
|
||||
// Both are written straight through to the saved table, because these fields save on
|
||||
// `input` like everything else here. Nothing is announced, nothing looks wrong, and
|
||||
// the tool has invented a chip.
|
||||
//
|
||||
// Blur rather than preventDefault. Cancelling the event would stop the value moving
|
||||
// and stop the page moving with it, which turns a silent bug into a sidebar that
|
||||
// mysteriously will not scroll. Dropping focus lets the scroll do what the dealer
|
||||
// asked for and leaves the number alone — and losing focus on a field you have just
|
||||
// scrolled away from is honest rather than surprising.
|
||||
//
|
||||
// Captured at the document, so it covers the fields the seat editor and the chip
|
||||
// legend build on the fly as well as the ones in the markup.
|
||||
document.addEventListener("wheel", event => {
|
||||
if (Money.wheelWouldEdit(event.target, document.activeElement)) event.target.blur();
|
||||
}, { passive: true });
|
||||
|
||||
// Undo works by swapping the whole table back to a snapshot. `state` is handed out
|
||||
// by reference to every module at mount time, so it has to be refilled in place —
|
||||
// reassigning the variable would leave the renderer and the betting bar pointing at
|
||||
|
||||
@@ -156,6 +156,29 @@ const Money = {
|
||||
return type === "text" && String(field.inputMode || "").toLowerCase() === "decimal";
|
||||
},
|
||||
|
||||
// Would a wheel over this field change what it says?
|
||||
//
|
||||
// True only for a focused `<input type="number">`, which a browser treats as a
|
||||
// spinbutton and lets the wheel drive. Every other way of driving one is deliberate
|
||||
// — the arrow keys, the stepper arrows, typing — and all of them are left alone.
|
||||
// Scrolling is the one nobody means: the dealer is reaching for another panel and
|
||||
// the cursor happens to still be over a field they touched a moment ago.
|
||||
//
|
||||
// A predicate rather than a listener so the rule can be tested without a browser,
|
||||
// the same way selectsOnFocus is. main.js does the blurring.
|
||||
wheelWouldEdit(field, activeElement) {
|
||||
// The tag check is belt and braces and known to be so: nothing but an <input>
|
||||
// carries type="number", so the line below already excludes every textarea,
|
||||
// select and button on the page. Deleting it passes every test in this file,
|
||||
// which was checked rather than assumed. It stays for symmetry with
|
||||
// selectsOnFocus above, not because it is load-bearing.
|
||||
if (!field || field.tagName !== "INPUT") return false;
|
||||
if (String(field.type || "").toLowerCase() !== "number") return false;
|
||||
// An unfocused spinbutton ignores the wheel, so there is nothing to defend and
|
||||
// blurring something that isn't focused would be a no-op with a side effect.
|
||||
return activeElement === field;
|
||||
},
|
||||
|
||||
// "+$40.00" / "-$40.00" rather than "$-40.00" — a standings column is read at a
|
||||
// glance, and the sign belongs out front where the eye lands first.
|
||||
formatSigned(amount) {
|
||||
|
||||
@@ -621,6 +621,53 @@ still names whoever didn't turn over; closing it leaves **Showdown — resolve p
|
||||
bar; and pressing it again resumes at the player the dealer stopped on, because the list
|
||||
is computed from the cards on the table rather than remembered from the last attempt.
|
||||
|
||||
## A scroll was an edit, and it reached the saved table
|
||||
|
||||
The worst bugs in this tool are the ones that move money without saying so, and this one
|
||||
was triggered by reaching for another panel.
|
||||
|
||||
A focused `<input type="number">` is a spinbutton, and a browser lets the mouse wheel
|
||||
drive it. Measured on the running page at 1280x900 with the sidebar open:
|
||||
|
||||
| gesture | before | after | saved? |
|
||||
|---|---|---|---|
|
||||
| focus the small blind, scroll up | $5 | **$5.50** | yes |
|
||||
| focus a seat's stack, scroll up | $500 | **$501** | yes |
|
||||
|
||||
Both go straight through to `localStorage`, because these fields save on `input` like
|
||||
everything else here. Nothing is announced, nothing looks wrong, and the tool has
|
||||
invented a chip — on a table where the whole point is that the chip count is right.
|
||||
|
||||
The rule that fixes it is narrow on purpose. Every *other* way of driving a spinbutton is
|
||||
deliberate: the arrow keys, the stepper arrows, typing. Scrolling is the one actuation
|
||||
nobody means, so it is the only one taken away. Verified after the change: the value
|
||||
holds, one ArrowUp still moves it, typing still works, **and the page still scrolls** —
|
||||
0 to 300px, which matters, because the obvious fix does not.
|
||||
|
||||
### Why it blurs instead of cancelling
|
||||
|
||||
`preventDefault()` on the wheel stops the value moving and stops the page moving with it.
|
||||
That turns a silent bug into a sidebar that mysteriously refuses to scroll, which is a
|
||||
worse trade: the dealer now has a problem they can see and can't explain, instead of one
|
||||
they can't see. Dropping focus lets the scroll do exactly what was asked and leaves the
|
||||
number alone.
|
||||
|
||||
### One guard that was checked and found to be decoration
|
||||
|
||||
The predicate starts with `field.tagName !== "INPUT"`. Injecting a fault into it — taking
|
||||
it out entirely — changed nothing: every test still passed, because nothing but an
|
||||
`<input>` carries `type="number"`, so the type check below it already excludes every
|
||||
textarea, select and button on the page.
|
||||
|
||||
That is written into the comment rather than quietly kept. A guard that cannot fail is
|
||||
fine to leave in for symmetry with `selectsOnFocus` beside it; a guard that cannot fail
|
||||
and is *presented* as load-bearing is how the next person wastes an afternoon.
|
||||
|
||||
The page's one `<select>` — which way the blinds move — was the obvious next suspect and
|
||||
is not one: focused, hovered and scrolled 300px in Chromium it stays on "clockwise". So
|
||||
it is left out of the rule rather than defended against, and the check keeps meaning
|
||||
exactly one thing.
|
||||
|
||||
## The motion setting was honoured in the three places it mattered least
|
||||
|
||||
The stylesheet had answered `prefers-reduced-motion` for a while. All three places it did
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
// Which fields replace their contents when a dealer reaches for them.
|
||||
const test = require("node:test");
|
||||
const { describe } = require("node:test");
|
||||
const assert = require("node:assert");
|
||||
const { Money } = require("./load");
|
||||
|
||||
@@ -51,3 +52,68 @@ test("nothing at all is not a field", () => {
|
||||
test("a field with no type stated is a text box, so it does not", () => {
|
||||
assert.equal(Money.selectsOnFocus({ tagName: "INPUT" }), false);
|
||||
});
|
||||
|
||||
// ---- A scroll is not an edit ------------------------------------------------
|
||||
//
|
||||
// A focused `<input type="number">` is a spinbutton, and the browser lets the mouse
|
||||
// wheel drive it. Measured on the running page at 1280x900 with the sidebar open: focus
|
||||
// the small blind and scroll, $5 becomes $5.50. Focus a seat's stack and scroll, $500
|
||||
// becomes $501. Both go straight through to the saved table, because these fields save
|
||||
// on `input` like everything else here.
|
||||
//
|
||||
// Nothing is announced and nothing looks wrong. The tool has invented a chip, and the
|
||||
// gesture that did it was a dealer reaching for another panel.
|
||||
//
|
||||
// The rule is narrow on purpose. Every other way of driving a spinbutton is deliberate —
|
||||
// the arrow keys, the stepper arrows, typing — and none of them is touched. Scrolling is
|
||||
// the one actuation nobody means.
|
||||
describe("a wheel over a field the dealer is in", () => {
|
||||
const box = (type, tag = "INPUT") => ({ tagName: tag, type });
|
||||
|
||||
test("a focused number box would be edited by it", () => {
|
||||
const stack = box("number");
|
||||
assert.equal(Money.wheelWouldEdit(stack, stack), true);
|
||||
});
|
||||
|
||||
test("the same box, not focused, would not", () => {
|
||||
// An unfocused spinbutton ignores the wheel. There is nothing to defend, and
|
||||
// blurring something that is not focused is a no-op with a side effect.
|
||||
const stack = box("number");
|
||||
assert.equal(Money.wheelWouldEdit(stack, null), false);
|
||||
assert.equal(Money.wheelWouldEdit(stack, box("number")), false);
|
||||
});
|
||||
|
||||
test("the amount boxes that already moved off type=number are safe as they are", () => {
|
||||
// Add chips, the raise pad and Quick Split use text + inputmode="decimal", which
|
||||
// is not a spinbutton and cannot be scrolled into a different number.
|
||||
const amount = { tagName: "INPUT", type: "text", inputMode: "decimal" };
|
||||
assert.equal(Money.wheelWouldEdit(amount, amount), false);
|
||||
// Still an amount field for the other rule, though — the two questions are
|
||||
// different and this one must not quietly answer both.
|
||||
assert.equal(Money.selectsOnFocus(amount), true);
|
||||
});
|
||||
|
||||
test("a name, a note and a table are left alone", () => {
|
||||
const name = { tagName: "INPUT", type: "text" };
|
||||
const notes = { tagName: "TEXTAREA", type: "textarea" };
|
||||
assert.equal(Money.wheelWouldEdit(name, name), false);
|
||||
assert.equal(Money.wheelWouldEdit(notes, notes), false);
|
||||
assert.equal(Money.wheelWouldEdit(null, null), false);
|
||||
assert.equal(Money.wheelWouldEdit(undefined, undefined), false);
|
||||
});
|
||||
|
||||
test("the type is read case-insensitively, the way an attribute comes back", () => {
|
||||
const shouty = box("NUMBER");
|
||||
assert.equal(Money.wheelWouldEdit(shouty, shouty), true);
|
||||
});
|
||||
|
||||
test("a select is a spinbutton to nobody", () => {
|
||||
// The page has one <select> — which way the blinds move — and it was the obvious
|
||||
// next suspect. It isn't one: focused, hovered and scrolled 300px in Chromium it
|
||||
// stays on "clockwise", because a select only takes the wheel while its dropdown
|
||||
// is open. Checked rather than assumed, and left out rather than defended
|
||||
// against, so this rule keeps meaning exactly one thing.
|
||||
const select = { tagName: "SELECT", type: "select-one" };
|
||||
assert.equal(Money.wheelWouldEdit(select, select), false);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user