fix(channels): address R13 S3 review findings

- 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).
This commit is contained in:
2026-08-15 04:09:30 +02:00
parent a26d96630d
commit bb2d6999fd
7 changed files with 221 additions and 9 deletions
@@ -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(
<FloatingTree>
<ActionMenu ariaLabel="Catalog actions" items={items} />
</FloatingTree>,
);
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(
<FloatingTree>
<ActionMenu
ariaLabel="More"
items={[{ id: "x", label: "X", onSelect: vi.fn() }]}
trigger={<span data-testid="custom-trigger">•••</span>}
/>
</FloatingTree>,
);
expect(screen.getByTestId("custom-trigger")).toBeTruthy();
});
});
+10 -1
View File
@@ -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 (
<>
<button
type="button"
ref={pop.refs.setReference}
{...pop.getReferenceProps()}
aria-label={ariaLabel}
@@ -91,6 +91,9 @@ export interface AnchoredPopoverOptions {
* lands the roving focus on it instead of the first item — the behaviour a "pick one of these
* modes" menu needs. Leave unset for action menus and multi-select toggle lists. */
selectedIndex?: number | null;
/** Indices of items that are disabled (e.g. an action whose mutation is already in flight), so
* roving keyboard nav SKIPS them instead of stranding focus on an unfocusable disabled button. */
disabledIndices?: number[];
}
// Internal: call sites use the inferred hook return, they don't name the type.
@@ -175,6 +178,7 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) {
setHoverIndex(null);
},
selectedIndex: opts.selectedIndex ?? null,
disabledIndices: opts.disabledIndices,
loop: true,
// Focus the first item on open for BOTH click and keyboard (the default 'auto' skips pointer-opens,
// which strands focus on the menu container — arrows then can't enter the roving list at all).
@@ -0,0 +1,92 @@
// @vitest-environment jsdom
//
// Contract of the catalog-stats hover chip: the compact trigger, the rows it reveals on open (value
// formatting + the conditional "left" row), and — the point of the R13-S3 review fix — that each row
// still carries its per-stat explanation as a `title`, so the guidance the old inline stats showed is
// not lost. i18n is mocked to a key-passthrough so assertions read the stable keys, not translations.
// The AnchoredPopover platform is floating-ui based (hover mode), so we stub the observers autoUpdate
// wires and wrap in a FloatingTree, and open via focus (hover mode enables useFocus).
import { describe, it, expect, afterEach, beforeAll, vi } from "vitest";
import { render, screen, fireEvent, cleanup } from "@testing-library/react";
import { FloatingTree } from "@floating-ui/react";
// Key-passthrough t so assertions read stable keys — but keep the rest of react-i18next intact
// (i18n/index.ts calls i18n.use(initReactI18next) at import time).
vi.mock("react-i18next", async (importOriginal) => {
const actual = await importOriginal<typeof import("react-i18next")>();
return { ...actual, useTranslation: () => ({ t: (key: string) => key }) };
});
import ChannelStatsPopover from "./ChannelStatsPopover";
import { type MyStatus } from "../lib/api";
beforeAll(() => {
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 status = (over: Partial<MyStatus> = {}): MyStatus =>
({
channels_total: 10,
channels_recent_synced: 3,
channels_deep_done: 1,
channels_deep_requested: 4,
deep_pending_count: 0,
deep_eta_seconds: 120,
my_videos: 1234,
quota_remaining_today: 5000,
...over,
}) as MyStatus;
const open = (s: MyStatus) => {
render(
<FloatingTree>
<ChannelStatsPopover status={s} />
</FloatingTree>,
);
fireEvent.focus(screen.getByRole("button", { name: "channels.stats.label" }));
};
describe("ChannelStatsPopover", () => {
it("renders a compact Stats trigger, panel closed until opened", () => {
render(
<FloatingTree>
<ChannelStatsPopover status={status()} />
</FloatingTree>,
);
expect(screen.getByRole("button", { name: "channels.stats.label" })).toBeTruthy();
expect(screen.queryByText("channels.stats.channels")).toBeNull();
});
it("reveals one row per stat with formatted values", () => {
open(status());
expect(screen.getByText("channels.stats.recentSynced")).toBeTruthy();
expect(screen.getByText("3/10")).toBeTruthy(); // recent-synced ratio
expect(screen.getByText("1/4")).toBeTruthy(); // full-history ratio
expect(screen.getByText((1234).toLocaleString())).toBeTruthy(); // my_videos, locale-agnostic
});
it("shows the 'left' ETA row only while a backfill is pending", () => {
open(status({ deep_pending_count: 0 }));
expect(screen.queryByText("channels.stats.left")).toBeNull();
cleanup();
open(status({ deep_pending_count: 2 }));
expect(screen.getByText("channels.stats.left")).toBeTruthy();
});
it("keeps each stat's explanation as a per-row title (hint not lost)", () => {
open(status());
const row = screen.getByText("channels.stats.channels").closest("div");
expect(row?.getAttribute("title")).toBe("channels.stats.channelsHint");
});
});
@@ -29,17 +29,26 @@ export default function ChannelStatsPopover({ status }: { status: MyStatus }) {
offset: 8,
});
const rows: { icon: LucideIcon; label: string; value: string | number }[] = [
{ icon: Users, label: t("channels.stats.channels"), value: status.channels_total },
// Each row keeps the per-stat explanation the old inline stats carried (as a native `title` on the
// row), so hovering a figure still tells the user what it counts — the guidance is not lost.
const rows: { icon: LucideIcon; label: string; value: string | number; hint: string }[] = [
{
icon: Users,
label: t("channels.stats.channels"),
value: status.channels_total,
hint: t("channels.stats.channelsHint"),
},
{
icon: RefreshCw,
label: t("channels.stats.recentSynced"),
value: `${status.channels_recent_synced}/${status.channels_total}`,
hint: t("channels.stats.recentSyncedHint"),
},
{
icon: History,
label: t("channels.stats.fullHistory"),
value: `${status.channels_deep_done}/${status.channels_deep_requested}`,
hint: t("channels.stats.fullHistoryHint"),
},
...(status.deep_pending_count > 0
? [
@@ -47,20 +56,28 @@ export default function ChannelStatsPopover({ status }: { status: MyStatus }) {
icon: Clock,
label: t("channels.stats.left"),
value: formatEta(status.deep_eta_seconds),
hint: t("channels.stats.leftHint", { count: status.deep_pending_count }),
},
]
: []),
{ icon: Film, label: t("channels.stats.myVideos"), value: status.my_videos.toLocaleString() },
{
icon: Film,
label: t("channels.stats.myVideos"),
value: status.my_videos.toLocaleString(),
hint: t("channels.stats.myVideosHint"),
},
{
icon: Zap,
label: t("channels.stats.quotaLeft"),
value: status.quota_remaining_today.toLocaleString(),
hint: t("channels.stats.quotaLeftHint"),
},
];
return (
<>
<button
type="button"
ref={pop.refs.setReference}
{...pop.getReferenceProps()}
className="glass-card glass-hover inline-flex items-center gap-1.5 rounded-lg px-2.5 py-1.5 text-sm text-muted transition"
@@ -74,7 +91,11 @@ export default function ChannelStatsPopover({ status }: { status: MyStatus }) {
className="glass-menu w-60 rounded-xl p-2 text-xs animate-[popIn_0.16s_ease]"
>
{rows.map((r) => (
<div key={r.label} className="flex items-center gap-2.5 px-1.5 py-1.5">
<div
key={r.label}
title={r.hint}
className="flex cursor-help items-center gap-2.5 px-1.5 py-1.5"
>
<r.icon className="h-4 w-4 shrink-0 text-muted" aria-hidden="true" />
<span className="flex-1 text-muted">{r.label}</span>
<span className="tabular-nums font-medium text-fg">{r.value}</span>
@@ -47,8 +47,6 @@
},
"tags": {
"label": "Tags",
"yourTags": "Your tags",
"manage": "Manage tags",
"yourTagsHint": "Your personal labels. Attach them to channels below, then filter the feed by tag from the sidebar. (Separate from the automatic language/topic tags.)",
"noneYet": "No tags yet — create one below.",
"create": "Create tag",
@@ -47,8 +47,6 @@
},
"tags": {
"label": "Címkék",
"yourTags": "Címkéid",
"manage": "Címkék kezelése",
"yourTagsHint": "Saját személyes címkéid. Csatold őket az alábbi csatornákhoz, majd szűrd a hírfolyamot címke szerint az oldalsávból. (Az automatikus nyelvi/téma címkéktől függetlenül.)",
"noneYet": "Még nincs címke — hozz létre egyet lent.",
"create": "Címke létrehozása",