From 8508a93f6e152b1388724a2daf347b2d3d771ecd Mon Sep 17 00:00:00 2001 From: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Date: Sat, 2 May 2026 23:54:13 +0000 Subject: [PATCH] fix(ui): clean up delete confirm popover listener Co-authored-by: Ricardo-M-L <69202550+Ricardo-M-L@users.noreply.github.com> --- CHANGELOG.md | 1 + scripts/test-projects.test-support.mjs | 20 ++++ test/scripts/test-projects.test.ts | 35 ++++++ ui/src/ui/chat/grouped-render.test.ts | 151 +++++++++++++++++++++++++ ui/src/ui/chat/grouped-render.ts | 47 ++++++-- 5 files changed, 244 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bf65b63cac..b3637cc5b3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,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..82212c48397 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/dreams.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/test/scripts/test-projects.test.ts b/test/scripts/test-projects.test.ts index 53d86c04489..50bc9881ed8 100644 --- a/test/scripts/test-projects.test.ts +++ b/test/scripts/test-projects.test.ts @@ -527,6 +527,41 @@ 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, + }, + ]); + }); + + 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/ui/src/ui/chat/grouped-render.test.ts b/ui/src/ui/chat/grouped-render.test.ts index 2af5f1dd279..24f0645b649 100644 --- a/ui/src/ui/chat/grouped-render.test.ts +++ b/ui/src/ui/chat/grouped-render.test.ts @@ -243,6 +243,96 @@ 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 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 +340,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 +413,62 @@ 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("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..deb057141f8 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) && target !== btn) { + 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}