mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 20:30:45 +00:00
fix(ui): clean up delete confirm popover listener
Co-authored-by: Ricardo-M-L <69202550+Ricardo-M-L@users.noreply.github.com>
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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/")) {
|
||||
|
||||
@@ -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([
|
||||
|
||||
@@ -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<HTMLButtonElement>(".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<typeof vi.spyOn>;
|
||||
}) {
|
||||
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<HTMLButtonElement>(
|
||||
".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<HTMLButtonElement>(".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<string, number>, contextWindow: number) => {
|
||||
const container = document.createElement("div");
|
||||
|
||||
@@ -607,6 +607,8 @@ const SKIP_DELETE_CONFIRM_KEY = "openclaw:skipDeleteConfirm";
|
||||
|
||||
type DeleteConfirmSide = "left" | "right";
|
||||
|
||||
const deleteConfirmDismissers = new WeakMap<Element, () => 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`
|
||||
<span class="chat-delete-wrap">
|
||||
@@ -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}
|
||||
|
||||
Reference in New Issue
Block a user