From 5100e004ae6cf54546758fdf9683b95e62f4b426 Mon Sep 17 00:00:00 2001 From: Cheng Peng Date: Tue, 3 Nov 2026 22:57:25 -0800 Subject: [PATCH 1/2] fix(channels): let shift+escape through the channel settings panel The channel settings panel closed on any Escape and stopped propagation, so Shift+Escape (Mark all as read) never reached the window shortcut dispatcher. Guard it with !event.shiftKey like the sibling panels. Co-authored-by: Claude Opus 5.5 Signed-off-by: Cheng Peng Co-authored-by: Cursor --- .../channels/ChannelSettingsPanel.test.tsx | 31 +++++++++++++++++++ src/bundled/channels/ChannelSettingsPanel.tsx | 3 +- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/bundled/channels/ChannelSettingsPanel.test.tsx b/src/bundled/channels/ChannelSettingsPanel.test.tsx index 3438571579..e0704d6c04 100644 --- a/src/bundled/channels/ChannelSettingsPanel.test.tsx +++ b/src/bundled/channels/ChannelSettingsPanel.test.tsx @@ -70,6 +70,37 @@ it("closes with Escape or its close button without swallowing other keys", async expect(close).toHaveBeenCalledTimes(3); }); +it("leaves Shift+Escape to Mark all as read", async () => { + const user = userEvent.setup(); + const close = vi.fn(); + const windowKeyDown = vi.fn(); + window.addEventListener("keydown", windowKeyDown); + try { + render( + + Diagnostics content + , + ); + await user.keyboard("{Shift>}{Escape}{/Shift}"); + expect(close).not.toHaveBeenCalled(); + expect(windowKeyDown).toHaveBeenCalledWith( + expect.objectContaining({ key: "Escape", shiftKey: true }), + ); + windowKeyDown.mockClear(); + await user.keyboard("{Escape}"); + expect(close).toHaveBeenCalledTimes(1); + expect(windowKeyDown).not.toHaveBeenCalledWith( + expect.objectContaining({ key: "Escape" }), + ); + } finally { + window.removeEventListener("keydown", windowKeyDown); + } +}); + it("does not invent unknown metadata and still exposes diagnostics without a channel", async () => { const user = userEvent.setup(); const { rerender } = render( diff --git a/src/bundled/channels/ChannelSettingsPanel.tsx b/src/bundled/channels/ChannelSettingsPanel.tsx index 7eae903c62..6407849e0a 100644 --- a/src/bundled/channels/ChannelSettingsPanel.tsx +++ b/src/bundled/channels/ChannelSettingsPanel.tsx @@ -66,7 +66,8 @@ export function ChannelSettingsPanel({ aria-label="Channel settings" aria-busy={loading} onKeyDown={(event) => { - if (event.key === "Escape") { + // Shift+Escape is Mark all as read, not a dismissal. + if (event.key === "Escape" && !event.shiftKey) { event.stopPropagation(); close(); } From 4b286248be268c768412ae9120ef8c0afe917bf9 Mon Sep 17 00:00:00 2001 From: Cheng Peng Date: Tue, 3 Nov 2026 23:30:21 -0800 Subject: [PATCH 2/2] test(channels): fold the shift+escape check into the escape test The new case mounted the same fixture as the existing Escape test and repeated its plain Escape assertion. Extend that test instead, as .agents/skills/test-audit asks for near-duplicates. Co-authored-by: Cursor Signed-off-by: Cheng Peng --- .../channels/ChannelSettingsPanel.test.tsx | 46 ++++++------------- 1 file changed, 14 insertions(+), 32 deletions(-) diff --git a/src/bundled/channels/ChannelSettingsPanel.test.tsx b/src/bundled/channels/ChannelSettingsPanel.test.tsx index e0704d6c04..629e1f9b55 100644 --- a/src/bundled/channels/ChannelSettingsPanel.test.tsx +++ b/src/bundled/channels/ChannelSettingsPanel.test.tsx @@ -2,7 +2,7 @@ import "@testing-library/jest-dom/vitest"; import { act, cleanup, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { afterEach, expect, it, vi } from "vitest"; +import { afterEach, expect, it, onTestFinished, vi } from "vitest"; import { ChannelSettingsPanel } from "./ChannelSettingsPanel"; afterEach(() => { @@ -48,6 +48,9 @@ it("shows known channel details with diagnostics collapsed until requested", asy it("closes with Escape or its close button without swallowing other keys", async () => { const user = userEvent.setup(); const close = vi.fn(); + const windowKeyDown = vi.fn(); + window.addEventListener("keydown", windowKeyDown); + onTestFinished(() => window.removeEventListener("keydown", windowKeyDown)); render( }{Escape}{/Shift}"); + expect(close).not.toHaveBeenCalled(); + expect(windowKeyDown).toHaveBeenCalledWith( + expect.objectContaining({ key: "Escape", shiftKey: true }), + ); + windowKeyDown.mockClear(); await user.keyboard("{Escape}"); expect(close).toHaveBeenCalledTimes(1); + expect(windowKeyDown).not.toHaveBeenCalledWith( + expect.objectContaining({ key: "Escape" }), + ); await user.click( screen.getByRole("button", { name: "Close Channel settings tab" }), ); @@ -70,37 +83,6 @@ it("closes with Escape or its close button without swallowing other keys", async expect(close).toHaveBeenCalledTimes(3); }); -it("leaves Shift+Escape to Mark all as read", async () => { - const user = userEvent.setup(); - const close = vi.fn(); - const windowKeyDown = vi.fn(); - window.addEventListener("keydown", windowKeyDown); - try { - render( - - Diagnostics content - , - ); - await user.keyboard("{Shift>}{Escape}{/Shift}"); - expect(close).not.toHaveBeenCalled(); - expect(windowKeyDown).toHaveBeenCalledWith( - expect.objectContaining({ key: "Escape", shiftKey: true }), - ); - windowKeyDown.mockClear(); - await user.keyboard("{Escape}"); - expect(close).toHaveBeenCalledTimes(1); - expect(windowKeyDown).not.toHaveBeenCalledWith( - expect.objectContaining({ key: "Escape" }), - ); - } finally { - window.removeEventListener("keydown", windowKeyDown); - } -}); - it("does not invent unknown metadata and still exposes diagnostics without a channel", async () => { const user = userEvent.setup(); const { rerender } = render(