From 403aeff7f6edd506f9eca7d3ed1e342acf572a30 Mon Sep 17 00:00:00 2001 From: dotta Date: Sat, 28 Mar 2026 16:45:44 -0500 Subject: [PATCH] Refine mine inbox shortcut behavior Co-Authored-By: Paperclip --- ui/src/components/SwipeToArchive.test.tsx | 4 ++-- ui/src/components/SwipeToArchive.tsx | 3 +-- ui/src/lib/inbox.test.ts | 15 +++++++++++---- ui/src/lib/inbox.ts | 15 +++++++++++++-- ui/src/pages/Inbox.tsx | 21 ++++++++++++--------- 5 files changed, 39 insertions(+), 19 deletions(-) diff --git a/ui/src/components/SwipeToArchive.test.tsx b/ui/src/components/SwipeToArchive.test.tsx index 7adb2d92..4cdf92db 100644 --- a/ui/src/components/SwipeToArchive.test.tsx +++ b/ui/src/components/SwipeToArchive.test.tsx @@ -136,8 +136,8 @@ describe("SwipeToArchive", () => { const surface = container.querySelector("[data-inbox-row-surface]") as HTMLDivElement | null; expect(surface).not.toBeNull(); - expect(surface?.style.backgroundColor).toBe("hsl(var(--primary) / 0.06)"); - expect(surface?.style.boxShadow).toBe("inset 3px 0 0 hsl(var(--primary))"); + expect(surface?.style.backgroundColor).toBe("hsl(var(--muted))"); + expect(surface?.style.boxShadow).toBe(""); act(() => { root.unmount(); diff --git a/ui/src/components/SwipeToArchive.tsx b/ui/src/components/SwipeToArchive.tsx index be9ca964..4b0f304c 100644 --- a/ui/src/components/SwipeToArchive.tsx +++ b/ui/src/components/SwipeToArchive.tsx @@ -155,8 +155,7 @@ export function SwipeToArchive({ style={{ transform: `translate3d(${offsetX}px, 0, 0)`, transition: isDragging ? "none" : "transform 180ms ease-out", - backgroundColor: selected ? "hsl(var(--primary) / 0.06)" : undefined, - boxShadow: selected ? "inset 3px 0 0 hsl(var(--primary))" : undefined, + backgroundColor: selected ? "hsl(var(--muted))" : undefined, }} > {children} diff --git a/ui/src/lib/inbox.test.ts b/ui/src/lib/inbox.test.ts index c7851afb..a67a7451 100644 --- a/ui/src/lib/inbox.test.ts +++ b/ui/src/lib/inbox.test.ts @@ -6,6 +6,7 @@ import { computeInboxBadgeData, getApprovalsForTab, getInboxWorkItems, + getInboxKeyboardSelectionIndex, getRecentTouchedIssues, getUnreadTouchedIssues, isMineInboxTab, @@ -411,9 +412,15 @@ describe("inbox helpers", () => { }); it("anchors Mine selection to the first available inbox row", () => { - expect(resolveInboxSelectionIndex(-1, 3, true)).toBe(0); - expect(resolveInboxSelectionIndex(-1, 3, false)).toBe(-1); - expect(resolveInboxSelectionIndex(5, 3, true)).toBe(2); - expect(resolveInboxSelectionIndex(1, 0, true)).toBe(-1); + expect(resolveInboxSelectionIndex(-1, 3)).toBe(-1); + expect(resolveInboxSelectionIndex(5, 3)).toBe(2); + expect(resolveInboxSelectionIndex(1, 0)).toBe(-1); + }); + + it("selects the first row only after keyboard navigation starts", () => { + expect(getInboxKeyboardSelectionIndex(-1, 3, "next")).toBe(0); + expect(getInboxKeyboardSelectionIndex(-1, 3, "previous")).toBe(0); + expect(getInboxKeyboardSelectionIndex(0, 3, "next")).toBe(1); + expect(getInboxKeyboardSelectionIndex(0, 3, "previous")).toBe(0); }); }); diff --git a/ui/src/lib/inbox.ts b/ui/src/lib/inbox.ts index 9031ccc0..e86a02ee 100644 --- a/ui/src/lib/inbox.ts +++ b/ui/src/lib/inbox.ts @@ -105,13 +105,24 @@ export function isMineInboxTab(tab: InboxTab): boolean { export function resolveInboxSelectionIndex( previousIndex: number, itemCount: number, - canSelectItems: boolean, ): number { if (itemCount === 0) return -1; - if (previousIndex < 0) return canSelectItems ? 0 : -1; + if (previousIndex < 0) return -1; return Math.min(previousIndex, itemCount - 1); } +export function getInboxKeyboardSelectionIndex( + previousIndex: number, + itemCount: number, + direction: "next" | "previous", +): number { + if (itemCount === 0) return -1; + if (previousIndex < 0) return 0; + return direction === "next" + ? Math.min(previousIndex + 1, itemCount - 1) + : Math.max(previousIndex - 1, 0); +} + export function getLatestFailedRunsByAgent(runs: HeartbeatRun[]): HeartbeatRun[] { const sorted = [...runs].sort( (a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime(), diff --git a/ui/src/pages/Inbox.tsx b/ui/src/pages/Inbox.tsx index 7ed6e97d..e84bf7ae 100644 --- a/ui/src/pages/Inbox.tsx +++ b/ui/src/pages/Inbox.tsx @@ -47,6 +47,7 @@ import { ACTIONABLE_APPROVAL_STATUSES, getApprovalsForTab, getInboxWorkItems, + getInboxKeyboardSelectionIndex, getLatestFailedRunsByAgent, getRecentTouchedIssues, isMineInboxTab, @@ -929,10 +930,10 @@ export function Inbox() { return `join:${item.joinRequest.id}`; }, []); - // Keep Mine anchored to a real row so keyboard navigation always lands on an item. + // Keep selection valid when the list shape changes, but do not auto-select on initial load. useEffect(() => { - setSelectedIndex((prev) => resolveInboxSelectionIndex(prev, workItemsToRender.length, canArchiveFromTab)); - }, [canArchiveFromTab, workItemsToRender.length]); + setSelectedIndex((prev) => resolveInboxSelectionIndex(prev, workItemsToRender.length)); + }, [workItemsToRender.length]); // Use refs for keyboard handler to avoid stale closures const kbStateRef = useRef({ @@ -976,13 +977,15 @@ export function Inbox() { // Keyboard shortcuts (mail-client style) — single stable listener using refs useEffect(() => { const handleKeyDown = (e: KeyboardEvent) => { + if (e.defaultPrevented) return; + // Don't capture when typing in inputs/textareas or with modifier keys - const target = e.target as HTMLElement; + const target = e.target; if ( - target.tagName === "INPUT" || - target.tagName === "TEXTAREA" || - target.tagName === "SELECT" || + !(target instanceof HTMLElement) || + target.closest("input, textarea, select, [contenteditable='true'], [role='textbox'], [role='combobox']") || target.isContentEditable || + document.querySelector("[role='dialog'], [aria-modal='true']") || e.metaKey || e.ctrlKey || e.altKey @@ -1002,12 +1005,12 @@ export function Inbox() { switch (e.key) { case "j": { e.preventDefault(); - setSelectedIndex((prev) => Math.min(prev + 1, itemCount - 1)); + setSelectedIndex((prev) => getInboxKeyboardSelectionIndex(prev, itemCount, "next")); break; } case "k": { e.preventDefault(); - setSelectedIndex((prev) => Math.max(prev - 1, 0)); + setSelectedIndex((prev) => getInboxKeyboardSelectionIndex(prev, itemCount, "previous")); break; } case "a":