From 5838cf1c56776833838a3ec34f73cd9cdc03c256 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 15 Aug 2026 04:45:32 +0200 Subject: [PATCH] test(channels): cover ActionMenu keyboard-skip + stats ETA value; drop redundant disabledIndices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deep-review follow-up on R13 S3. - Add a roving-keyboard-nav test proving a disabled ActionMenu item is skipped by arrow keys, and assert the stats "left" row renders the formatEta value (formatEta had no coverage anywhere). - Remove the disabledIndices plumbing from AnchoredPopover + ActionMenu: it was redundant. useListNavigation already skips items via their DOM disabled/aria-disabled attribute, and PopoverItem renders a native disabled button — verified against @floating-ui/react 0.27 isListIndexDisabled and by a mutation test. Behaviour is unchanged and now guarded by the new test. --- .../src/components/ActionMenu.dom.test.tsx | 37 +++++++++++++++++++ frontend/src/components/ActionMenu.tsx | 13 ++----- frontend/src/components/AnchoredPopover.tsx | 8 ++-- .../ChannelStatsPopover.dom.test.tsx | 10 ++++- 4 files changed, 53 insertions(+), 15 deletions(-) diff --git a/frontend/src/components/ActionMenu.dom.test.tsx b/frontend/src/components/ActionMenu.dom.test.tsx index bf213acd..72090311 100644 --- a/frontend/src/components/ActionMenu.dom.test.tsx +++ b/frontend/src/components/ActionMenu.dom.test.tsx @@ -73,6 +73,43 @@ describe("ActionMenu", () => { expect(onSelect).not.toHaveBeenCalled(); }); + it("skips a disabled item in roving keyboard nav", () => { + // items[0] disabled, items[1] + items[2] enabled. Roving keyboard focus (the `tabindex=0` item, + // which tracks useListNavigation's activeIndex) must NEVER land on the disabled index-0 — arrowing + // skips straight over it. The skip comes for free: PopoverItem renders a native `disabled` button + // and useListNavigation reads that DOM state (no explicit disabledIndices needed). Without it, + // ArrowUp/Down would strand roving focus on the inert index-0 button. (floating-ui wires this on + // real key nav, not on open, in jsdom — so we drive it with arrow keys.) + renderMenu([ + { id: "sync", label: "Sync", onSelect: vi.fn(), disabled: true }, + { id: "backfill", label: "Backfill", onSelect: vi.fn() }, + { id: "reset", label: "Reset", onSelect: vi.fn() }, + ]); + fireEvent.click(screen.getByRole("button", { name: "Catalog actions" })); + const [disabled, backfill, reset] = screen.getAllByRole("menuitem", { hidden: true }) as [ + HTMLElement, + HTMLElement, + HTMLElement, + ]; + const menu = screen.getByRole("menu", { hidden: true }); + const rover = () => + [disabled, backfill, reset].find((el) => el.getAttribute("tabindex") === "0")?.textContent; + + // The disabled item never becomes the roving target, whichever way we arrow. + fireEvent.keyDown(menu, { key: "ArrowDown" }); // skips disabled[0] -> first enabled + expect(rover()).toBe("Backfill"); + expect(disabled.getAttribute("tabindex")).toBe("-1"); + + fireEvent.keyDown(menu, { key: "ArrowDown" }); + expect(rover()).toBe("Reset"); + + fireEvent.keyDown(menu, { key: "ArrowUp" }); // back to Backfill + expect(rover()).toBe("Backfill"); + fireEvent.keyDown(menu, { key: "ArrowUp" }); // wraps over disabled[0] to the last item + expect(rover()).toBe("Reset"); + expect(disabled.getAttribute("tabindex")).toBe("-1"); // still never the disabled item + }); + it("honours a custom trigger node", () => { renderMenu([{ id: "sync", label: "Sync", onSelect: vi.fn() }]); cleanup(); diff --git a/frontend/src/components/ActionMenu.tsx b/frontend/src/components/ActionMenu.tsx index 6c999a38..b14bb497 100644 --- a/frontend/src/components/ActionMenu.tsx +++ b/frontend/src/components/ActionMenu.tsx @@ -31,15 +31,10 @@ export default function ActionMenu({ trigger?: ReactNode; }) { const [open, setOpen] = useState(false); - // A disabled item (a mutation already in flight) can't take DOM focus, so roving keyboard nav must - // SKIP it — otherwise ArrowUp/Down strands the highlight on an inert button. The platform handles it. - const disabledIndices = items.flatMap((it, i) => (it.disabled ? [i] : [])); - const pop = useAnchoredPopover({ - open, - onOpenChange: setOpen, - placement: "bottom-end", - disabledIndices, - }); + // A disabled item (a mutation already in flight) is skipped by roving keyboard nav automatically: + // PopoverItem renders it as a native `disabled` button, which useListNavigation reads and never + // focuses. No explicit disabledIndices needed. See ActionMenu.dom.test.tsx for the guard. + const pop = useAnchoredPopover({ open, onOpenChange: setOpen, placement: "bottom-end" }); return ( <>