fix(ui): focus, paging and state that a key or a finger can reach
- A key landing on a side panel's tab steps into the card it peeks open, not onto the hidden tab. - The Channels cards keep their status / pager row at the list's end while the band is folded. - A menu row's note shows on an enabled row too: the download state under 'Download…', not a title. - A removal chip hands the focus to its neighbour before it goes, for every caller (the feed's and Plex's active filters); the hidden-filter cue's own copy is gone. - Back to a spot within a page's end spacer lands at the page's end, not its top. - A running push / revert is tracked per list: another list's push is no longer refused unseen.
This commit is contained in:
@@ -771,6 +771,14 @@ export default function Channels() {
|
|||||||
rowProps={rowMenu.triggerProps}
|
rowProps={rowMenu.triggerProps}
|
||||||
rowClassName={() => "group"}
|
rowClassName={() => "group"}
|
||||||
/>
|
/>
|
||||||
|
{/* Folded, the status / pager row leaves the band, as the table's does — and the
|
||||||
|
list's own end keeps it here too (round 2, F2: the cards had lost their pager). */}
|
||||||
|
{fold.folded && (
|
||||||
|
<div className="pt-3 flex items-center justify-between gap-3 flex-wrap">
|
||||||
|
{statusChips}
|
||||||
|
{cardPager}
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
</>
|
</>
|
||||||
)}
|
)}
|
||||||
<ContextMenu menu={rowMenu} ariaLabel={t("channels.row.menu")} items={menuItems} />
|
<ContextMenu menu={rowMenu} ariaLabel={t("channels.row.menu")} items={menuItems} />
|
||||||
|
|||||||
@@ -34,4 +34,11 @@ describe("the Channels band on a short window", () => {
|
|||||||
expect(src).toMatch(/controlsPosition=\{fold\.folded \? "bottom" : "both"\}/);
|
expect(src).toMatch(/controlsPosition=\{fold\.folded \? "bottom" : "both"\}/);
|
||||||
expect(src).toMatch(/layout === "cards" && !fold\.folded/);
|
expect(src).toMatch(/layout === "cards" && !fold\.folded/);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Round 2 (F2): folded, the table's status / pager row moves to the list's end; the cards' one
|
||||||
|
// simply vanished, and with it the only way to page them.
|
||||||
|
it("keeps the cards' status / pager row at the list's end while folded", () => {
|
||||||
|
const grid = src.slice(src.indexOf("<ChannelLayoutGrid"));
|
||||||
|
expect(grid).toMatch(/\{fold\.folded && \(\s*<div[^>]*>\s*\{statusChips\}\s*\{cardPager\}/);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -42,16 +42,20 @@ describe("ActionMenu", () => {
|
|||||||
|
|
||||||
// floating-ui never computes a position in jsdom, so the surface stays visibility:hidden; the items
|
// 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.
|
// are in the DOM but the a11y tree treats them as hidden — hence `{ hidden: true }` on role queries.
|
||||||
it("says why a disabled row is off, under its label — and only on a disabled row", () => {
|
it("says under its label why a disabled row is off, and what an enabled one opens stands at", () => {
|
||||||
// A `title` on a disabled row reached nobody: a disabled button fires no pointer events and
|
// A `title` on a disabled row reached nobody: a disabled button fires no pointer events and
|
||||||
// roving focus skips it. The reason is visible text now, undimmed so it can be read.
|
// roving focus skips it. The reason is visible text now, undimmed so it can be read. An enabled
|
||||||
|
// row's state ("Downloaded" under "Download…") is the same line: a `title` shows to a resting
|
||||||
|
// mouse only, never to a key or a finger (round 2, F5).
|
||||||
renderMenu([
|
renderMenu([
|
||||||
{ id: "move", label: "Move to", onSelect: vi.fn(), disabled: true, note: "No other list" },
|
{ id: "move", label: "Move to", onSelect: vi.fn(), disabled: true, note: "No other list" },
|
||||||
{ id: "sync", label: "Sync", onSelect: vi.fn(), note: "never shown" },
|
{ id: "dl", label: "Download…", onSelect: vi.fn(), note: "Downloaded" },
|
||||||
|
{ id: "sync", label: "Sync", onSelect: vi.fn() },
|
||||||
]);
|
]);
|
||||||
fireEvent.click(screen.getByRole("button", { name: "Catalog actions" }));
|
fireEvent.click(screen.getByRole("button", { name: "Catalog actions" }));
|
||||||
const [move, sync] = screen.getAllByRole("menuitem", { hidden: true });
|
const [move, dl, sync] = screen.getAllByRole("menuitem", { hidden: true });
|
||||||
expect(move!.textContent).toBe("Move toNo other list");
|
expect(move!.textContent).toBe("Move toNo other list");
|
||||||
|
expect(dl!.textContent).toBe("Download…Downloaded");
|
||||||
expect(sync!.textContent).toBe("Sync");
|
expect(sync!.textContent).toBe("Sync");
|
||||||
const note = screen.getByText("No other list");
|
const note = screen.getByText("No other list");
|
||||||
expect(note.closest(".opacity-50")).toBeNull();
|
expect(note.closest(".opacity-50")).toBeNull();
|
||||||
|
|||||||
@@ -27,14 +27,12 @@ const ROW =
|
|||||||
"flex w-full items-center gap-2.5 rounded-lg px-2 py-1.5 text-left text-sm outline-hidden transition";
|
"flex w-full items-center gap-2.5 rounded-lg px-2 py-1.5 text-left text-sm outline-hidden transition";
|
||||||
const dim = (disabled: boolean | undefined) => (disabled ? " opacity-50" : "");
|
const dim = (disabled: boolean | undefined) => (disabled ? " opacity-50" : "");
|
||||||
|
|
||||||
/** The label, and under it — on a disabled row only — the reason it is off. */
|
/** The label, and under it the row's note: why it is off, or the state of what it opens. */
|
||||||
function RowLabel({ item }: { item: MenuItemBase }) {
|
function RowLabel({ item }: { item: MenuItemBase }) {
|
||||||
return (
|
return (
|
||||||
<span className="min-w-0 flex-1">
|
<span className="min-w-0 flex-1">
|
||||||
<span className={`block truncate${dim(item.disabled)}`}>{item.label}</span>
|
<span className={`block truncate${dim(item.disabled)}`}>{item.label}</span>
|
||||||
{item.disabled && item.note && (
|
{item.note && <span className="block text-2xs leading-snug text-muted">{item.note}</span>}
|
||||||
<span className="block text-2xs leading-snug text-muted">{item.note}</span>
|
|
||||||
)}
|
|
||||||
</span>
|
</span>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
@@ -50,7 +48,9 @@ interface MenuItemBase {
|
|||||||
* fires no pointer events (no tooltip) and roving focus skips it, so a reason there reached
|
* fires no pointer events (no tooltip) and roving focus skips it, so a reason there reached
|
||||||
* nobody. An `aria-disabled` row was tried and reverted (E10 review rounds 1–2: floating-ui skips
|
* nobody. An `aria-disabled` row was tried and reverted (E10 review rounds 1–2: floating-ui skips
|
||||||
* it identically, and the mouse got a highlight that then no-opped). Visible text is the one
|
* it identically, and the mouse got a highlight that then no-opped). Visible text is the one
|
||||||
* place a disabled row's reason can be: seen, and read in a screen reader's browse mode. */
|
* place a disabled row's reason can be: seen, and read in a screen reader's browse mode.
|
||||||
|
* On an ENABLED row it is the state of what the row opens ("Downloaded" under "Download…"),
|
||||||
|
* for the same reason: a `title` shows to a resting mouse only (round 2, F5). */
|
||||||
note?: string;
|
note?: string;
|
||||||
/** Draw a divider above this item. Ignored on the first item — a menu never opens with a rule. */
|
/** Draw a divider above this item. Ignored on the first item — a menu never opens with a rule. */
|
||||||
separatorBefore?: boolean;
|
separatorBefore?: boolean;
|
||||||
|
|||||||
@@ -3,7 +3,8 @@
|
|||||||
// E28 S3b: the chip family (docs/DESIGN.md "One primitive per button family"). One geometry for a
|
// E28 S3b: the chip family (docs/DESIGN.md "One primitive per button family"). One geometry for a
|
||||||
// filter, an applied narrowing, a tag and a suggestion, where five files had five.
|
// filter, an applied narrowing, a tag and a suggestion, where five files had five.
|
||||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||||
import { cleanup, fireEvent, render, screen } from "@testing-library/react";
|
import { act, cleanup, fireEvent, render, screen } from "@testing-library/react";
|
||||||
|
import { useState } from "react";
|
||||||
import { Chip, DismissibleChip } from "./Chip";
|
import { Chip, DismissibleChip } from "./Chip";
|
||||||
import { chipClass } from "./chipStyles";
|
import { chipClass } from "./chipStyles";
|
||||||
|
|
||||||
@@ -41,6 +42,34 @@ describe("Chip", () => {
|
|||||||
expect(onClick).toHaveBeenCalledOnce();
|
expect(onClick).toHaveBeenCalledOnce();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Round 2 (F6): the feed's and Plex's active-filter chips unmount with the filter they remove,
|
||||||
|
// and a keyboard press left the focus on <body>. The chip hands it on itself, for every caller.
|
||||||
|
it("a removing chip pressed from the keyboard hands the focus to its neighbour", () => {
|
||||||
|
function Row() {
|
||||||
|
const [chips, setChips] = useState(["Music", "News"]);
|
||||||
|
return (
|
||||||
|
<div>
|
||||||
|
{chips.map((c) => (
|
||||||
|
<Chip
|
||||||
|
key={c}
|
||||||
|
variant="token"
|
||||||
|
removeLabel={`Remove ${c}`}
|
||||||
|
onClick={() => setChips((all) => all.filter((x) => x !== c))}
|
||||||
|
>
|
||||||
|
{c}
|
||||||
|
</Chip>
|
||||||
|
))}
|
||||||
|
</div>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
render(<Row />);
|
||||||
|
const music = screen.getByRole("button", { name: "Remove Music" });
|
||||||
|
act(() => music.focus());
|
||||||
|
fireEvent.click(music, { detail: 0 });
|
||||||
|
expect(screen.queryByRole("button", { name: "Remove Music" })).toBeNull();
|
||||||
|
expect(document.activeElement).toBe(screen.getByRole("button", { name: "Remove News" }));
|
||||||
|
});
|
||||||
|
|
||||||
it("the measuring mirror lays out exactly like the chip it measures", () => {
|
it("the measuring mirror lays out exactly like the chip it measures", () => {
|
||||||
render(
|
render(
|
||||||
<Chip variant="tag" size="xs">
|
<Chip variant="tag" size="xs">
|
||||||
|
|||||||
@@ -27,7 +27,8 @@ type ChipProps = ButtonHTMLAttributes<HTMLButtonElement> & {
|
|||||||
pressed?: boolean;
|
pressed?: boolean;
|
||||||
/** Leading glyph (a lucide element). */
|
/** Leading glyph (a lucide element). */
|
||||||
icon?: ReactNode;
|
icon?: ReactNode;
|
||||||
/** The whole chip removes what it stands for: a trailing × and the removal as its name. */
|
/** The whole chip removes what it stands for: a trailing × and the removal as its name. A press
|
||||||
|
* that holds the focus hands it to the chip's neighbour before the removal, not to <body>. */
|
||||||
removeLabel?: string;
|
removeLabel?: string;
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -42,6 +43,7 @@ export const Chip = forwardRef<HTMLButtonElement, ChipProps>(function Chip(
|
|||||||
type,
|
type,
|
||||||
className,
|
className,
|
||||||
children,
|
children,
|
||||||
|
onClick,
|
||||||
...rest
|
...rest
|
||||||
},
|
},
|
||||||
ref,
|
ref,
|
||||||
@@ -54,6 +56,14 @@ export const Chip = forwardRef<HTMLButtonElement, ChipProps>(function Chip(
|
|||||||
aria-label={removeLabel}
|
aria-label={removeLabel}
|
||||||
title={removeLabel}
|
title={removeLabel}
|
||||||
className={clsx(chipClass(variant, size), removeLabel && "pr-1.5", className)}
|
className={clsx(chipClass(variant, size), removeLabel && "pr-1.5", className)}
|
||||||
|
onClick={(e) => {
|
||||||
|
// A removal chip unmounts with what it removes: from the keyboard it holds the focus, which
|
||||||
|
// went to <body> (round 2, F6 — the active-filter chips of the feed and Plex). Every
|
||||||
|
// removal chip gets it here, once, instead of each caller remembering.
|
||||||
|
if (removeLabel && e.currentTarget === document.activeElement)
|
||||||
|
focusNeighbour(e.currentTarget);
|
||||||
|
onClick?.(e);
|
||||||
|
}}
|
||||||
{...rest}
|
{...rest}
|
||||||
>
|
>
|
||||||
{icon}
|
{icon}
|
||||||
|
|||||||
@@ -1,7 +1,5 @@
|
|||||||
import type { MouseEvent } from "react";
|
|
||||||
import { useTranslation } from "react-i18next";
|
import { useTranslation } from "react-i18next";
|
||||||
import clsx from "clsx";
|
import clsx from "clsx";
|
||||||
import { focusNeighbour } from "../../lib/useFocusKeeper";
|
|
||||||
import {
|
import {
|
||||||
columnHideClass,
|
columnHideClass,
|
||||||
filterValueLabel,
|
filterValueLabel,
|
||||||
@@ -62,11 +60,8 @@ export function HiddenFilterCue<T>({
|
|||||||
variant="token"
|
variant="token"
|
||||||
className={cue(col)}
|
className={cue(col)}
|
||||||
removeLabel={t("datatable.hiddenFilter.remove", { name: text })}
|
removeLabel={t("datatable.hiddenFilter.remove", { name: text })}
|
||||||
onClick={(e: MouseEvent<HTMLButtonElement>) => {
|
// A removal chip hands its focus on before it goes (ui/Chip).
|
||||||
// The chip goes with its filter: hand the focus on first, not to <body>.
|
onClick={() => onFilter(col.key, Array.isArray(filters[col.key]) ? [] : "")}
|
||||||
focusNeighbour(e.currentTarget);
|
|
||||||
onFilter(col.key, Array.isArray(filters[col.key]) ? [] : "");
|
|
||||||
}}
|
|
||||||
>
|
>
|
||||||
<span className="max-w-56 truncate">{text}</span>
|
<span className="max-w-56 truncate">{text}</span>
|
||||||
</Chip>
|
</Chip>
|
||||||
|
|||||||
@@ -81,6 +81,50 @@ describe("PageScroller scroll memory", () => {
|
|||||||
expect(main.scrollTop).toBe(200);
|
expect(main.scrollTop).toBe(200);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Round 2 (F8): the page's very end, whose Back-to-top spacer only shows once scrolled, is out of
|
||||||
|
// reach when Back rebuilds the page — and the restore gave up with the page at the top.
|
||||||
|
it("Back to a spot within the end's spacer lands at the page's end, not its top", () => {
|
||||||
|
setEntry(1);
|
||||||
|
const { container } = render(<PageScroller scrollKey="e:k1">x</PageScroller>);
|
||||||
|
const main = sizedMain(container);
|
||||||
|
main.scrollTop = 500; // the very end, the spacer showing
|
||||||
|
fireEvent.scroll(main);
|
||||||
|
setEntry(2);
|
||||||
|
notifyNav();
|
||||||
|
Object.defineProperty(main, "scrollHeight", { configurable: true, value: 928 }); // no spacer
|
||||||
|
setEntry(1);
|
||||||
|
notifyNav();
|
||||||
|
expect(main.scrollTop).toBe(428);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("a list still far short of the spot waits for it rather than settling early", () => {
|
||||||
|
let frames: FrameRequestCallback[] = [];
|
||||||
|
vi.stubGlobal("requestAnimationFrame", (cb: FrameRequestCallback) => frames.push(cb));
|
||||||
|
const flush = (n: number) => {
|
||||||
|
for (let i = 0; i < n; i++) {
|
||||||
|
const run = frames;
|
||||||
|
frames = [];
|
||||||
|
for (const cb of run) cb(0);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
setEntry(1);
|
||||||
|
const { container } = render(<PageScroller scrollKey="e:k1">x</PageScroller>);
|
||||||
|
const main = sizedMain(container);
|
||||||
|
main.scrollTop = 450;
|
||||||
|
fireEvent.scroll(main);
|
||||||
|
setEntry(2);
|
||||||
|
notifyNav();
|
||||||
|
flush(1);
|
||||||
|
Object.defineProperty(main, "scrollHeight", { configurable: true, value: 600 }); // loading
|
||||||
|
setEntry(1);
|
||||||
|
notifyNav();
|
||||||
|
flush(20); // held still, but 350px short: no answer yet
|
||||||
|
expect(main.scrollTop).toBe(0);
|
||||||
|
Object.defineProperty(main, "scrollHeight", { configurable: true, value: 1000 }); // loaded
|
||||||
|
flush(1);
|
||||||
|
expect(main.scrollTop).toBe(450);
|
||||||
|
});
|
||||||
|
|
||||||
it("restores per content key within one entry: a new key resets to top, returning restores", () => {
|
it("restores per content key within one entry: a new key resets to top, returning restores", () => {
|
||||||
setEntry(10);
|
setEntry(10);
|
||||||
const { container, rerender } = render(<PageScroller scrollKey="e10:a">x</PageScroller>);
|
const { container, rerender } = render(<PageScroller scrollKey="e10:a">x</PageScroller>);
|
||||||
|
|||||||
@@ -28,6 +28,12 @@ function rememberScroll(key: string, top: number): void {
|
|||||||
// Put the scroller back to `top`. The feed is virtualized, so on a remount the list needs a frame
|
// Put the scroller back to `top`. The feed is virtualized, so on a remount the list needs a frame
|
||||||
// or two to render enough rows to reach its full height; an immediate `scrollTop` would otherwise
|
// or two to render enough rows to reach its full height; an immediate `scrollTop` would otherwise
|
||||||
// clamp short. Re-apply over a few frames until it sticks (or the content is simply shorter now).
|
// clamp short. Re-apply over a few frames until it sticks (or the content is simply shorter now).
|
||||||
|
// A spot the page can no longer reach — the last stretch of a page scrolled to its very end, whose
|
||||||
|
// Back-to-top spacer only shows once scrolled — goes as far as the page does: it used to be left at
|
||||||
|
// the top (round 2, F8). Only a spacer's worth short, and only once the height has held still, is
|
||||||
|
// that answer early; a list still loading is further short than that, and gets the full wait.
|
||||||
|
const SETTLED_FRAMES = 10;
|
||||||
|
const SPACER_REM = 6;
|
||||||
function restoreScroll(el: HTMLElement, top: number): () => void {
|
function restoreScroll(el: HTMLElement, top: number): () => void {
|
||||||
if (top <= 0) {
|
if (top <= 0) {
|
||||||
el.scrollTop = 0;
|
el.scrollTop = 0;
|
||||||
@@ -35,11 +41,22 @@ function restoreScroll(el: HTMLElement, top: number): () => void {
|
|||||||
}
|
}
|
||||||
let raf = 0;
|
let raf = 0;
|
||||||
let frames = 0;
|
let frames = 0;
|
||||||
|
let still = 0;
|
||||||
|
let lastMax = -1;
|
||||||
let cancelled = false;
|
let cancelled = false;
|
||||||
const tick = () => {
|
const tick = () => {
|
||||||
if (cancelled) return;
|
if (cancelled) return;
|
||||||
if (el.scrollHeight - el.clientHeight >= top) el.scrollTop = top;
|
const max = el.scrollHeight - el.clientHeight;
|
||||||
if (el.scrollTop >= top - 1 || frames++ > 90) return; // reached, or gave up after ~1.5s
|
if (max >= top) el.scrollTop = top;
|
||||||
|
if (el.scrollTop >= top - 1) return; // reached
|
||||||
|
still = max === lastMax ? still + 1 : 0;
|
||||||
|
lastMax = max;
|
||||||
|
const rem = parseFloat(getComputedStyle(document.documentElement).fontSize) || 16;
|
||||||
|
const nearEnd = max > 0 && top - max <= SPACER_REM * rem;
|
||||||
|
if ((nearEnd && still >= SETTLED_FRAMES) || frames++ > 90) {
|
||||||
|
el.scrollTop = max; // as near as the page reaches now
|
||||||
|
return;
|
||||||
|
}
|
||||||
raf = requestAnimationFrame(tick);
|
raf = requestAnimationFrame(tick);
|
||||||
};
|
};
|
||||||
raf = requestAnimationFrame(tick);
|
raf = requestAnimationFrame(tick);
|
||||||
|
|||||||
@@ -6,7 +6,7 @@
|
|||||||
// hover-to-peek and click-to-pin. jsdom has no matchMedia: the stub answers the two queries SidePanel
|
// hover-to-peek and click-to-pin. jsdom has no matchMedia: the stub answers the two queries SidePanel
|
||||||
// asks from three switches, and can flip them live (a tablet turning).
|
// asks from three switches, and can flip them live (a tablet turning).
|
||||||
import { describe, it, expect, afterEach, vi } from "vitest";
|
import { describe, it, expect, afterEach, vi } from "vitest";
|
||||||
import { act, render, screen, fireEvent, cleanup } from "@testing-library/react";
|
import { act, render, screen, fireEvent, cleanup, waitFor } from "@testing-library/react";
|
||||||
|
|
||||||
vi.mock("react-i18next", async (importOriginal) => {
|
vi.mock("react-i18next", async (importOriginal) => {
|
||||||
const actual = await importOriginal<typeof import("react-i18next")>();
|
const actual = await importOriginal<typeof import("react-i18next")>();
|
||||||
@@ -105,6 +105,21 @@ describe("SidePanel · the tab under an open card", () => {
|
|||||||
mount(true);
|
mount(true);
|
||||||
expect(screen.getByRole("button", { name: "sidebar.pinNamed:Filters" }).tabIndex).toBe(0);
|
expect(screen.getByRole("button", { name: "sidebar.pinNamed:Filters" }).tabIndex).toBe(0);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("hands a key's focus on the tab to the card it peeks open, not to the hidden tab", async () => {
|
||||||
|
stubMedia({ coarse: false, narrow: false, wide: true });
|
||||||
|
globalThis.ResizeObserver ??= class {
|
||||||
|
observe() {}
|
||||||
|
unobserve() {}
|
||||||
|
disconnect() {}
|
||||||
|
} as unknown as typeof ResizeObserver;
|
||||||
|
mount(true);
|
||||||
|
const tab = screen.getByRole("button", { name: "sidebar.pinNamed:Filters" });
|
||||||
|
act(() => tab.focus()); // jsdom calls every focus :focus-visible — a key's focus, here
|
||||||
|
await waitFor(() => expect(document.activeElement?.closest("aside")).not.toBeNull());
|
||||||
|
expect(document.activeElement?.getAttribute("aria-label")).toBe("sidebar.pinNamed:Filters");
|
||||||
|
expect(document.activeElement).not.toBe(tab);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("SidePanel · on touch", () => {
|
describe("SidePanel · on touch", () => {
|
||||||
|
|||||||
@@ -160,6 +160,15 @@ export default function SidePanel({
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
// A key landing on the tab peeks the card open over it, and the tab then goes inert below —
|
||||||
|
// with the focus still on it: a hidden control, out of the tree, that Enter still pinned
|
||||||
|
// (round 2, F1). The focus steps into the card instead, onto its pin: the same control.
|
||||||
|
onFocus={(e) => {
|
||||||
|
if (drawerMode || pinned || !e.currentTarget.matches(":focus-visible")) return;
|
||||||
|
requestAnimationFrame(() =>
|
||||||
|
cardRef.current?.querySelector<HTMLElement>("button")?.focus(),
|
||||||
|
);
|
||||||
|
}}
|
||||||
// Under an open card the tab is transparent and inert: out of the Tab order AND of the
|
// Under an open card the tab is transparent and inert: out of the Tab order AND of the
|
||||||
// accessibility tree, where it would repeat the card's own pin button by name.
|
// accessibility tree, where it would repeat the card's own pin button by name.
|
||||||
tabIndex={expanded ? -1 : undefined}
|
tabIndex={expanded ? -1 : undefined}
|
||||||
|
|||||||
@@ -32,6 +32,8 @@ vi.mock("../lib/useMe", () => ({ useMe: () => ({ can_write: h.canWrite }) }));
|
|||||||
import { usePlaylistActions } from "./usePlaylistActions";
|
import { usePlaylistActions } from "./usePlaylistActions";
|
||||||
import { HttpError, api } from "../lib/api";
|
import { HttpError, api } from "../lib/api";
|
||||||
|
|
||||||
|
type Menuable = Parameters<ReturnType<typeof usePlaylistActions>["items"]>[0];
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
cleanup();
|
cleanup();
|
||||||
vi.restoreAllMocks();
|
vi.restoreAllMocks();
|
||||||
@@ -198,4 +200,30 @@ describe("usePlaylistActions — one push at a time, across menus", () => {
|
|||||||
});
|
});
|
||||||
expect(push(header).disabled).toBeFalsy();
|
expect(push(header).disabled).toBeFalsy();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Round 2 (F9): the running push was ONE slot for the app — another list's push, from a row that
|
||||||
|
// looked enabled, was refused without a word.
|
||||||
|
it("a push of one list leaves another list's push free, and its own revert off", async () => {
|
||||||
|
type Plan = Awaited<ReturnType<typeof api.playlistPushPlan>>;
|
||||||
|
const pending: ((v: Plan) => void)[] = [];
|
||||||
|
vi.spyOn(api, "playlistPushPlan").mockImplementation(
|
||||||
|
() => new Promise<Plan>((r) => pending.push(r)),
|
||||||
|
);
|
||||||
|
const m = mount();
|
||||||
|
const a: Menuable = { ...linked, dirty: true };
|
||||||
|
const b: Menuable = { ...linked, id: 5, dirty: true };
|
||||||
|
const row = (pl: Menuable, id: string) =>
|
||||||
|
m.result.current.items(pl, { onRename: () => {}, pin: [] }).find((i) => i.id === id)!;
|
||||||
|
act(() => row(a, "push").onSelect!());
|
||||||
|
expect(row(b, "push").disabled).toBeFalsy();
|
||||||
|
expect(row(a, "revert").disabled).toBe(true);
|
||||||
|
await act(async () => row(b, "push").onSelect!());
|
||||||
|
expect(api.playlistPushPlan).toHaveBeenCalledTimes(2);
|
||||||
|
expect(vi.mocked(api.playlistPushPlan).mock.calls.map((c) => c[0])).toEqual([4, 5]);
|
||||||
|
await act(async () => {
|
||||||
|
for (const r of pending) r({ action: "update", affordable: true } as Plan);
|
||||||
|
});
|
||||||
|
expect(row(a, "push").disabled).toBeFalsy();
|
||||||
|
expect(row(b, "push").disabled).toBeFalsy();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -25,14 +25,29 @@ import { useCopyItems } from "./copyItems";
|
|||||||
* and `PlaylistDetail` (the detail pane), which is why neither is named here. */
|
* and `PlaylistDetail` (the detail pane), which is why neither is named here. */
|
||||||
type Menuable = Pick<Playlist, "id" | "name" | "kind" | "yt_playlist_id" | "dirty">;
|
type Menuable = Pick<Playlist, "id" | "name" | "kind" | "yt_playlist_id" | "dirty">;
|
||||||
|
|
||||||
// Which list a push / revert is running for, so its row is off (and says so) until it is done. ONE
|
// Which lists a push / revert is running for, so their rows are off (and say so) until it is done.
|
||||||
// for the app, not per hook: the rail row's menu and the list page header's "…" are two instances at
|
// ONE for the app, not per hook: the rail row's menu and the list page header's "…" are two instances
|
||||||
// once, and a per-instance flag left the other offering a second push of the same list (E28 review
|
// at once, and a per-instance flag left the other offering a second push of the same list (E28 review
|
||||||
// C1). Read with `get()` at the start, so a second start refuses before any render has happened.
|
// C1). Per LIST, not one slot: a single id refused a push of ANOTHER list, silently, from a row that
|
||||||
const inFlight = createStore<{ pushing: number | null; reverting: number | null }>({
|
// looked enabled (round 2, F9) — and the server serializes a list's syncs anyway (`sync_claim`).
|
||||||
pushing: null,
|
// Read with `get()` at the start, so a second start refuses before any render has happened.
|
||||||
reverting: null,
|
type Running = ReadonlySet<number>;
|
||||||
|
const inFlight = createStore<{ pushing: Running; reverting: Running }>({
|
||||||
|
pushing: new Set(),
|
||||||
|
reverting: new Set(),
|
||||||
});
|
});
|
||||||
|
const mark = (kind: "pushing" | "reverting", id: number, on: boolean) =>
|
||||||
|
inFlight.set((s) => {
|
||||||
|
const next = new Set(s[kind]);
|
||||||
|
if (on) next.add(id);
|
||||||
|
else next.delete(id);
|
||||||
|
return { ...s, [kind]: next };
|
||||||
|
});
|
||||||
|
/** A push and a revert of one list write the same rows: either one running holds both off. */
|
||||||
|
const syncing = (id: number) => {
|
||||||
|
const s = inFlight.get();
|
||||||
|
return s.pushing.has(id) || s.reverting.has(id);
|
||||||
|
};
|
||||||
|
|
||||||
export function usePlaylistActions() {
|
export function usePlaylistActions() {
|
||||||
const qc = useQueryClient();
|
const qc = useQueryClient();
|
||||||
@@ -44,8 +59,6 @@ export function usePlaylistActions() {
|
|||||||
const { selectedId } = usePlaylists();
|
const { selectedId } = usePlaylists();
|
||||||
const { setSelectedId } = usePlaylistsActions();
|
const { setSelectedId } = usePlaylistsActions();
|
||||||
const { pushing, reverting } = inFlight.use();
|
const { pushing, reverting } = inFlight.use();
|
||||||
const setPushing = (id: number | null) => inFlight.set((s) => ({ ...s, pushing: id }));
|
|
||||||
const setReverting = (id: number | null) => inFlight.set((s) => ({ ...s, reverting: id }));
|
|
||||||
|
|
||||||
const refresh = (id: number) => {
|
const refresh = (id: number) => {
|
||||||
qc.invalidateQueries({ queryKey: qk.playlists() });
|
qc.invalidateQueries({ queryKey: qk.playlists() });
|
||||||
@@ -107,7 +120,7 @@ export function usePlaylistActions() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async function revert(pl: Menuable) {
|
async function revert(pl: Menuable) {
|
||||||
if (inFlight.get().reverting != null) return;
|
if (syncing(pl.id)) return;
|
||||||
const ok = await confirm({
|
const ok = await confirm({
|
||||||
title: t("playlists.revertTitle"),
|
title: t("playlists.revertTitle"),
|
||||||
message: t("playlists.revertMsg"),
|
message: t("playlists.revertMsg"),
|
||||||
@@ -115,8 +128,8 @@ export function usePlaylistActions() {
|
|||||||
danger: true,
|
danger: true,
|
||||||
});
|
});
|
||||||
// Re-read after the dialog: another menu may have started one while this one asked.
|
// Re-read after the dialog: another menu may have started one while this one asked.
|
||||||
if (!ok || inFlight.get().reverting != null) return;
|
if (!ok || syncing(pl.id)) return;
|
||||||
setReverting(pl.id);
|
mark("reverting", pl.id, true);
|
||||||
try {
|
try {
|
||||||
await api.revertPlaylist(pl.id);
|
await api.revertPlaylist(pl.id);
|
||||||
refresh(pl.id);
|
refresh(pl.id);
|
||||||
@@ -127,14 +140,14 @@ export function usePlaylistActions() {
|
|||||||
} catch (e) {
|
} catch (e) {
|
||||||
notifyYouTubeActionError(e, t("playlists.revertFailed"));
|
notifyYouTubeActionError(e, t("playlists.revertFailed"));
|
||||||
} finally {
|
} finally {
|
||||||
setReverting(null);
|
mark("reverting", pl.id, false);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
async function push(pl: Menuable) {
|
async function push(pl: Menuable) {
|
||||||
if (inFlight.get().pushing != null) return;
|
if (syncing(pl.id)) return;
|
||||||
const name = playlistName(pl, t);
|
const name = playlistName(pl, t);
|
||||||
setPushing(pl.id);
|
mark("pushing", pl.id, true);
|
||||||
try {
|
try {
|
||||||
const plan = await api.playlistPushPlan(pl.id);
|
const plan = await api.playlistPushPlan(pl.id);
|
||||||
const nothingToDo =
|
const nothingToDo =
|
||||||
@@ -209,7 +222,7 @@ export function usePlaylistActions() {
|
|||||||
} catch (e) {
|
} catch (e) {
|
||||||
notifyYouTubeActionError(e, t("playlists.pushFailed"));
|
notifyYouTubeActionError(e, t("playlists.pushFailed"));
|
||||||
} finally {
|
} finally {
|
||||||
setPushing(null);
|
mark("pushing", pl.id, false);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -228,7 +241,7 @@ export function usePlaylistActions() {
|
|||||||
const lockedNote = builtin
|
const lockedNote = builtin
|
||||||
? t("playlists.builtinLocked", { name: t("playlists.watchLater") })
|
? t("playlists.builtinLocked", { name: t("playlists.watchLater") })
|
||||||
: undefined;
|
: undefined;
|
||||||
const busy = (id: number | null) => id === pl.id;
|
const busy = pushing.has(pl.id) || reverting.has(pl.id);
|
||||||
const ytUrl = pl.yt_playlist_id
|
const ytUrl = pl.yt_playlist_id
|
||||||
? `https://www.youtube.com/playlist?list=${pl.yt_playlist_id}`
|
? `https://www.youtube.com/playlist?list=${pl.yt_playlist_id}`
|
||||||
: null;
|
: null;
|
||||||
@@ -250,14 +263,14 @@ export function usePlaylistActions() {
|
|||||||
{
|
{
|
||||||
id: "push",
|
id: "push",
|
||||||
label: linked ? t("playlists.pushToYoutube") : t("playlists.exportToYoutube"),
|
label: linked ? t("playlists.pushToYoutube") : t("playlists.exportToYoutube"),
|
||||||
icon: busy(pushing) ? (
|
icon: pushing.has(pl.id) ? (
|
||||||
<Upload className="w-4 h-4 animate-pulse" />
|
<Upload className="w-4 h-4 animate-pulse" />
|
||||||
) : linked ? (
|
) : linked ? (
|
||||||
<Youtube className="w-4 h-4" />
|
<Youtube className="w-4 h-4" />
|
||||||
) : (
|
) : (
|
||||||
<Upload className="w-4 h-4" />
|
<Upload className="w-4 h-4" />
|
||||||
),
|
),
|
||||||
disabled: busy(pushing),
|
disabled: busy,
|
||||||
onSelect: () => void push(pl),
|
onSelect: () => void push(pl),
|
||||||
},
|
},
|
||||||
]
|
]
|
||||||
@@ -267,9 +280,11 @@ export function usePlaylistActions() {
|
|||||||
{
|
{
|
||||||
id: "revert",
|
id: "revert",
|
||||||
label: t("playlists.revert"),
|
label: t("playlists.revert"),
|
||||||
icon: <RotateCcw className={`w-4 h-4${busy(reverting) ? " animate-spin" : ""}`} />,
|
icon: (
|
||||||
|
<RotateCcw className={`w-4 h-4${reverting.has(pl.id) ? " animate-spin" : ""}`} />
|
||||||
|
),
|
||||||
danger: true,
|
danger: true,
|
||||||
disabled: busy(reverting),
|
disabled: busy,
|
||||||
onSelect: () => void revert(pl),
|
onSelect: () => void revert(pl),
|
||||||
},
|
},
|
||||||
]
|
]
|
||||||
|
|||||||
@@ -44,13 +44,14 @@ function downloadRow(state: string | undefined) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
describe("the video menu's download row", () => {
|
describe("the video menu's download row", () => {
|
||||||
it("says what it does, whatever the state, and carries the state as its hint", async () => {
|
// Round 2 (F5): the state rode in a `title`, which a key or a finger never shows.
|
||||||
|
it("says what it does, whatever the state, and carries the state as its note", async () => {
|
||||||
const done = downloadRow("done");
|
const done = downloadRow("done");
|
||||||
await waitFor(() => expect(done().title).toBe("downloads.button.downloaded"));
|
await waitFor(() => expect(done().note).toBe("downloads.button.downloaded"));
|
||||||
expect(done().label).toBe("downloads.menu.download");
|
expect(done().label).toBe("downloads.menu.download");
|
||||||
cleanup();
|
cleanup();
|
||||||
const none = downloadRow(undefined);
|
const none = downloadRow(undefined);
|
||||||
await waitFor(() => expect(none().label).toBe("downloads.menu.download"));
|
await waitFor(() => expect(none().label).toBe("downloads.menu.download"));
|
||||||
expect(none().title).toBeUndefined();
|
expect(none().note).toBeUndefined();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -74,8 +74,9 @@ export function useVideoActions(handlers: VideoActionHandlers, open: boolean) {
|
|||||||
const hidden = v.status === "hidden";
|
const hidden = v.status === "hidden";
|
||||||
const inProgress = v.position_seconds > 0 && !watched;
|
const inProgress = v.position_seconds > 0 && !watched;
|
||||||
const dl = index.data?.[v.id];
|
const dl = index.data?.[v.id];
|
||||||
// The row opens the download dialog, so it says so; the download's state is its hint, not its
|
// The row opens the download dialog, so it says so; the download's state is the line under its
|
||||||
// label — "Downloaded" read as a status, not as a row that opens something (E28 review J4).
|
// label — "Downloaded" read as a status, not as a row that opens something (E28 review J4) — and
|
||||||
|
// not a `title`, which a key or a finger never shows (round 2, F5).
|
||||||
const dlState =
|
const dlState =
|
||||||
dl === "done"
|
dl === "done"
|
||||||
? t("downloads.button.downloaded")
|
? t("downloads.button.downloaded")
|
||||||
@@ -143,7 +144,7 @@ export function useVideoActions(handlers: VideoActionHandlers, open: boolean) {
|
|||||||
{
|
{
|
||||||
id: "download",
|
id: "download",
|
||||||
label: t("downloads.menu.download"),
|
label: t("downloads.menu.download"),
|
||||||
title: dlState,
|
note: dlState,
|
||||||
icon: <Download className={icon} />,
|
icon: <Download className={icon} />,
|
||||||
onSelect: () => setFollowUp({ kind: "download", video: v }),
|
onSelect: () => setFollowUp({ kind: "download", video: v }),
|
||||||
},
|
},
|
||||||
|
|||||||
Reference in New Issue
Block a user