canvas/BatchActionBar: wire Esc to clear selection (matches button title)
Two small fixes for the batch-action toolbar: 1. The deselect button's title says "Clear selection (Escape)" — but pressing Escape did NOTHING. The title has been lying since the bar shipped. Now wired: window keydown handler calls clearSelection when Esc fires. Skipped while the confirm dialog is open (`pending !== null`) so the dialog's own Esc-cancels takes precedence, and skipped during a busy in-flight action so the user can't strand a partial-failure mid-flight. 2. focus-visible:ring-zinc-500/70 → focus-visible:ring-accent/50 on the deselect button. The hardcoded zinc broke the semantic- token pattern used by the other action buttons. Tests: two new vitest cases — Esc clears with selection, Esc no-op when empty (the bar isn't mounted at count===0 so the listener never registers). Full suite: 1222/1222. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
3c127ae3b9
commit
b1a1c8e4a9
@ -30,6 +30,24 @@ export function BatchActionBar() {
|
||||
if (count === 0 && hasFailedBatch) setHasFailedBatch(false);
|
||||
}, [count, hasFailedBatch]);
|
||||
|
||||
// Esc clears selection — the deselect button title has been promising
|
||||
// "(Escape)" since the bar shipped, but no handler was wired. Skip when
|
||||
// the confirm dialog is open (`pending !== null`) so the dialog's own
|
||||
// Esc-cancels takes precedence and we don't double-handle the keystroke.
|
||||
// Also skip during a busy in-flight action so the user can't accidentally
|
||||
// strand a partial-failure mid-flight.
|
||||
useEffect(() => {
|
||||
if (count === 0 || pending !== null || busy) return;
|
||||
const onKey = (e: KeyboardEvent) => {
|
||||
if (e.key === "Escape") {
|
||||
e.stopPropagation();
|
||||
clearSelection();
|
||||
}
|
||||
};
|
||||
window.addEventListener("keydown", onKey);
|
||||
return () => window.removeEventListener("keydown", onKey);
|
||||
}, [count, pending, busy, clearSelection]);
|
||||
|
||||
// Hide when nothing is selected. Hide for single-node selection UNLESS a
|
||||
// partial-failure left a survivor awaiting retry.
|
||||
if (count === 0) return null;
|
||||
@ -129,7 +147,7 @@ export function BatchActionBar() {
|
||||
onClick={clearSelection}
|
||||
aria-label="Clear selection"
|
||||
title="Clear selection (Escape)"
|
||||
className="p-1.5 rounded-lg text-[12px] text-ink-mid hover:text-ink hover:bg-surface-card/50 transition-colors disabled:opacity-50 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-zinc-500/70"
|
||||
className="p-1.5 rounded-lg text-[12px] text-ink-mid hover:text-ink hover:bg-surface-card/50 transition-colors disabled:opacity-50 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-accent/50"
|
||||
>
|
||||
✕
|
||||
</button>
|
||||
|
||||
@ -130,6 +130,26 @@ describe("BatchActionBar", () => {
|
||||
const toolbar = screen.getByRole("toolbar");
|
||||
expect(toolbar.getAttribute("aria-label")).toBe("Batch workspace actions");
|
||||
});
|
||||
|
||||
it("Esc clears the selection — matches the deselect button title", () => {
|
||||
// The deselect button has been promising "Clear selection (Escape)"
|
||||
// since the bar shipped, but no handler was wired. This pins the
|
||||
// contract.
|
||||
mockSelectedNodeIds = new Set(["ws-1", "ws-2"]);
|
||||
render(<BatchActionBar />);
|
||||
fireEvent.keyDown(window, { key: "Escape" });
|
||||
expect(mockClearSelection).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("Esc is a no-op when nothing is selected", () => {
|
||||
mockSelectedNodeIds = new Set<string>();
|
||||
render(<BatchActionBar />);
|
||||
fireEvent.keyDown(window, { key: "Escape" });
|
||||
// The early-return at count===0 prevents the bar from mounting at all,
|
||||
// so the keydown listener never registers. clearSelection must NOT be
|
||||
// called.
|
||||
expect(mockClearSelection).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
|
||||
Loading…
Reference in New Issue
Block a user