From 054b076b58c76847f2a860f1d735f6da494671fa Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Thu, 16 Jul 2026 07:50:48 -0500 Subject: [PATCH] fix(ui): keep inbox hover and j/k keyboard selection in sync across list reshapes (#9680) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The Inbox is the primary triage surface; it supports both mouse hover and `j`/`k` keyboard navigation over a flat, expandable list of rows > - Hover and keyboard selection are meant to be a single shared "cursor": hovering a row and then pressing `j`/`k` should continue from the hovered row, not jump elsewhere > - A prior hover-perf rewrite moved the hovered row into a numeric `hoveredIndexRef` and nulled it whenever the list reshaped; because the inbox polls constantly, any refresh between hovering and pressing a key dropped the hovered index > - With the hovered index dropped, the next keypress fell back to selection index `0`, stranding the cursor at the top of the list instead of continuing from the hovered row > - This pull request tracks the hovered row's stable identity (a nav key) alongside the numeric index and re-anchors it by key across reshapes, mirroring the existing keyboard-selection reconciliation > - The benefit is that mouse hover and keyboard navigation stay in sync exactly as intended, even while the inbox is polling ## Linked Issues or Issue Description **Bug report** - **What happened:** In the Inbox, hovering a row with the mouse and then pressing `j`/`k` (or another keyboard shortcut) does not continue from the hovered row. If the list refreshes (which happens on the inbox's constant polling) in the moment between hovering and pressing a key, keyboard selection snaps back to the top row instead. - **Expected behavior:** The keyboard cursor should be in sync with the hovered row — pressing `j`/`k` after hovering should move relative to the row the mouse is over. - **Steps to reproduce:** 1. Open the Inbox with several rows. 2. Hover the mouse over a row partway down the list. 3. Wait for (or trigger) a background poll/refresh of the list. 4. Press `j` or `k`. 5. Observe selection jumps to the top of the list instead of continuing from the hovered row. - **Root cause:** The `[flatNavItems]` effect nulled `hoveredIndexRef` on every list reshape. Since hover also clears the keyboard selection band to `-1`, the fallback selection index resolved to `0`. ## What Changed - Hoisted `navEntryKey` to module scope so it can compute a stable, index-independent identity for a nav row from both the hover handler and the reshape effect. - Added `hoveredNavKeyRef` to track the hovered row's stable key alongside the existing numeric `hoveredIndexRef`, set whenever the pointer selects a row. - On list reshape, re-anchor the hovered index by key (find the row with the same key) instead of unconditionally dropping it; only drop the hover when the row is actually gone. This mirrors the existing `selectedIndex` key-based reconciliation. - Added a unit test that hovers a row, reshapes the list via a simulated poll, presses `j`, and asserts selection continues from the hovered row. ## Verification - `pnpm --filter ./ui exec vitest run src/pages/Inbox.test.tsx` → 15/15 passing, including the new hover→`j`/`k` sync test and the existing keyboard-nav tests. - The new test explicitly covers the reshape-during-hover path that the prior test suite had deferred to live/e2e verification. ## Risks - Low risk. The change is confined to the Inbox's in-memory hover/selection bookkeeping (two refs and one effect); it adds no new renders (hover still paints via CSS `:hover`) and touches no data fetching, routing, or persistence. Behavior is unchanged when the list does not reshape; when it does, the hover now follows the same row instead of being dropped. ## Model Used - Claude Opus 4.8 (Anthropic), model id `claude-opus-4-8`, used with extended thinking and tool use (file edits, running the UI test suite locally). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Claude Opus 4.8 --- ui/src/pages/Inbox.test.tsx | 69 ++++++++++++++++++++++++++++++++++++- ui/src/pages/Inbox.tsx | 39 +++++++++++++++------ 2 files changed, 96 insertions(+), 12 deletions(-) diff --git a/ui/src/pages/Inbox.test.tsx b/ui/src/pages/Inbox.test.tsx index 275ebcaf91..feef05e839 100644 --- a/ui/src/pages/Inbox.test.tsx +++ b/ui/src/pages/Inbox.test.tsx @@ -107,8 +107,9 @@ vi.mock("../context/SidebarContext", () => ({ useSidebar: () => ({ isMobile: false }), })); +const generalSettingsMock = { keyboardShortcutsEnabled: false }; vi.mock("../context/GeneralSettingsContext", () => ({ - useGeneralSettings: () => ({ keyboardShortcutsEnabled: false }), + useGeneralSettings: () => generalSettingsMock, })); vi.mock("../hooks/useInboxBadge", () => ({ @@ -500,6 +501,72 @@ describe("Inbox toolbar", () => { }); }); + it("keeps hover→j/k selection in sync after the list reshapes (PAP-9679)", async () => { + routerMock.location.pathname = "/inbox/mine"; + generalSettingsMock.keyboardShortcutsEnabled = true; + const issueA = createIssue({ id: "issue-a", identifier: "PAP-2001", title: "Sync row A" }); + const issueB = createIssue({ id: "issue-b", identifier: "PAP-2002", title: "Sync row B" }); + const issueC = createIssue({ id: "issue-c", identifier: "PAP-2003", title: "Sync row C" }); + apiMocks.issuesList.mockResolvedValue([issueA, issueB, issueC]); + + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false, staleTime: 0, gcTime: 0 } }, + }); + const root = createRoot(container); + + const linkOf = (row: Element): HTMLAnchorElement | null => + row.querySelector("a[data-inbox-issue-link]"); + // The keyboard-selected row swaps to `hover:bg-transparent`; find its index. + const selectedRowIndex = () => + [...container.querySelectorAll("[data-inbox-item]")].findIndex((row) => + linkOf(row)?.className.includes("hover:bg-transparent"), + ); + + try { + await act(async () => { + root.render( + + + , + ); + }); + await vi.waitFor(() => { + expect(container.querySelectorAll("[data-inbox-item]").length).toBeGreaterThanOrEqual(3); + }); + + // Pointer physically moves, then hovers the middle row (index 1). + await act(async () => { + window.dispatchEvent(new MouseEvent("mousemove", { bubbles: true })); + const rows = container.querySelectorAll("[data-inbox-item]"); + rows[1]!.dispatchEvent(new MouseEvent("mouseover", { bubbles: true })); + rows[1]!.dispatchEvent(new MouseEvent("mouseenter", { bubbles: false })); + }); + + // A poll reshapes the list (row B's title changes → new nav array) before + // the keypress. This is what used to null the hovered index and strand + // j/k back at the top. + apiMocks.issuesList.mockResolvedValue([issueA, { ...issueB, title: "Sync row B (updated)" }, issueC]); + await act(async () => { + await queryClient.invalidateQueries(); + }); + await vi.waitFor(() => { + expect(container.textContent).toContain("Sync row B (updated)"); + }); + + // j must continue from the hovered row (index 1) → index 2, not jump to + // the top of the list. + await act(async () => { + document.body.dispatchEvent(new KeyboardEvent("keydown", { key: "j", bubbles: true })); + }); + expect(selectedRowIndex()).toBe(2); + } finally { + generalSettingsMock.keyboardShortcutsEnabled = false; + act(() => { + root.unmount(); + }); + } + }); + it("keeps other issue archive controls enabled while one archive is pending", async () => { routerMock.location.pathname = "/inbox/mine"; const issueA = createIssue({ id: "issue-a", identifier: "PAP-1001", title: "First inbox row" }); diff --git a/ui/src/pages/Inbox.tsx b/ui/src/pages/Inbox.tsx index bcafa2de32..f3ff80fe12 100644 --- a/ui/src/pages/Inbox.tsx +++ b/ui/src/pages/Inbox.tsx @@ -181,6 +181,17 @@ type SectionKey = /** A flat navigation entry for keyboard j/k traversal that includes expanded children. */ type NavEntry = InboxKeyboardNavEntry; +// Stable identity for a nav row, resilient to the numeric index shifting when +// the inbox reshapes (archive/poll). Used to re-anchor both the keyboard +// selection and the hovered row across list refreshes. +const navEntryKey = (entry: NavEntry | undefined): string | null => + !entry + ? null + : entry.type === "top" + ? `top:${entry.itemKey}` + : entry.type === "child" + ? `child:${entry.issueId}` + : `group:${entry.groupKey}`; type CreatorOption = { id: string; label: string; @@ -1405,6 +1416,10 @@ export function Inbox() { const flatNavItems = useMemo((): NavEntry[] => { return buildInboxKeyboardNavEntries(groupedSections, collapsedGroupKeys, collapsedInboxParents); }, [collapsedGroupKeys, collapsedInboxParents, groupedSections]); + // Read the current nav list from event handlers without recreating them (and + // without capturing a stale array), so hover can resolve the row's key. + const flatNavItemsRef = useRef(flatNavItems); + flatNavItemsRef.current = flatNavItems; // Roll live descendant runs up to their ancestors across the loaded inbox tree // so a parent that is not itself live can still surface "n live below". const subtreeLiveCounts = useMemo(() => { @@ -1614,9 +1629,14 @@ export function Inbox() { // list costs zero re-renders (hover paints via CSS `:hover`, see IssueRow). // Keyboard nav reads this to continue from the hovered row. const hoveredIndexRef = useRef(null); + // The hovered row's stable key, kept alongside the numeric index so a poll + // that reshapes the list can re-anchor the hover to the same row instead of + // dropping it (which stranded j/k back at the top — PAP-9679 regression). + const hoveredNavKeyRef = useRef(null); const setSelectedIndexFromPointer = useCallback((idx: number) => { if (!pointerMovedSinceKeyNavRef.current) return; hoveredIndexRef.current = idx; + hoveredNavKeyRef.current = navEntryKey(flatNavItemsRef.current[idx]); // Drop any keyboard selection band the moment the mouse takes over, so we // never show two identical highlights at once. React bails out when the // value is already -1, so continuous hovering triggers no re-render. @@ -1786,19 +1806,16 @@ export function Inbox() { // numeric index onto a neighboring row (and Enter would open the wrong // task). Falls back to clamping when the item is gone; never auto-selects // on initial load. - const navEntryKey = (entry: NavEntry | undefined): string | null => - !entry - ? null - : entry.type === "top" - ? `top:${entry.itemKey}` - : entry.type === "child" - ? `child:${entry.issueId}` - : `group:${entry.groupKey}`; const selectedNavKeyRef = useRef(null); useEffect(() => { - // A reshaped list invalidates the numeric hover index; drop it so the next - // keypress falls back to the (key-reconciled) keyboard selection. - hoveredIndexRef.current = null; + // A reshaped list invalidates the numeric hover index. Re-anchor it to the + // same row by key (the inbox polls constantly, so nulling it here silently + // broke hover→j/k sync — PAP-9679). Drop it only when the row is gone. + const hoveredKey = hoveredNavKeyRef.current; + const nextHovered = + hoveredKey === null ? -1 : flatNavItems.findIndex((entry) => navEntryKey(entry) === hoveredKey); + hoveredIndexRef.current = nextHovered >= 0 ? nextHovered : null; + if (nextHovered < 0) hoveredNavKeyRef.current = null; setSelectedIndex((prev) => { if (prev < 0) return resolveInboxSelectionIndex(prev, flatNavItems.length); const prevKey = selectedNavKeyRef.current;