From 882b2637010f63586f88a209b07dcfe247e00c0c Mon Sep 17 00:00:00 2001 From: Coracle-Bot Date: Fri, 18 Sep 2026 18:44:59 +0000 Subject: [PATCH] Stop a closing dialog taking focus back from whatever moved on (#584) --- e2e/ARCHITECTURE.md | 3 +++ e2e/harness/index.ts | 35 ++++++++++++++++++++++++-------- e2e/specs/dms.spec.ts | 4 ++++ src/lib/components/Dialog.svelte | 9 +++++++- 4 files changed, 42 insertions(+), 9 deletions(-) diff --git a/e2e/ARCHITECTURE.md b/e2e/ARCHITECTURE.md index a0e031ec..ec9d17ef 100644 --- a/e2e/ARCHITECTURE.md +++ b/e2e/ARCHITECTURE.md @@ -448,6 +448,9 @@ playwright's own reporting only cover contexts playwright made itself, and the h own, so without that attachment a failure carries the DOM snapshot and nothing the app said — which is unreadable when what failed is the app rendering its error page. +It also attaches `relay-transcript`, what each user said to each relay and what came back. The +console says what the app did with an event, and the transcript says whether it ever had one. + One engine per run. These specs exercise sockets, auth and sync, so running them under three engines adds little coverage. `E2E_BROWSER=webkit pnpm test` runs the whole suite under another one. The container listens on a fixed port and cannot be sharded, so diff --git a/e2e/harness/index.ts b/e2e/harness/index.ts index 6fcf6b01..ec1465db 100644 --- a/e2e/harness/index.ts +++ b/e2e/harness/index.ts @@ -16,7 +16,12 @@ import { mockRelayInfo, } from "./net/http" import type {BlossomOptions, HostingFixtures, RelayInfoOverrides} from "./net/http" -import {assertNoLeaks, installWebSocketRoutes, silenceRelay} from "./net/websocket" +import { + assertNoLeaks, + formatTranscript, + installWebSocketRoutes, + silenceRelay, +} from "./net/websocket" import {watchFaults} from "./faults" import {boot} from "./app/boot" import {injectNip07} from "./app/nip07" @@ -206,7 +211,7 @@ export const test = base.extend({ await zooid.start() await zooid.ensure() - const contexts: BrowserContext[] = [] + const contexts: {name: string; context: BrowserContext}[] = [] const faults = watchFaults() let scenario: Maybe @@ -233,7 +238,7 @@ export const test = base.extend({ ...options.context, }) - contexts.push(context) + contexts.push({name: user?.name ?? "anonymous", context}) // `use.trace` and the built-in reporting only cover contexts playwright made itself, so a // failure in a context opened here arrives with the app's own account of it thrown away — @@ -306,7 +311,7 @@ export const test = base.extend({ // Closed before anything is read off it: an $effect teardown reading a binding svelte has // already cleared throws on unmount, and that is the fault the suite was blindest to. - for (const context of contexts) { + for (const {context} of contexts) { await context.close() } @@ -318,15 +323,29 @@ export const test = base.extend({ if (faults.found.length > 0 || testInfo.status !== testInfo.expectedStatus) { // A path rather than a body: the list reporter truncates an inline attachment, and the // nightly run on the box keeps test-results and nothing else. - const path = testInfo.outputPath("browser-console.log") + const consolePath = testInfo.outputPath("browser-console.log") - await writeFile(path, faults.log.join("\n")) - await testInfo.attach("browser-console", {path, contentType: "text/plain"}) + await writeFile(consolePath, faults.log.join("\n")) + await testInfo.attach("browser-console", {path: consolePath, contentType: "text/plain"}) + + // What each user said to each relay and what came back. The console says what the app did + // with an event; this says whether it ever had one, which is the only way to tell a client + // that dropped a message from a relay that never sent it. + const transcriptPath = testInfo.outputPath("relay-transcript.log") + + await writeFile( + transcriptPath, + contexts.map(({name, context}) => `== ${name}\n${formatTranscript(context)}`).join("\n"), + ) + await testInfo.attach("relay-transcript", { + path: transcriptPath, + contentType: "text/plain", + }) } faults.assertNone() - for (const context of contexts) { + for (const {context} of contexts) { assertNoLeaks(context) } }, diff --git a/e2e/specs/dms.spec.ts b/e2e/specs/dms.spec.ts index 424f7b71..96016529 100644 --- a/e2e/specs/dms.spec.ts +++ b/e2e/specs/dms.spec.ts @@ -630,6 +630,10 @@ test("US-036 receive a new conversation live", async ({seed, as}) => { await sendDm(bob, "starting a chat with you") + // His own copy first, so a send that lost keystrokes to something else on the page fails here + // rather than thirty seconds later as a message that never reached her + await expect(bubble(bob, "starting a chat with you")).toBeVisible() + // Her list picks the conversation up on its own const fromBob = chatItems(alice).filter({hasText: "Bob Barnacle"}) diff --git a/src/lib/components/Dialog.svelte b/src/lib/components/Dialog.svelte index da64395b..b98ee0b6 100644 --- a/src/lib/components/Dialog.svelte +++ b/src/lib/components/Dialog.svelte @@ -104,8 +104,15 @@ }) onMount(() => { + const overlay = element const autofocus = panel.querySelector("[autofocus]") + const holdsFocus = () => { + const {activeElement} = document + + return !activeElement || activeElement === document.body || overlay.contains(activeElement) + } + trap = createFocusTrap(element, { allowOutsideClick: true, escapeDeactivates: false, @@ -113,7 +120,7 @@ ...(autofocus ? {initialFocus: autofocus} : {}), isolateSubtrees: false, returnFocusOnDeactivate: false, - setReturnFocus: previous => (previous.isConnected ? previous : false), + setReturnFocus: previous => (previous.isConnected && holdsFocus() ? previous : false), tabbableOptions: {getShadowRoot: true}, })