From bb2d6999fd5132ec969a295ba9076cd781e68a9e Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 15 Aug 2026 04:09:30 +0200 Subject: [PATCH] fix(channels): address R13 S3 review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ActionMenu: skip disabled items in roving keyboard nav (plumb disabledIndices through the AnchoredPopover platform); a pending action no longer strands focus on an unfocusable button. - ActionMenu + ChannelStatsPopover: add type="button" to the popover triggers so they can't submit an enclosing form. - ChannelStatsPopover: restore the per-stat explanations as a native title on each row — the guidance the old inline stats showed is back. - i18n: drop the now-dead channels.tags.yourTags / .manage keys (en+hu); the stats *Hint keys are live again via the restored row titles. - test: cover ActionMenu (open/select/close/disabled) and ChannelStatsPopover (rows, conditional ETA row, per-row hint title). --- .../src/components/ActionMenu.dom.test.tsx | 90 ++++++++++++++++++ frontend/src/components/ActionMenu.tsx | 11 ++- frontend/src/components/AnchoredPopover.tsx | 4 + .../ChannelStatsPopover.dom.test.tsx | 92 +++++++++++++++++++ .../src/components/ChannelStatsPopover.tsx | 29 +++++- frontend/src/i18n/locales/en/channels.json | 2 - frontend/src/i18n/locales/hu/channels.json | 2 - 7 files changed, 221 insertions(+), 9 deletions(-) create mode 100644 frontend/src/components/ActionMenu.dom.test.tsx create mode 100644 frontend/src/components/ChannelStatsPopover.dom.test.tsx diff --git a/frontend/src/components/ActionMenu.dom.test.tsx b/frontend/src/components/ActionMenu.dom.test.tsx new file mode 100644 index 00000000..bf213acd --- /dev/null +++ b/frontend/src/components/ActionMenu.dom.test.tsx @@ -0,0 +1,90 @@ +// @vitest-environment jsdom +// +// Contract of the reusable ⋯ overflow menu. Labels/handlers are plain props (no i18n), so we assert +// the open/select/close + disabled-item behaviour. The menu content only mounts while open, and the +// AnchoredPopover platform is floating-ui based, so we stub the two observers autoUpdate wires and +// wrap in a FloatingTree (its real-app parent) so nested-layer dispatch has a node to register. +import { describe, it, expect, vi, afterEach, beforeAll } from "vitest"; +import { render, screen, fireEvent, cleanup } from "@testing-library/react"; +import { FloatingTree } from "@floating-ui/react"; +import ActionMenu, { type ActionMenuItem } from "./ActionMenu"; + +beforeAll(() => { + // floating-ui's autoUpdate observes with ResizeObserver + IntersectionObserver (layoutShift); jsdom + // ships neither, and the floating element mounts (so they run) the moment the menu opens. + class Stub { + observe() {} + unobserve() {} + disconnect() {} + takeRecords() { + return []; + } + } + globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver; + globalThis.IntersectionObserver ??= Stub as unknown as typeof IntersectionObserver; +}); + +afterEach(cleanup); + +const renderMenu = (items: ActionMenuItem[]) => + render( + + + , + ); + +describe("ActionMenu", () => { + it("renders a labelled trigger and keeps items unmounted until opened", () => { + renderMenu([{ id: "sync", label: "Sync", onSelect: vi.fn() }]); + expect(screen.getByRole("button", { name: "Catalog actions" })).toBeTruthy(); + expect(screen.queryByRole("menuitem")).toBeNull(); + }); + + // floating-ui never computes a position in jsdom, so the surface stays visibility:hidden; the items + // are in the DOM but the a11y tree treats them as hidden — hence `{ hidden: true }` on role queries. + it("opens on click and shows one menuitem per item", () => { + renderMenu([ + { id: "sync", label: "Sync", onSelect: vi.fn() }, + { id: "backfill", label: "Backfill", onSelect: vi.fn() }, + ]); + fireEvent.click(screen.getByRole("button", { name: "Catalog actions" })); + expect(screen.getAllByRole("menuitem", { hidden: true }).map((b) => b.textContent)).toEqual([ + "Sync", + "Backfill", + ]); + }); + + it("selecting an item fires its onSelect and closes the menu", () => { + const onSelect = vi.fn(); + renderMenu([{ id: "sync", label: "Sync", onSelect }]); + fireEvent.click(screen.getByRole("button", { name: "Catalog actions" })); + fireEvent.click(screen.getByText("Sync")); + expect(onSelect).toHaveBeenCalledTimes(1); + expect(screen.queryByRole("menuitem", { hidden: true })).toBeNull(); + }); + + it("a disabled item is inert — rendered disabled and never fires onSelect", () => { + const onSelect = vi.fn(); + renderMenu([{ id: "sync", label: "Sync", onSelect, disabled: true }]); + fireEvent.click(screen.getByRole("button", { name: "Catalog actions" })); + const item = screen.getByText("Sync").closest("button")!; + expect((item as HTMLButtonElement).disabled).toBe(true); + fireEvent.click(item); + expect(onSelect).not.toHaveBeenCalled(); + }); + + it("honours a custom trigger node", () => { + renderMenu([{ id: "sync", label: "Sync", onSelect: vi.fn() }]); + cleanup(); + render( + + •••} + /> + , + ); + expect(screen.getByTestId("custom-trigger")).toBeTruthy(); + }); +}); diff --git a/frontend/src/components/ActionMenu.tsx b/frontend/src/components/ActionMenu.tsx index f959b974..6c999a38 100644 --- a/frontend/src/components/ActionMenu.tsx +++ b/frontend/src/components/ActionMenu.tsx @@ -31,10 +31,19 @@ export default function ActionMenu({ trigger?: ReactNode; }) { const [open, setOpen] = useState(false); - const pop = useAnchoredPopover({ open, onOpenChange: setOpen, placement: "bottom-end" }); + // 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, + }); return ( <>