From eb84d91a8050eabc6c231d47facb00729387acd1 Mon Sep 17 00:00:00 2001 From: Jacob Tomlinson Date: Tue, 31 Mar 2026 02:42:07 -0700 Subject: [PATCH] UI: build delete confirm popover without HTML strings (#58269) * UI: build delete confirm popover safely * UI: share delete confirm storage key --- ui/src/ui/chat/grouped-render.ts | 87 +++++++++----- ui/src/ui/views/chat.test.ts | 192 ++++++++++++++++++++++--------- 2 files changed, 196 insertions(+), 83 deletions(-) diff --git a/ui/src/ui/chat/grouped-render.ts b/ui/src/ui/chat/grouped-render.ts index 3dc1187ff86..6e350129d84 100644 --- a/ui/src/ui/chat/grouped-render.ts +++ b/ui/src/ui/chat/grouped-render.ts @@ -315,9 +315,15 @@ function extractGroupText(group: MessageGroup): string { return parts.join("\n\n"); } -const SKIP_DELETE_CONFIRM_KEY = "openclaw:skipDeleteConfirm"; +export const SKIP_DELETE_CONFIRM_KEY = "openclaw:skipDeleteConfirm"; type DeleteConfirmSide = "left" | "right"; +type DeleteConfirmPopover = { + popover: HTMLDivElement; + cancel: HTMLButtonElement; + yes: HTMLButtonElement; + check: HTMLInputElement; +}; function shouldSkipDeleteConfirm(): boolean { try { @@ -327,6 +333,45 @@ function shouldSkipDeleteConfirm(): boolean { } } +function createDeleteConfirmPopover(side: DeleteConfirmSide): DeleteConfirmPopover { + const popover = document.createElement("div"); + popover.className = `chat-delete-confirm chat-delete-confirm--${side}`; + + const text = document.createElement("p"); + text.className = "chat-delete-confirm__text"; + text.textContent = "Delete this message?"; + + const remember = document.createElement("label"); + remember.className = "chat-delete-confirm__remember"; + + const check = document.createElement("input"); + check.className = "chat-delete-confirm__check"; + check.type = "checkbox"; + + const rememberText = document.createElement("span"); + rememberText.textContent = "Don't ask again"; + + remember.append(check, rememberText); + + const actions = document.createElement("div"); + actions.className = "chat-delete-confirm__actions"; + + const cancel = document.createElement("button"); + cancel.className = "chat-delete-confirm__cancel"; + cancel.type = "button"; + cancel.textContent = "Cancel"; + + const yes = document.createElement("button"); + yes.className = "chat-delete-confirm__yes"; + yes.type = "button"; + yes.textContent = "Delete"; + + actions.append(cancel, yes); + popover.append(text, remember, actions); + + return { popover, cancel, yes, check }; +} + function renderDeleteButton(onDelete: () => void, side: DeleteConfirmSide) { return html` @@ -346,43 +391,31 @@ function renderDeleteButton(onDelete: () => void, side: DeleteConfirmSide) { existing.remove(); return; } - const popover = document.createElement("div"); - popover.className = `chat-delete-confirm chat-delete-confirm--${side}`; - popover.innerHTML = ` -

Delete this message?

- -
- - -
- `; + const { popover, cancel, yes, check } = createDeleteConfirmPopover(side); wrap.appendChild(popover); - const cancel = popover.querySelector(".chat-delete-confirm__cancel")!; - const yes = popover.querySelector(".chat-delete-confirm__yes")!; - const check = popover.querySelector(".chat-delete-confirm__check") as HTMLInputElement; + const removePopover = () => { + popover.remove(); + document.removeEventListener("click", closeOnOutside, true); + }; - cancel.addEventListener("click", () => popover.remove()); + // Close on click outside. + const closeOnOutside = (evt: MouseEvent) => { + if (!popover.contains(evt.target as Node) && evt.target !== btn) { + removePopover(); + } + }; + + cancel.addEventListener("click", removePopover); yes.addEventListener("click", () => { if (check.checked) { try { getSafeLocalStorage()?.setItem(SKIP_DELETE_CONFIRM_KEY, "1"); } catch {} } - popover.remove(); + removePopover(); onDelete(); }); - - // Close on click outside - const closeOnOutside = (evt: MouseEvent) => { - if (!popover.contains(evt.target as Node) && evt.target !== btn) { - popover.remove(); - document.removeEventListener("click", closeOnOutside, true); - } - }; requestAnimationFrame(() => document.addEventListener("click", closeOnOutside, true)); }} > diff --git a/ui/src/ui/views/chat.test.ts b/ui/src/ui/views/chat.test.ts index 2cfd252eea6..296769ad428 100644 --- a/ui/src/ui/views/chat.test.ts +++ b/ui/src/ui/views/chat.test.ts @@ -12,12 +12,41 @@ import { DEEPSEEK_CHAT_MODEL, DEFAULT_CHAT_MODEL_CATALOG, } from "../chat-model.test-helpers.ts"; +import { SKIP_DELETE_CONFIRM_KEY } from "../chat/grouped-render.ts"; import type { GatewayBrowserClient } from "../gateway.ts"; import type { ModelCatalogEntry } from "../types.ts"; import type { SessionsListResult } from "../types.ts"; import { renderChat, type ChatProps } from "./chat.ts"; import { renderOverview, type OverviewProps } from "./overview.ts"; +function readDeleteConfirmPreference(): string | null { + try { + return getSafeLocalStorage()?.getItem(SKIP_DELETE_CONFIRM_KEY) ?? null; + } catch { + return null; + } +} + +function clearDeleteConfirmPreference(): void { + try { + getSafeLocalStorage()?.removeItem(SKIP_DELETE_CONFIRM_KEY); + } catch { + /* noop */ + } +} + +function restoreDeleteConfirmPreference(value: string | null): void { + try { + if (value === null) { + getSafeLocalStorage()?.removeItem(SKIP_DELETE_CONFIRM_KEY); + return; + } + getSafeLocalStorage()?.setItem(SKIP_DELETE_CONFIRM_KEY, value); + } catch { + /* noop */ + } +} + function createSessions(): SessionsListResult { return { ts: 0, @@ -717,71 +746,122 @@ describe("chat view", () => { }); it("opens delete confirm on the left for user messages", () => { - try { - getSafeLocalStorage()?.removeItem("openclaw:skipDeleteConfirm"); - } catch { - /* noop */ - } + const originalPreference = readDeleteConfirmPreference(); + clearDeleteConfirmPreference(); const container = document.createElement("div"); - render( - renderChat( - createProps({ - messages: [ - { - role: "user", - content: "hello from user", - timestamp: 1000, - }, - ], - }), - ), - container, - ); + try { + render( + renderChat( + createProps({ + messages: [ + { + role: "user", + content: "hello from user", + timestamp: 1000, + }, + ], + }), + ), + container, + ); - const deleteButton = container.querySelector( - ".chat-group.user .chat-group-delete", - ); - expect(deleteButton).not.toBeNull(); - deleteButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + const deleteButton = container.querySelector( + ".chat-group.user .chat-group-delete", + ); + expect(deleteButton).not.toBeNull(); + deleteButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); - const confirm = container.querySelector(".chat-group.user .chat-delete-confirm"); - expect(confirm).not.toBeNull(); - expect(confirm?.classList.contains("chat-delete-confirm--left")).toBe(true); + const confirm = container.querySelector(".chat-group.user .chat-delete-confirm"); + expect(confirm).not.toBeNull(); + expect(confirm?.classList.contains("chat-delete-confirm--left")).toBe(true); + } finally { + restoreDeleteConfirmPreference(originalPreference); + } }); it("opens delete confirm on the right for assistant messages", () => { - try { - getSafeLocalStorage()?.removeItem("openclaw:skipDeleteConfirm"); - } catch { - /* noop */ - } + const originalPreference = readDeleteConfirmPreference(); + clearDeleteConfirmPreference(); const container = document.createElement("div"); - render( - renderChat( - createProps({ - messages: [ - { - role: "assistant", - content: "hello from assistant", - timestamp: 1000, - }, - ], - }), - ), - container, - ); + try { + render( + renderChat( + createProps({ + messages: [ + { + role: "assistant", + content: "hello from assistant", + timestamp: 1000, + }, + ], + }), + ), + container, + ); - const deleteButton = container.querySelector( - ".chat-group.assistant .chat-group-delete", - ); - expect(deleteButton).not.toBeNull(); - deleteButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + const deleteButton = container.querySelector( + ".chat-group.assistant .chat-group-delete", + ); + expect(deleteButton).not.toBeNull(); + deleteButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); - const confirm = container.querySelector( - ".chat-group.assistant .chat-delete-confirm", - ); - expect(confirm).not.toBeNull(); - expect(confirm?.classList.contains("chat-delete-confirm--right")).toBe(true); + const confirm = container.querySelector( + ".chat-group.assistant .chat-delete-confirm", + ); + expect(confirm).not.toBeNull(); + expect(confirm?.classList.contains("chat-delete-confirm--right")).toBe(true); + } finally { + restoreDeleteConfirmPreference(originalPreference); + } + }); + + it("renders delete confirm with the expected safe structure", () => { + const originalPreference = readDeleteConfirmPreference(); + clearDeleteConfirmPreference(); + const container = document.createElement("div"); + try { + render( + renderChat( + createProps({ + messages: [ + { + role: "assistant", + content: "hello from assistant", + timestamp: 1000, + }, + ], + }), + ), + container, + ); + + const deleteButton = container.querySelector( + ".chat-group.assistant .chat-group-delete", + ); + expect(deleteButton).not.toBeNull(); + deleteButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + + const confirm = container.querySelector( + ".chat-group.assistant .chat-delete-confirm", + ); + expect(confirm?.querySelector(".chat-delete-confirm__text")?.textContent).toBe( + "Delete this message?", + ); + expect(confirm?.querySelector(".chat-delete-confirm__remember span")?.textContent).toBe( + "Don't ask again", + ); + expect(confirm?.querySelector(".chat-delete-confirm__cancel")?.type).toBe( + "button", + ); + expect(confirm?.querySelector(".chat-delete-confirm__yes")?.type).toBe( + "button", + ); + expect(confirm?.querySelector(".chat-delete-confirm__check")?.type).toBe( + "checkbox", + ); + } finally { + restoreDeleteConfirmPreference(originalPreference); + } }); it("patches the current session model from the chat header picker", async () => {