diff options
| author | Adam Malczewski <[email protected]> | 2026-10-24 17:09:17 +0900 |
|---|---|---|
| committer | Adam Malczewski <[email protected]> | 2026-10-24 17:09:17 +0900 |
| commit | 0ba54b4455cb560cf7a82baf71b31ed129cfc29b (patch) | |
| tree | 424ac806b3822247c0b835ee38ffabdf87f5016e /src | |
| parent | 83442d00d9d9c7f0ca200d6a2e8b42195708493f (diff) | |
| download | dispatch-web-0ba54b4455cb560cf7a82baf71b31ed129cfc29b.tar.gz dispatch-web-0ba54b4455cb560cf7a82baf71b31ed129cfc29b.zip | |
fix(tabs): finish accessible cascade confirmation and verify active closes
Diffstat (limited to 'src')
| -rw-r--r-- | src/app/App.test.ts | 115 | ||||
| -rw-r--r-- | src/features/tabs/tab-groups.test.ts | 9 | ||||
| -rw-r--r-- | src/features/tabs/ui.test.ts | 71 | ||||
| -rw-r--r-- | src/features/tabs/ui/CloseSubagentsConfirm.svelte | 44 | ||||
| -rw-r--r-- | src/features/tabs/ui/TabList.svelte | 12 |
5 files changed, 240 insertions, 11 deletions
diff --git a/src/app/App.test.ts b/src/app/App.test.ts index 9c5f5cc..ebc8004 100644 --- a/src/app/App.test.ts +++ b/src/app/App.test.ts @@ -2,7 +2,7 @@ import type { LayoutTree } from "@dispatch/layout-contract"; import { DEFAULT_LAYOUT } from "@dispatch/layout-contract"; import type { SetCwdRequest, WsServerMessage } from "@dispatch/transport-contract"; import type { SurfaceServerMessage } from "@dispatch/ui-contract"; -import { render, screen } from "@testing-library/svelte"; +import { fireEvent, render, screen, waitFor, within } from "@testing-library/svelte"; import userEvent from "@testing-library/user-event"; import { describe, expect, it, vi } from "vitest"; import type { WebSocketLike } from "../adapters/ws"; @@ -180,6 +180,119 @@ function activeConversationId(store: ReturnType<typeof createAppStore>): string } describe("App component interaction tests", () => { + it.each([ + "all", + "parent", + "cancel", + ] as const)("group close %s flows through the real store, persistence and HTTP close port", async (choice) => { + const ws = fakeSocket(); + const storage = createFakeStorage(); + const closed: string[] = []; + const base = fakeFetchWithLayout(["tabs"]); + const store = createAppStore({ + socketFactory: () => ws, + fetchImpl: async (input, init) => { + const url = + typeof input === "string" ? input : input instanceof URL ? input.href : input.url; + if (url.includes("/conversations?status=")) { + return new Response( + JSON.stringify({ + conversations: [ + { id: "p", workspaceId: "default" }, + { id: "c1", workspaceId: "default", tabGroupParentId: "p" }, + { id: "c2", workspaceId: "default", tabGroupParentId: "p" }, + { id: "g", workspaceId: "default", tabGroupParentId: "c1" }, + { id: "solo", workspaceId: "default" }, + { id: "other-workspace", workspaceId: "other", tabGroupParentId: "p" }, + ].map((conversation) => ({ + ...conversation, + title: conversation.id, + status: "idle", + lastActivityAt: 0, + })), + }), + { status: 200 }, + ); + } + if (url.endsWith("/close")) { + expect(init?.method).toBe("POST"); + const id = url.split("/").at(-2) as string; + closed.push(id); + return new Response(JSON.stringify({ conversationId: id, abortedTurn: false }), { + status: 200, + }); + } + return base(input, init); + }, + localStorage: storage, + }); + try { + ws.resolveOpen(); + render(App, { props: { store, onNavigate: vi.fn() } }); + await screen.findByRole("tablist"); + for (const [id, parent] of [ + ["p", null], + ["c1", "p"], + ["c2", "p"], + ["g", "c1"], + ["solo", null], + ] as const) { + ws.feedServerMessage({ + type: "conversation.open", + conversationId: id, + workspaceId: "default", + ...(parent === null ? {} : { tabGroupParentId: parent }), + }); + } + ws.feedServerMessage({ + type: "conversation.open", + conversationId: "other-workspace", + workspaceId: "other", + tabGroupParentId: "p", + }); + await waitFor(() => expect(screen.getAllByRole("tab")).toHaveLength(5)); + store.selectTab("p"); + const parent = await waitFor(() => { + const tab = document.querySelector<HTMLElement>('[role="tab"][data-conversation-id="p"]'); + expect(tab).not.toBeNull(); + return tab; + }); + if (parent === null) throw new Error("Parent tab missing"); + await fireEvent.click(within(parent).getByRole("button", { name: "Close tab" })); + const dialog = screen.getByRole("dialog", { name: "Close subagents too?" }); + expect(dialog).toHaveTextContent("3 subagent tabs"); + expect(closed).toEqual([]); + await fireEvent.click( + within(dialog).getByRole("button", { + name: + choice === "all" + ? /Yes, close all 4/ + : choice === "parent" + ? /No, keep them open/ + : "Cancel", + }), + ); + const expected = choice === "all" ? ["p", "c1", "c2", "g"] : choice === "parent" ? ["p"] : []; + expect(closed).toEqual(expected); + expect(store.tabs.map((tab) => tab.conversationId)).toEqual( + ["p", "c1", "c2", "g", "solo"].filter((id) => !expected.includes(id)), + ); + expect(screen.queryByRole("dialog")).toBeNull(); + if (choice !== "cancel") { + const persisted = JSON.parse(storage.getItem("dispatch.tabs") as string) as { + tabs: { conversationId: string }[]; + }; + expect(persisted.tabs.map((tab) => tab.conversationId)).toEqual([ + ...["p", "c1", "c2", "g", "solo"].filter((id) => !expected.includes(id)), + "other-workspace", + ]); + expect(store.activeConversationId).not.toBe("p"); + } + } finally { + store.dispose(); + } + }); + it("renders the model selector and composer in draft mode", () => { const ws = fakeSocket(); const store = createAppStore({ diff --git a/src/features/tabs/tab-groups.test.ts b/src/features/tabs/tab-groups.test.ts index 67150ff..46b04e7 100644 --- a/src/features/tabs/tab-groups.test.ts +++ b/src/features/tabs/tab-groups.test.ts @@ -127,6 +127,15 @@ describe("cascadeCloseIds (the ids a cascading tab close removes)", () => { expect(cascadeCloseIds(tabs, "c2")).toEqual(["c2"]); }); + it("matches the rendered forest for cycles and manual regrouping", () => { + const cycle = [tab("p", "c1"), tab("c1", "p"), tab("g", "c1")]; + expect(cascadeCloseIds(cycle, "p")).toEqual(["p"]); + expect(cascadeCloseIds(cycle, "c1")).toEqual(["c1", "g"]); + const regrouped = [tab("p"), tab("c1", "solo"), tab("g", "c1"), tab("solo")]; + expect(cascadeCloseIds(regrouped, "p")).toEqual(["p"]); + expect(cascadeCloseIds(regrouped, "solo")).toEqual(["solo", "c1", "g"]); + }); + it("ignores tabs whose group parent is no longer open", () => { const stale = [tab("p"), tab("orphan", "gone")]; expect(cascadeCloseIds(stale, "p")).toEqual(["p"]); diff --git a/src/features/tabs/ui.test.ts b/src/features/tabs/ui.test.ts index 88b89bf..8f775a1 100644 --- a/src/features/tabs/ui.test.ts +++ b/src/features/tabs/ui.test.ts @@ -744,10 +744,79 @@ describe("TabList", () => { expect(onClose.mock.calls.map((c) => c[0])).toEqual(["c1", "g"]); }); + it("restores keyboard focus to the opener after cancellation", async () => { + renderClosable(); + const user = userEvent.setup(); + const opener = within(card("p")).getByRole("button", { name: "Close tab" }); + await user.click(opener); + expect(screen.getByRole("button", { name: "Cancel" })).toHaveFocus(); + await user.click(screen.getByRole("button", { name: "Cancel" })); + expect(opener).toHaveFocus(); + }); + + it("Cancel and backdrop dismiss without closing any tab", async () => { + const onClose = renderClosable(); + await clickCloseOn("p"); + expect(screen.getByRole("button", { name: "Cancel" })).toHaveFocus(); + await fireEvent.click(screen.getByRole("button", { name: "Cancel" })); + expect(screen.queryByRole("dialog")).toBeNull(); + await clickCloseOn("p"); + await fireEvent.submit(dialog().querySelector("form") as HTMLFormElement); + expect(screen.queryByRole("dialog")).toBeNull(); + expect(onClose).not.toHaveBeenCalled(); + }); + + it("keeps the confirmed subtree snapshot when new tabs arrive", async () => { + const onClose = vi.fn(); + const props = { + tabs: grouped, + activeConversationId: "p", + onSelect: vi.fn(), + onClose, + onNewDraft: vi.fn(), + }; + const { rerender } = render(TabList, { props }); + await clickCloseOn("p"); + await rerender({ + ...props, + tabs: [ + ...grouped, + { + conversationId: "late", + model: "m", + title: "Late", + workspaceId: "w", + tabGroupParentId: "p", + }, + ], + }); + expect(dialog()).toHaveTextContent("3 subagent tabs"); + await fireEvent.click(screen.getByRole("button", { name: /Yes, close all/ })); + expect(onClose.mock.calls.map((call) => call[0])).toEqual(["p", "c1", "c2", "g"]); + }); + + it("dismisses when the target disappears (remote close or workspace switch)", async () => { + const onClose = vi.fn(); + const props = { + tabs: grouped, + activeConversationId: "p", + onSelect: vi.fn(), + onClose, + onNewDraft: vi.fn(), + }; + const { rerender } = render(TabList, { props }); + await clickCloseOn("p"); + await rerender({ ...props, tabs: grouped.filter((tab) => tab.conversationId !== "p") }); + expect(screen.queryByRole("dialog")).toBeNull(); + await rerender(props); + expect(screen.queryByRole("dialog")).toBeNull(); + expect(onClose).not.toHaveBeenCalled(); + }); + it("Escape dismisses the dialog without closing anything", async () => { const onClose = renderClosable(); await clickCloseOn("p"); - await fireEvent.keyDown(window, { key: "Escape" }); + await fireEvent(dialog(), new Event("cancel", { cancelable: true })); expect(screen.queryByRole("dialog")).toBeNull(); expect(onClose).not.toHaveBeenCalled(); }); diff --git a/src/features/tabs/ui/CloseSubagentsConfirm.svelte b/src/features/tabs/ui/CloseSubagentsConfirm.svelte index cbbec79..7ffda6c 100644 --- a/src/features/tabs/ui/CloseSubagentsConfirm.svelte +++ b/src/features/tabs/ui/CloseSubagentsConfirm.svelte @@ -7,7 +7,8 @@ * the whole subtree — "Yes" cascades the close to every transitive * descendant (sub-sub-agents included, no further prompts); "No" closes * only the tab itself. Escape / backdrop dismisses without closing anything. - * daisyUI `modal` (controlled: `modal-open`). + * Native modal dialog with daisyUI chrome: top-layer rendering, focus + * containment, and inert background even inside the animated sidebar. */ let { tab, @@ -27,14 +28,31 @@ /** Escape / backdrop — close nothing. */ onDismiss: () => void; } = $props(); + function openModal(node: HTMLDialogElement) { + const opener = document.activeElement; + node.showModal(); + return { + destroy() { + node.close(); + if (opener instanceof HTMLElement && opener.isConnected) opener.focus(); + }, + }; + } </script> -<svelte:window onkeydown={(e) => e.key === "Escape" && onDismiss()} /> - -<div class="modal modal-open" role="dialog" aria-label="Close subagents too?"> +<dialog + use:openModal + class="modal" + aria-label="Close subagents too?" + aria-describedby="close-subagents-description" + oncancel={(event) => { + event.preventDefault(); + onDismiss(); + }} +> <div class="modal-box max-w-md"> <h3 class="text-lg font-bold">Close subagents too?</h3> - <div class="flex flex-col gap-1 py-3"> + <div id="close-subagents-description" class="flex flex-col gap-1 py-3"> <p class="text-sm"> "{tab.title}" has {subagentCount} {subagentCount === 1 ? "subagent tab" : "subagent tabs"} (including nested ones). @@ -44,7 +62,7 @@ stopped. </p> </div> - <div class="modal-action"> + <div class="modal-action flex-wrap"> <button type="button" class="btn btn-ghost btn-sm" onclick={onDismiss}>Cancel</button> <button type="button" class="btn btn-sm" onclick={onKeep}>No, keep them open</button> <button type="button" class="btn btn-warning btn-sm" onclick={onCloseAll}> @@ -52,6 +70,14 @@ </button> </div> </div> - <!-- svelte-ignore a11y_click_events_have_key_events, a11y_no_static_element_interactions --> - <div class="modal-backdrop" onclick={onDismiss}></div> -</div> + <form + method="dialog" + class="modal-backdrop" + onsubmit={(event) => { + event.preventDefault(); + onDismiss(); + }} + > + <button type="submit" aria-label="Dismiss close confirmation">close</button> + </form> +</dialog> diff --git a/src/features/tabs/ui/TabList.svelte b/src/features/tabs/ui/TabList.svelte index 0493b5c..8b388f0 100644 --- a/src/features/tabs/ui/TabList.svelte +++ b/src/features/tabs/ui/TabList.svelte @@ -98,7 +98,19 @@ // in the meantime is a no-op for `onClose`. A plain tab closes directly. let closeConfirm = $state<{ tab: Tab; cascadeIds: readonly string[] } | null>(null); + // A remote close or workspace switch must not leave an actionable dialog + // for a tab that is no longer in this list. + $effect(() => { + if ( + closeConfirm !== null && + !tabs.some((tab) => tab.conversationId === closeConfirm?.tab.conversationId) + ) { + closeConfirm = null; + } + }); + function requestClose(conversationId: string): void { + menu = null; const cascadeIds = cascadeCloseIds(tabs, conversationId); if (cascadeIds.length <= 1) { onClose(conversationId); |
