diff --git a/web/scripts/signin-form.test.mjs b/web/scripts/signin-form.test.mjs index a776cae..df71793 100644 --- a/web/scripts/signin-form.test.mjs +++ b/web/scripts/signin-form.test.mjs @@ -86,3 +86,39 @@ test("the dialog is named by the form's heading", () => { "the dialog is unnamed; a modal with no name announces only 'dialog'", ); }); + +test("the dialog answers Escape itself, because the browser's own will not", () => { + // wraps the handle field and preventDefaults every Escape + // it sees, and a prevented keydown never becomes the dialog's `cancel`. The + // field is focused the moment the dialog opens, so relying on the browser + // meant Escape did nothing from the one control a player was already in. + // + // Nothing about that is visible from the source of this repo alone — it + // reads as a plain , and plain closes on Escape — so the + // handler is the sort of thing a tidy-up removes. + // The call spans several lines and its own body contains a `);`, so it is + // read to the closing paren at this call's indent rather than by a lazy + // regex — which stops inside the handler. + const start = modal.indexOf('dialog.addEventListener(\n "keydown"'); + assert.notEqual( + start, + -1, + "the dialog no longer handles Escape, and the typeahead eats it", + ); + const end = modal.indexOf("\n );", start); + assert.notEqual(end, -1, "the keydown listener has no end"); + const listener = modal.slice(start, end); + + assert.match( + listener, + /event\.key !== "Escape"/, + "the keydown handler is not about Escape any more", + ); + // Capture. In the bubble phase the typeahead has already had the key. + assert.match( + listener, + /\n {4}true,\s*$/, + "the Escape handler is not in the capture phase, so it runs after the " + + "typeahead has already prevented the key", + ); +}); diff --git a/web/src/signin-modal.ts b/web/src/signin-modal.ts index 79f203a..32f2caf 100644 --- a/web/src/signin-modal.ts +++ b/web/src/signin-modal.ts @@ -9,8 +9,8 @@ * A modal rather than an inline panel: neither caller has room to grow. The * editor is a grid of blocks sized to each other and shifts everything the * player is looking at if one of them gets taller; the masthead is a 3.5rem - * bar. `` also brings focus trapping, Escape and the backdrop for - * free. + * bar. `` brings focus trapping and the backdrop for free. Escape is + * the one it does not — see below. * * Loaded on demand — the form pulls in a handle typeahead that is a chunk of * its own, and nobody who does not click Login should pay for either. @@ -58,7 +58,38 @@ export function openSignIn({ onBeforeConnect }: SignInOptions = {}): void { dialog.remove(); }; cancel.addEventListener("click", close); - // Escape closes a dialog on its own, but leaves the element in the document. + + /** + * Escape, which a dialog would answer on its own if nothing inside it wanted + * the key. + * + * `` wraps the handle field and calls preventDefault() on + * every Escape it sees — unconditionally, to clear a suggestion list it does + * not first check for — and a prevented keydown never becomes the dialog's + * `cancel`. The field is focused the moment this opens, so what a player + * actually got was a modal that ignored Escape from the one control they + * were already in. Tabbing to "Not now" first made it work, which is not + * something anyone would think to try. + * + * Capture, so this runs before the typeahead rather than after it has eaten + * the key. The cost is that Escape with suggestions showing now closes the + * whole dialog instead of dismissing just the list: the component keeps that + * state in a closed shadow root, so there is no way to ask. A modal that + * cannot be dismissed with Escape is the worse of the two, and the list goes + * with it either way. + */ + dialog.addEventListener( + "keydown", + (event) => { + if (event.key !== "Escape") return; + event.preventDefault(); + close(); + }, + true, + ); + + // The backdrop and "Not now" both go through close(); this catches any other + // way the dialog ends, and stops a closed one being left in the document. dialog.addEventListener("close", () => dialog.remove()); dialog.addEventListener("click", (event) => { if (event.target === dialog) close();