diff --git a/JoshHeaps.Net/wwwroot/js/casino/holdem/shortcuts.js b/JoshHeaps.Net/wwwroot/js/casino/holdem/shortcuts.js index 2a767b0..4843a64 100644 --- a/JoshHeaps.Net/wwwroot/js/casino/holdem/shortcuts.js +++ b/JoshHeaps.Net/wwwroot/js/casino/holdem/shortcuts.js @@ -40,6 +40,22 @@ const Shortcuts = { return [...document.querySelectorAll('[id$="Overlay"]')].some(el => !el.hidden); }, + // Enter and Space are how a keyboard presses the control it is already sitting on. + // Claiming them for "advance" didn't just block that press — it ran a different + // action instead. Measured: Tab to Resolve Pot, press Enter, and the table dealt + // the next hand. Space on the same button worked, because "advance" happened to + // decline that time and the browser's own activation got through. A control that + // does something else every other press is worse than one that does nothing. + // + // Only these two keys are handed back. The letter shortcuts stay live with a + // button focused, because a button does nothing with "f" and the dealer's hand + // shouldn't have to leave them. + focusedControlOwnsKey(key) { + if (key !== "enter" && key !== " ") return false; + const el = document.activeElement; + return !!(el && el.matches && el.matches('button, [role="button"], a[href], summary')); + }, + onKey(e) { if (e.metaKey || e.ctrlKey || e.altKey) return; @@ -47,6 +63,8 @@ const Shortcuts = { const action = this.KEYS[key]; if (!action) return; + if (this.focusedControlOwnsKey(key)) return; + // Escape is the exception: it should still close the raise pad even if focus // happens to be sitting in its amount field. if (this.isTyping() && !(action === "cancel")) return; diff --git a/tests/holdem/shortcuts.test.js b/tests/holdem/shortcuts.test.js new file mode 100644 index 0000000..931db39 --- /dev/null +++ b/tests/holdem/shortcuts.test.js @@ -0,0 +1,110 @@ +// The keyboard shortcuts, and the one rule that keeps them from stealing a press. +// +// Shortcuts talks to `document` directly, so these tests stand a small fake one up +// rather than a whole DOM: the only things it asks about are what has focus and +// whether an overlay is open. +const test = require("node:test"); +const assert = require("node:assert"); +const path = require("path"); + +const Shortcuts = require(path.join( + __dirname, "..", "..", "JoshHeaps.Net", "wwwroot", "js", "casino", "holdem", "shortcuts.js")); + +// A stand-in for a focused element. `matches` answers the same selector list the +// real one would, which is the whole of what focusedControlOwnsKey looks at. +function element(tagName, { selectors = [], isContentEditable = false } = {}) { + return { + tagName, + isContentEditable, + matches: selector => selectors.some(s => selector.includes(s)), + }; +} + +const BUTTON = element("BUTTON", { selectors: ["button"] }); +const SUMMARY = element("SUMMARY", { selectors: ["summary"] }); +const LINK = element("A", { selectors: ["a[href]"] }); +const DIV = element("DIV"); +const TEXT_FIELD = element("INPUT"); + +// Runs one keypress against a fresh Shortcuts and reports which action fired. +function press(key, { focused = DIV, overlays = [] } = {}) { + const fired = []; + let defaultPrevented = false; + + globalThis.document = { + activeElement: focused, + addEventListener() {}, + querySelectorAll: () => overlays, + }; + + const handlers = {}; + for (const action of Object.values(Shortcuts.KEYS)) { + handlers[action] = () => { fired.push(action); return true; }; + } + Shortcuts.handlers = handlers; + Shortcuts.onKey({ key, preventDefault: () => { defaultPrevented = true; } }); + + return { fired: fired[0] || null, defaultPrevented }; +} + +test.after(() => { delete globalThis.document; }); + +test("Enter on a focused button is left to the button", () => { + assert.equal(press("Enter", { focused: BUTTON }).fired, null); +}); + +test("Space on a focused button is left to the button", () => { + assert.equal(press(" ", { focused: BUTTON }).fired, null); +}); + +test("a key left to the control is not swallowed either", () => { + // preventDefault would stop the browser activating the button, which is the + // half of the bug that turned "wrong action" into "no action". + assert.equal(press("Enter", { focused: BUTTON }).defaultPrevented, false); +}); + +test("summary and links own Enter and Space too", () => { + assert.equal(press("Enter", { focused: SUMMARY }).fired, null); + assert.equal(press(" ", { focused: LINK }).fired, null); +}); + +test("Space still advances when nothing pressable has focus", () => { + assert.equal(press(" ", { focused: DIV }).fired, "advance"); +}); + +test("Enter still advances when nothing pressable has focus", () => { + assert.equal(press("Enter", { focused: DIV }).fired, "advance"); +}); + +test("the letter shortcuts stay live with a button focused", () => { + // A button does nothing with "f", so there is no press to hand back — and a + // dealer whose hand is on the keys shouldn't lose them to a stray focus ring. + assert.equal(press("f", { focused: BUTTON }).fired, "fold"); + assert.equal(press("c", { focused: BUTTON }).fired, "checkOrCall"); + assert.equal(press("r", { focused: BUTTON }).fired, "raise"); + assert.equal(press("a", { focused: BUTTON }).fired, "allIn"); + assert.equal(press("u", { focused: BUTTON }).fired, "undo"); +}); + +test("typing still silences everything but Escape", () => { + assert.equal(press("f", { focused: TEXT_FIELD }).fired, null); + assert.equal(press("Escape", { focused: TEXT_FIELD }).fired, "cancel"); +}); + +test("an open overlay still owns the keyboard", () => { + assert.equal(press("f", { focused: DIV, overlays: [{ hidden: false }] }).fired, null); + assert.equal(press("f", { focused: DIV, overlays: [{ hidden: true }] }).fired, "fold"); +}); + +test("a focused control wins before the overlay check, not after", () => { + // Both would decline here; the point is that neither path fires an action. + assert.equal(press("Enter", { focused: BUTTON, overlays: [{ hidden: false }] }).fired, null); +}); + +test("modified keys are never shortcuts", () => { + globalThis.document = { activeElement: DIV, addEventListener() {}, querySelectorAll: () => [] }; + const fired = []; + Shortcuts.handlers = { advance: () => { fired.push("advance"); return true; } }; + Shortcuts.onKey({ key: " ", ctrlKey: true, preventDefault() {} }); + assert.deepEqual(fired, []); +});