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;