From cb31616d889ab371f6260477496c16c2257a3e3f Mon Sep 17 00:00:00 2001 From: "clawsweeper[bot]" <274271284+clawsweeper[bot]@users.noreply.github.com> Date: Sun, 3 May 2026 02:21:10 +0000 Subject: [PATCH] fix(ui): clean up delete confirm popover listener (#76318) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: - The PR centralizes Control UI chat delete-confirm popover dismissal, adds listener-cleanup regression coverage and unit-UI test routing fixes, and records the fix in the changelog. - Reproducibility: yes. Current-main source shows a high-confidence path: open the delete confirm, let `reques ... ncel, Delete, or same-button toggle; those paths remove the popover without removing the document listener. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(ui): clean up delete confirm popover listener - PR branch already contained follow-up commit before automerge: fix(clawsweeper): address review for automerge-openclaw-openclaw-7559… - PR branch already contained follow-up commit before automerge: fix(clawsweeper): reconcile automerge-openclaw-openclaw-75590 with ma… - PR branch already contained follow-up commit before automerge: fix(ui): repair delete confirm listener cleanup checks Validation: - ClawSweeper review passed for head 62240d8153b8f3ec58c5d174f1fff93371b38d5a. - Required merge gates passed before the squash merge. Prepared head SHA: 62240d8153b8f3ec58c5d174f1fff93371b38d5a Review: https://github.com/openclaw/openclaw/pull/76318#issuecomment-4364990281 Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: Ricardo-M-L <69202550+Ricardo-M-L@users.noreply.github.com> --- CHANGELOG.md | 1 + scripts/test-projects.test-support.mjs | 20 +++ src/scripts/test-projects.test.ts | 4 +- test/scripts/test-projects.test.ts | 44 +++++++ test/vitest-projects-config.test.ts | 3 +- test/vitest/vitest.shared.config.ts | 2 +- test/vitest/vitest.ui.config.ts | 2 +- ui/src/ui/chat/grouped-render.test.ts | 168 +++++++++++++++++++++++++ ui/src/ui/chat/grouped-render.ts | 47 +++++-- 9 files changed, 276 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d60db56b26..b521c5c70c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,6 +70,7 @@ Docs: https://docs.openclaw.ai - Codex harness: forward OpenClaw workspace bootstrap files such as `SOUL.md` through native Codex config instructions while leaving `AGENTS.md` to Codex project-doc discovery. Fixes #76273. Thanks @zknicker. - Parallels/Windows update smoke: escape the stale post-swap import regex in the generated PowerShell script so expected `ERR_MODULE_NOT_FOUND` update handoffs continue to post-update health checks. (#75315) - Slack: allow draft preview streaming in top-level DMs when `replyToMode` is `off` while keeping Slack native streaming and assistant thread status gated on reply threads. Fixes #56480. (#56544) Thanks @HangGlidersRule. +- Control UI/chat: remove the delete-confirm popover outside-click listener on every dismiss path, so Cancel, Delete, outside clicks, and same-button toggles no longer leave stale document listeners behind. Refs #75590 and #69982. Thanks @Ricardo-M-L. ## 2026.5.2 diff --git a/scripts/test-projects.test-support.mjs b/scripts/test-projects.test-support.mjs index 74749e43ed7..ba2230897e9 100644 --- a/scripts/test-projects.test-support.mjs +++ b/scripts/test-projects.test-support.mjs @@ -634,6 +634,23 @@ function isVitestConfigTargetForKind(kind, targetArg, cwd) { return resolveVitestConfigTargetKind(toRepoRelativeTarget(targetArg, cwd)) === kind; } +function isUnitUiTestTarget(relative) { + if (!relative.endsWith(".test.ts")) { + return false; + } + return ( + relative === "ui/src/ui/app-chat.test.ts" || + relative.startsWith("ui/src/ui/chat/") || + relative === "ui/src/ui/views/agents-utils.test.ts" || + relative === "ui/src/ui/views/channels.test.ts" || + relative === "ui/src/ui/views/chat.test.ts" || + relative === "ui/src/ui/views/dreaming.test.ts" || + relative === "ui/src/ui/views/usage-render-details.test.ts" || + relative === "ui/src/ui/controllers/agents.test.ts" || + relative === "ui/src/ui/controllers/chat.test.ts" + ); +} + function resolveChannelContractTargetKind(relative) { if (!relative.startsWith("src/channels/plugins/contracts/")) { return null; @@ -1037,6 +1054,9 @@ function classifyTarget(arg, cwd) { return "plugin"; } if (relative.startsWith("ui/src/")) { + if (isUnitUiTestTarget(relative)) { + return "unitUi"; + } return "ui"; } if (relative.startsWith("src/utils/")) { diff --git a/src/scripts/test-projects.test.ts b/src/scripts/test-projects.test.ts index d9cb000f923..1b41a5f4263 100644 --- a/src/scripts/test-projects.test.ts +++ b/src/scripts/test-projects.test.ts @@ -778,10 +778,10 @@ describe("test-projects args", () => { ]); }); - it("routes ui targets to the ui config", () => { + it("routes unit ui targets to the unit ui config", () => { expect(buildVitestRunPlans(["ui/src/ui/views/channels.test.ts"])).toEqual([ { - config: "test/vitest/vitest.ui.config.ts", + config: "test/vitest/vitest.unit-ui.config.ts", forwardedArgs: [], includePatterns: ["ui/src/ui/views/channels.test.ts"], watchMode: false, diff --git a/test/scripts/test-projects.test.ts b/test/scripts/test-projects.test.ts index 53d86c04489..1338507fef2 100644 --- a/test/scripts/test-projects.test.ts +++ b/test/scripts/test-projects.test.ts @@ -527,6 +527,50 @@ describe("scripts/test-projects changed-target routing", () => { ]); }); + it("routes unit ui test targets to the unit ui lane", () => { + expect(buildVitestRunPlans(["ui/src/ui/chat/grouped-render.test.ts"])).toEqual([ + { + config: "test/vitest/vitest.unit-ui.config.ts", + forwardedArgs: [], + includePatterns: ["ui/src/ui/chat/grouped-render.test.ts"], + watchMode: false, + }, + ]); + + expect(buildVitestRunPlans(["ui/src/ui/views/chat.test.ts"])).toEqual([ + { + config: "test/vitest/vitest.unit-ui.config.ts", + forwardedArgs: [], + includePatterns: ["ui/src/ui/views/chat.test.ts"], + watchMode: false, + }, + ]); + + expect(buildVitestRunPlans(["ui/src/ui/views/dreaming.test.ts"])).toEqual([ + { + config: "test/vitest/vitest.unit-ui.config.ts", + forwardedArgs: [], + includePatterns: ["ui/src/ui/views/dreaming.test.ts"], + watchMode: false, + }, + ]); + }); + + it("routes changed unit ui tests to the unit ui lane", () => { + const plans = buildVitestRunPlans(["--changed", "origin/main"], process.cwd(), () => [ + "ui/src/ui/chat/grouped-render.test.ts", + ]); + + expect(plans).toEqual([ + { + config: "test/vitest/vitest.unit-ui.config.ts", + forwardedArgs: [], + includePatterns: ["ui/src/ui/chat/grouped-render.test.ts"], + watchMode: false, + }, + ]); + }); + it("routes auto-reply route source files to route regression tests", () => { expect( resolveChangedTestTargetPlan([ diff --git a/test/vitest-projects-config.test.ts b/test/vitest-projects-config.test.ts index a0b37eb45ca..5ad09be828d 100644 --- a/test/vitest-projects-config.test.ts +++ b/test/vitest-projects-config.test.ts @@ -25,7 +25,7 @@ import { resolveSharedVitestWorkerConfig, sharedVitestConfig, } from "./vitest/vitest.shared.config.ts"; -import { createUiVitestConfig } from "./vitest/vitest.ui.config.ts"; +import { createUiVitestConfig, unitUiIncludePatterns } from "./vitest/vitest.ui.config.ts"; import { createUnitFastVitestConfig } from "./vitest/vitest.unit-fast.config.ts"; import unitUiConfig from "./vitest/vitest.unit-ui.config.ts"; import { createUnitVitestConfig } from "./vitest/vitest.unit.config.ts"; @@ -163,6 +163,7 @@ describe("projects vitest config", () => { expect(unitUiConfig.test?.environment).toBe("jsdom"); expect(unitUiConfig.test?.isolate).toBe(false); expect(normalizeConfigPath(unitUiConfig.test?.runner)).toBe("test/non-isolated-runner.ts"); + expect(unitUiIncludePatterns).toContain("ui/src/ui/views/dreaming.test.ts"); const setupFiles = normalizeConfigPaths(unitUiConfig.test?.setupFiles); expect(setupFiles).not.toContain("test/setup-openclaw-runtime.ts"); expect(setupFiles).toContain("ui/src/test-helpers/lit-warnings.setup.ts"); diff --git a/test/vitest/vitest.shared.config.ts b/test/vitest/vitest.shared.config.ts index 559ed31e3d4..e37495b2d06 100644 --- a/test/vitest/vitest.shared.config.ts +++ b/test/vitest/vitest.shared.config.ts @@ -275,7 +275,7 @@ export const sharedVitestConfig = { "ui/src/ui/views/chat.test.ts", "ui/src/ui/views/nodes.devices.test.ts", "ui/src/ui/views/skills.test.ts", - "ui/src/ui/views/dreams.test.ts", + "ui/src/ui/views/dreaming.test.ts", "ui/src/ui/views/usage-render-details.test.ts", "ui/src/ui/controllers/agents.test.ts", "ui/src/ui/controllers/chat.test.ts", diff --git a/test/vitest/vitest.ui.config.ts b/test/vitest/vitest.ui.config.ts index 80a647fd9bf..655359f2412 100644 --- a/test/vitest/vitest.ui.config.ts +++ b/test/vitest/vitest.ui.config.ts @@ -7,7 +7,7 @@ export const unitUiIncludePatterns = [ "ui/src/ui/views/agents-utils.test.ts", "ui/src/ui/views/channels.test.ts", "ui/src/ui/views/chat.test.ts", - "ui/src/ui/views/dreams.test.ts", + "ui/src/ui/views/dreaming.test.ts", "ui/src/ui/views/usage-render-details.test.ts", "ui/src/ui/controllers/agents.test.ts", "ui/src/ui/controllers/chat.test.ts", diff --git a/ui/src/ui/chat/grouped-render.test.ts b/ui/src/ui/chat/grouped-render.test.ts index 2af5f1dd279..4dd75f0060b 100644 --- a/ui/src/ui/chat/grouped-render.test.ts +++ b/ui/src/ui/chat/grouped-render.test.ts @@ -243,6 +243,104 @@ function clearDeleteConfirmSkip() { localStorageValues.delete("openclaw:skipDeleteConfirm"); } +function stubAnimationFrameQueue() { + const callbacks: FrameRequestCallback[] = []; + vi.stubGlobal("requestAnimationFrame", (callback: FrameRequestCallback) => { + callbacks.push(callback); + return callbacks.length; + }); + return () => { + const pending = callbacks.splice(0); + for (const callback of pending) { + callback(performance.now()); + } + }; +} + +function getLastCaptureClickListener(calls: readonly unknown[][]) { + for (let index = calls.length - 1; index >= 0; index--) { + const [type, listener, options] = calls[index] ?? []; + if (type === "click" && options === true && listener) { + return listener; + } + } + return null; +} + +function countCaptureClickListenerRemovals(calls: readonly unknown[][], listener: unknown) { + return calls.filter( + ([type, removedListener, options]) => + type === "click" && options === true && removedListener === listener, + ).length; +} + +function renderDeleteConfirmFixture() { + const container = document.createElement("div"); + container.dataset.deleteConfirmFixture = "true"; + document.body.appendChild(container); + const onDelete = vi.fn(); + clearDeleteConfirmSkip(); + renderMessageGroups( + container, + [ + createMessageGroup( + { + role: "assistant", + content: "hello from assistant", + timestamp: 1000, + }, + "assistant", + ), + ], + { onDelete }, + ); + const deleteButton = container.querySelector(".chat-group-delete"); + expect(deleteButton).not.toBeNull(); + return { container, deleteButton: deleteButton!, onDelete }; +} + +function openDeleteConfirm(deleteButton: HTMLButtonElement) { + deleteButton.dispatchEvent(new MouseEvent("click", { bubbles: true })); +} + +function clickDeleteButtonIconPath(deleteButton: HTMLButtonElement) { + const icon = document.createElementNS("http://www.w3.org/2000/svg", "svg"); + const path = document.createElementNS("http://www.w3.org/2000/svg", "path"); + icon.appendChild(path); + deleteButton.appendChild(icon); + path.dispatchEvent(new MouseEvent("click", { bubbles: true })); +} + +function setupArmedDeleteConfirm() { + const flushAnimationFrames = stubAnimationFrameQueue(); + const addListenerSpy = vi.spyOn(document, "addEventListener"); + const removeListenerSpy = vi.spyOn(document, "removeEventListener"); + const fixture = renderDeleteConfirmFixture(); + + openDeleteConfirm(fixture.deleteButton); + flushAnimationFrames(); + + const outsideClickListener = getLastCaptureClickListener(addListenerSpy.mock.calls); + expect(outsideClickListener).not.toBeNull(); + expect(fixture.container.querySelector(".chat-delete-confirm")).not.toBeNull(); + + return { ...fixture, outsideClickListener, removeListenerSpy }; +} + +function expectDeleteConfirmDismissed(params: { + container: HTMLElement; + outsideClickListener: unknown; + removeListenerSpy: ReturnType; +}) { + expect(params.container.querySelector(".chat-delete-confirm")).toBeNull(); + expect( + countCaptureClickListenerRemovals( + params.removeListenerSpy.mock.calls, + params.outsideClickListener, + ), + ).toBe(1); +} + async function flushAssistantAttachmentAvailabilityChecks() { for (let i = 0; i < 6; i++) { await Promise.resolve(); @@ -250,8 +348,13 @@ async function flushAssistantAttachmentAvailabilityChecks() { } afterEach(() => { + document.querySelectorAll("[data-delete-confirm-fixture]").forEach((element) => { + element.remove(); + }); + clearDeleteConfirmSkip(); vi.useRealTimers(); vi.unstubAllGlobals(); + vi.restoreAllMocks(); }); describe("grouped chat rendering", () => { @@ -318,6 +421,71 @@ describe("grouped chat rendering", () => { expect(assistantConfirm?.classList.contains("chat-delete-confirm--right")).toBe(true); }); + it("removes the delete confirm outside-click listener when Cancel dismisses it", () => { + const fixture = setupArmedDeleteConfirm(); + const cancel = fixture.container.querySelector( + ".chat-delete-confirm__cancel", + ); + + expect(cancel).not.toBeNull(); + cancel?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + + expectDeleteConfirmDismissed(fixture); + expect(fixture.onDelete).not.toHaveBeenCalled(); + }); + + it("removes the delete confirm outside-click listener when Delete dismisses it", () => { + const fixture = setupArmedDeleteConfirm(); + const confirm = fixture.container.querySelector(".chat-delete-confirm__yes"); + + expect(confirm).not.toBeNull(); + confirm?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + + expectDeleteConfirmDismissed(fixture); + expect(fixture.onDelete).toHaveBeenCalledTimes(1); + }); + + it("removes the delete confirm outside-click listener when an outside click dismisses it", () => { + const fixture = setupArmedDeleteConfirm(); + + document.body.dispatchEvent(new MouseEvent("click", { bubbles: true })); + + expectDeleteConfirmDismissed(fixture); + expect(fixture.onDelete).not.toHaveBeenCalled(); + }); + + it("removes the delete confirm outside-click listener when the delete button toggles it", () => { + const fixture = setupArmedDeleteConfirm(); + + openDeleteConfirm(fixture.deleteButton); + + expectDeleteConfirmDismissed(fixture); + expect(fixture.onDelete).not.toHaveBeenCalled(); + }); + + it("removes the delete confirm outside-click listener when the delete button icon toggles it", () => { + const fixture = setupArmedDeleteConfirm(); + + clickDeleteButtonIconPath(fixture.deleteButton); + + expectDeleteConfirmDismissed(fixture); + expect(fixture.onDelete).not.toHaveBeenCalled(); + }); + + it("does not attach the delete confirm outside-click listener after an immediate toggle", () => { + const flushAnimationFrames = stubAnimationFrameQueue(); + const addListenerSpy = vi.spyOn(document, "addEventListener"); + const fixture = renderDeleteConfirmFixture(); + + openDeleteConfirm(fixture.deleteButton); + openDeleteConfirm(fixture.deleteButton); + flushAnimationFrames(); + + expect(fixture.container.querySelector(".chat-delete-confirm")).toBeNull(); + expect(getLastCaptureClickListener(addListenerSpy.mock.calls)).toBeNull(); + expect(fixture.onDelete).not.toHaveBeenCalled(); + }); + it("renders assistant context usage from input and cache tokens", () => { const renderUsage = (usage: Record, contextWindow: number) => { const container = document.createElement("div"); diff --git a/ui/src/ui/chat/grouped-render.ts b/ui/src/ui/chat/grouped-render.ts index 80423d2be68..53fe34d903a 100644 --- a/ui/src/ui/chat/grouped-render.ts +++ b/ui/src/ui/chat/grouped-render.ts @@ -607,6 +607,8 @@ const SKIP_DELETE_CONFIRM_KEY = "openclaw:skipDeleteConfirm"; type DeleteConfirmSide = "left" | "right"; +const deleteConfirmDismissers = new WeakMap void>(); + function shouldSkipDeleteConfirm(): boolean { try { return getSafeLocalStorage()?.getItem(SKIP_DELETE_CONFIRM_KEY) === "1"; @@ -615,6 +617,15 @@ function shouldSkipDeleteConfirm(): boolean { } } +function dismissDeleteConfirm(element: Element) { + const dismiss = deleteConfirmDismissers.get(element); + if (dismiss) { + dismiss(); + return; + } + element.remove(); +} + function renderDeleteButton(onDelete: () => void, side: DeleteConfirmSide) { return html` @@ -631,7 +642,7 @@ function renderDeleteButton(onDelete: () => void, side: DeleteConfirmSide) { const wrap = btn.closest(".chat-delete-wrap") as HTMLElement; const existing = wrap?.querySelector(".chat-delete-confirm"); if (existing) { - existing.remove(); + dismissDeleteConfirm(existing); return; } const popover = document.createElement("div"); @@ -653,25 +664,41 @@ function renderDeleteButton(onDelete: () => void, side: DeleteConfirmSide) { const yes = popover.querySelector(".chat-delete-confirm__yes")!; const check = popover.querySelector(".chat-delete-confirm__check") as HTMLInputElement; - cancel.addEventListener("click", () => popover.remove()); + let dismissed = false; + function dismissPopover() { + if (dismissed) { + return; + } + dismissed = true; + document.removeEventListener("click", closeOnOutside, true); + deleteConfirmDismissers.delete(popover); + popover.remove(); + } + function closeOnOutside(evt: MouseEvent) { + const target = evt.target; + if (target instanceof Node && !popover.contains(target) && !btn.contains(target)) { + dismissPopover(); + } + } + + deleteConfirmDismissers.set(popover, dismissPopover); + + cancel.addEventListener("click", dismissPopover); yes.addEventListener("click", () => { if (check.checked) { try { getSafeLocalStorage()?.setItem(SKIP_DELETE_CONFIRM_KEY, "1"); } catch {} } - popover.remove(); + dismissPopover(); 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(() => { + if (!dismissed && popover.isConnected) { + document.addEventListener("click", closeOnOutside, true); } - }; - requestAnimationFrame(() => document.addEventListener("click", closeOnOutside, true)); + }); }} > ${icons.trash ?? icons.x}