diff --git a/apps/webapp/app/components/dashboard-agent/DashboardAgentHeader.tsx b/apps/webapp/app/components/dashboard-agent/DashboardAgentHeader.tsx index 841e59920..38a70a01a 100644 --- a/apps/webapp/app/components/dashboard-agent/DashboardAgentHeader.tsx +++ b/apps/webapp/app/components/dashboard-agent/DashboardAgentHeader.tsx @@ -6,7 +6,11 @@ import { Button } from "~/components/primitives/Buttons"; import { Popover, PopoverArrowTrigger, PopoverContent } from "~/components/primitives/Popover"; import { ShortcutKey } from "~/components/primitives/ShortcutKey"; import type { Shortcut } from "~/hooks/useShortcutKeys"; -import { DashboardAgentHistoryMenu, type DashboardAgentChat } from "./DashboardAgentHistory"; +import { + DashboardAgentDeleteChatDialog, + DashboardAgentHistoryMenu, + type DashboardAgentChat, +} from "./DashboardAgentHistory"; import { chatHistoryTriggerLabel } from "./header-labels"; // Display only. The key is registered once, in `DashboardAgent`; registering it @@ -45,6 +49,7 @@ export function DashboardAgentHeader({ onClose: () => void; }) { const [isHistoryOpen, setHistoryOpen] = useState(false); + const [pendingDelete, setPendingDelete] = useState(null); return (
@@ -77,11 +82,20 @@ export function DashboardAgentHeader({ setHistoryOpen(false); onSelectChat(chatId); }} - onDelete={onDeleteChat} + onRequestDelete={(chat) => { + setHistoryOpen(false); + setPendingDelete(chat); + }} /> + !open && setPendingDelete(null)} + onConfirm={onDeleteChat} + /> +
{showNewChat && ( - } - cancelButton={ - - } - /> -
- - - +
+ {chats.length === 0 ? ( + + No previous chats yet. + + ) : ( + + {unreadFirst(chats).map((chat) => { + const process = chatProcess(chat, chat.id === thinkingChatId); + const age = chat.lastMessageAt ? chatAge(chat.lastMessageAt, now) : undefined; + return ( + : null} + meta={age} + variant={chat.id === currentChatId ? "selected" : "default"} + onSelect={() => onSelect(chat.id)} + action={ + onRequestDelete(chat)} + danger + /> + } + /> + ); + })} + + )} +
+ ); +} + +// Rendered outside the history popover: inside it, focus moving to the dialog dismisses the +// popover, which unmounts the dialog before it can be answered. +export function DashboardAgentDeleteChatDialog({ + chat, + onOpenChange, + onConfirm, +}: { + chat: DashboardAgentChat | null; + onOpenChange: (open: boolean) => void; + onConfirm: (chatId: string) => void; +}) { + return ( + + + Delete this chat? +
+ + "{chat?.title}" and everything in it will be deleted. This can't be undone. + + { + if (chat) onConfirm(chat.id); + onOpenChange(false); + }} + > + Delete chat + + } + cancelButton={ + + } + /> +
+ + ); } diff --git a/apps/webapp/app/components/dashboard-agent/DashboardAgentPanel.tsx b/apps/webapp/app/components/dashboard-agent/DashboardAgentPanel.tsx index 3f9304b6b..ea7bf25d0 100644 --- a/apps/webapp/app/components/dashboard-agent/DashboardAgentPanel.tsx +++ b/apps/webapp/app/components/dashboard-agent/DashboardAgentPanel.tsx @@ -525,10 +525,18 @@ export function DashboardAgentPanel({ return (
{ - if (event.key !== "Escape" || event.defaultPrevented) return; + if ( + !escapeClosesPanel({ + key: event.key, + defaultPrevented: event.defaultPrevented, + targetInsidePanel: panelRef.current?.contains(event.target as Node) ?? false, + }) + ) + return; event.preventDefault(); onClose(); }} diff --git a/apps/webapp/app/components/dashboard-agent/panel-escape.test.ts b/apps/webapp/app/components/dashboard-agent/panel-escape.test.ts new file mode 100644 index 000000000..cfb578283 --- /dev/null +++ b/apps/webapp/app/components/dashboard-agent/panel-escape.test.ts @@ -0,0 +1,75 @@ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; +import { escapeClosesPanel } from "./panel-escape"; + +/** + * Escape has to reach the thing the user meant. Radix dismisses a popover or a dialog from a + * document listener that runs after the panel's own handler and never marks the event handled, + * so the panel has to decide for itself whether the keystroke came from inside it. + */ +describe("escapeClosesPanel", () => { + it("closes the panel when Escape comes from the panel itself", () => { + expect( + escapeClosesPanel({ key: "Escape", defaultPrevented: false, targetInsidePanel: true }) + ).toBe(true); + }); + + it("leaves the panel open when Escape comes from a portalled layer", () => { + // The history popover and the delete dialog both render outside the panel's DOM subtree. + expect( + escapeClosesPanel({ key: "Escape", defaultPrevented: false, targetInsidePanel: false }) + ).toBe(false); + }); + + it("stays out of the way once something else has handled the key", () => { + expect( + escapeClosesPanel({ key: "Escape", defaultPrevented: true, targetInsidePanel: true }) + ).toBe(false); + }); + + it("ignores every other key", () => { + expect( + escapeClosesPanel({ key: "Enter", defaultPrevented: false, targetInsidePanel: true }) + ).toBe(false); + expect(escapeClosesPanel({ key: "j", defaultPrevented: false, targetInsidePanel: true })).toBe( + false + ); + }); +}); + +/** + * Structural guards, not behavioural proof: the delete confirmation's survival depends on where + * it is mounted in the tree, which these assertions pin down without rendering anything. + */ +describe("the delete confirmation lives outside the history popover", () => { + const header = readFileSync(new URL("./DashboardAgentHeader.tsx", import.meta.url), "utf8"); + const history = readFileSync(new URL("./DashboardAgentHistory.tsx", import.meta.url), "utf8"); + const panel = readFileSync(new URL("./DashboardAgentPanel.tsx", import.meta.url), "utf8"); + + const menuBody = history.slice( + history.indexOf("export function DashboardAgentHistoryMenu"), + history.indexOf("export function DashboardAgentDeleteChatDialog") + ); + + it("keeps no dialog and no pending state inside the popover's menu", () => { + expect(menuBody).not.toContain(" { + const popoverEnd = header.indexOf(""); + const dialog = header.indexOf(" { + expect(header).toContain("const [pendingDelete, setPendingDelete] = useState"); + }); + + it("gates the panel's Escape on the shared rule rather than defaultPrevented alone", () => { + expect(panel).toContain("escapeClosesPanel({"); + expect(panel).toContain("panelRef.current?.contains(event.target as Node)"); + expect(panel).not.toContain('if (event.key !== "Escape" || event.defaultPrevented) return;'); + }); +}); diff --git a/apps/webapp/app/components/dashboard-agent/panel-escape.ts b/apps/webapp/app/components/dashboard-agent/panel-escape.ts new file mode 100644 index 000000000..201d7b09c --- /dev/null +++ b/apps/webapp/app/components/dashboard-agent/panel-escape.ts @@ -0,0 +1,15 @@ +/** + * Escape inside the panel closes the panel — but a popover or a dialog is portalled out of + * the panel's DOM subtree while still bubbling through the React tree, and Radix dismisses + * those from a document listener that runs after this handler, so the event arrives here + * undefaulted. Deciding on the DOM target is what tells the two apart. + */ +export function escapeClosesPanel(event: { + key: string; + defaultPrevented: boolean; + /** Whether the event's target is a DOM descendant of the panel. */ + targetInsidePanel: boolean; +}): boolean { + if (event.key !== "Escape" || event.defaultPrevented) return false; + return event.targetInsidePanel; +}