Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix: prevent double Escape in dialogs
Ensure AboutDialog only listens for Escape in the about view, wire ChangelogDialog Escape to go back to About, remove the redundant Close button in the changelog header, and update dialog tests to match the new navigation behavior.
  • Loading branch information
hearsilent committed Mar 18, 2026
commit ce45bd9d23b9dc300df3f352215d2b5ad2ef7945
14 changes: 14 additions & 0 deletions src/components/about-dialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,20 @@ describe("AboutDialog", () => {
expect(onClose).toHaveBeenCalled()
})

it("goes back to about view on Escape when showing changelog", async () => {
const onClose = vi.fn()
render(<AboutDialog version="1.2.3" onClose={onClose} />)

// Switch to changelog view.
await userEvent.click(screen.getByRole("button", { name: "View Changelog" }))
Comment thread
hearsilent marked this conversation as resolved.

// Press Escape; should go back to About view, not close.
await userEvent.keyboard("{Escape}")

expect(onClose).not.toHaveBeenCalled()
expect(screen.getByText("OpenUsage")).toBeInTheDocument()
})

it("does not close on other keys", async () => {
const onClose = vi.fn()
render(<AboutDialog version="1.2.3" onClose={onClose} />)
Expand Down
16 changes: 14 additions & 2 deletions src/components/about-dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ export function AboutDialog({ version, onClose }: AboutDialogProps) {

// Close on ESC key
useEffect(() => {
if (view !== "about") {
return;
}

const handleKeyDown = (e: KeyboardEvent) => {
if (e.key === "Escape") {
e.preventDefault();
Expand All @@ -43,7 +47,7 @@ export function AboutDialog({ version, onClose }: AboutDialogProps) {
};
document.addEventListener("keydown", handleKeyDown);
return () => document.removeEventListener("keydown", handleKeyDown);
}, [onClose]);
}, [onClose, view]);

// Close when panel hides (loses visibility)
useEffect(() => {
Expand All @@ -64,7 +68,15 @@ export function AboutDialog({ version, onClose }: AboutDialogProps) {
};

if (view === "changelog") {
return <ChangelogDialog currentVersion={version} onBack={() => setView("about")} onClose={onClose} />;
return (
<ChangelogDialog
currentVersion={version}
onBack={() => setView("about")}
// In changelog view, Escape should go back to About instead of
// closing the entire dialog, so hand off to setView.
onClose={() => setView("about")}
/>
);
}

return (
Expand Down
7 changes: 3 additions & 4 deletions src/components/changelog-dialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -219,14 +219,13 @@ describe("ChangelogDialog", () => {
/>,
)

// Back goes to previous view
await userEvent.click(screen.getByRole("button", { name: "Back" }))
expect(onBack).toHaveBeenCalled()

await userEvent.click(screen.getByRole("button", { name: "Close" }))
expect(onClose).toHaveBeenCalledTimes(1)

// Escape should trigger onClose once
await userEvent.keyboard("{Escape}")
expect(onClose).toHaveBeenCalledTimes(2)
expect(onClose).toHaveBeenCalledTimes(1)
})
})

7 changes: 0 additions & 7 deletions src/components/changelog-dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -201,13 +201,6 @@ export function ChangelogDialog({ currentVersion, onBack, onClose }: ChangelogDi
</button>
<h2 className="font-semibold text-sm tracking-tight">Release Notes</h2>
</div>
<button
onClick={onClose}
className="p-1.5 hover:bg-muted rounded-md transition-colors text-muted-foreground hover:text-foreground"
title="Close"
>
<X className="w-4 h-4" />
</button>
</div>

<div className="flex-1 overflow-y-auto p-5 custom-scrollbar overflow-x-hidden">
Expand Down