From 740602b9bfc738b8d9f0902eab79e8ed0010c0d3 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 27 Sep 2026 19:43:04 +0200 Subject: [PATCH] fix(ui): focus, paging and state that a key or a finger can reach MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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. --- frontend/src/components/Channels.tsx | 8 +++ .../channelsTagRow.contract.test.ts | 7 +++ .../src/components/ui/ActionMenu.dom.test.tsx | 12 ++-- frontend/src/components/ui/ActionMenu.tsx | 10 ++-- frontend/src/components/ui/Chip.dom.test.tsx | 31 +++++++++- frontend/src/components/ui/Chip.tsx | 12 +++- .../src/components/ui/HiddenFilterCue.tsx | 9 +-- .../components/ui/PageScroller.dom.test.tsx | 44 ++++++++++++++ frontend/src/components/ui/PageScroller.tsx | 21 ++++++- .../ui/SidePanel.drawer.dom.test.tsx | 17 +++++- frontend/src/components/ui/SidePanel.tsx | 9 +++ .../usePlaylistActions.dom.test.tsx | 28 +++++++++ .../src/components/usePlaylistActions.tsx | 57 ++++++++++++------- .../components/useVideoActions.dom.test.tsx | 7 ++- frontend/src/components/useVideoActions.tsx | 7 ++- 15 files changed, 231 insertions(+), 48 deletions(-) diff --git a/frontend/src/components/Channels.tsx b/frontend/src/components/Channels.tsx index 6778926b..14c2e9c5 100644 --- a/frontend/src/components/Channels.tsx +++ b/frontend/src/components/Channels.tsx @@ -771,6 +771,14 @@ export default function Channels() { rowProps={rowMenu.triggerProps} 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 && ( +
+ {statusChips} + {cardPager} +
+ )} )} diff --git a/frontend/src/components/channelsTagRow.contract.test.ts b/frontend/src/components/channelsTagRow.contract.test.ts index daaf82bd..2db52de9 100644 --- a/frontend/src/components/channelsTagRow.contract.test.ts +++ b/frontend/src/components/channelsTagRow.contract.test.ts @@ -34,4 +34,11 @@ describe("the Channels band on a short window", () => { expect(src).toMatch(/controlsPosition=\{fold\.folded \? "bottom" : "both"\}/); 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("]*>\s*\{statusChips\}\s*\{cardPager\}/); + }); }); diff --git a/frontend/src/components/ui/ActionMenu.dom.test.tsx b/frontend/src/components/ui/ActionMenu.dom.test.tsx index 4b4928e4..7f4a9a42 100644 --- a/frontend/src/components/ui/ActionMenu.dom.test.tsx +++ b/frontend/src/components/ui/ActionMenu.dom.test.tsx @@ -42,16 +42,20 @@ describe("ActionMenu", () => { // 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("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 - // 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([ { 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" })); - 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(dl!.textContent).toBe("Download…Downloaded"); expect(sync!.textContent).toBe("Sync"); const note = screen.getByText("No other list"); expect(note.closest(".opacity-50")).toBeNull(); diff --git a/frontend/src/components/ui/ActionMenu.tsx b/frontend/src/components/ui/ActionMenu.tsx index 6f1817ad..72dda344 100644 --- a/frontend/src/components/ui/ActionMenu.tsx +++ b/frontend/src/components/ui/ActionMenu.tsx @@ -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"; 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 }) { return ( {item.label} - {item.disabled && item.note && ( - {item.note} - )} + {item.note && {item.note}} ); } @@ -50,7 +48,9 @@ interface MenuItemBase { * 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 * 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; /** Draw a divider above this item. Ignored on the first item — a menu never opens with a rule. */ separatorBefore?: boolean; diff --git a/frontend/src/components/ui/Chip.dom.test.tsx b/frontend/src/components/ui/Chip.dom.test.tsx index fc036f02..ff30b4fb 100644 --- a/frontend/src/components/ui/Chip.dom.test.tsx +++ b/frontend/src/components/ui/Chip.dom.test.tsx @@ -3,7 +3,8 @@ // 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. 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 { chipClass } from "./chipStyles"; @@ -41,6 +42,34 @@ describe("Chip", () => { 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 . 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 ( +
+ {chips.map((c) => ( + setChips((all) => all.filter((x) => x !== c))} + > + {c} + + ))} +
+ ); + } + render(); + 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", () => { render( diff --git a/frontend/src/components/ui/Chip.tsx b/frontend/src/components/ui/Chip.tsx index cf859e02..1f9b0716 100644 --- a/frontend/src/components/ui/Chip.tsx +++ b/frontend/src/components/ui/Chip.tsx @@ -27,7 +27,8 @@ type ChipProps = ButtonHTMLAttributes & { pressed?: boolean; /** Leading glyph (a lucide element). */ 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 . */ removeLabel?: string; }; @@ -42,6 +43,7 @@ export const Chip = forwardRef(function Chip( type, className, children, + onClick, ...rest }, ref, @@ -54,6 +56,14 @@ export const Chip = forwardRef(function Chip( aria-label={removeLabel} title={removeLabel} 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 (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} > {icon} diff --git a/frontend/src/components/ui/HiddenFilterCue.tsx b/frontend/src/components/ui/HiddenFilterCue.tsx index d82e7205..22c4ad25 100644 --- a/frontend/src/components/ui/HiddenFilterCue.tsx +++ b/frontend/src/components/ui/HiddenFilterCue.tsx @@ -1,7 +1,5 @@ -import type { MouseEvent } from "react"; import { useTranslation } from "react-i18next"; import clsx from "clsx"; -import { focusNeighbour } from "../../lib/useFocusKeeper"; import { columnHideClass, filterValueLabel, @@ -62,11 +60,8 @@ export function HiddenFilterCue({ variant="token" className={cue(col)} removeLabel={t("datatable.hiddenFilter.remove", { name: text })} - onClick={(e: MouseEvent) => { - // The chip goes with its filter: hand the focus on first, not to . - focusNeighbour(e.currentTarget); - onFilter(col.key, Array.isArray(filters[col.key]) ? [] : ""); - }} + // A removal chip hands its focus on before it goes (ui/Chip). + onClick={() => onFilter(col.key, Array.isArray(filters[col.key]) ? [] : "")} > {text} diff --git a/frontend/src/components/ui/PageScroller.dom.test.tsx b/frontend/src/components/ui/PageScroller.dom.test.tsx index c7095ac7..a0da1d53 100644 --- a/frontend/src/components/ui/PageScroller.dom.test.tsx +++ b/frontend/src/components/ui/PageScroller.dom.test.tsx @@ -81,6 +81,50 @@ describe("PageScroller scroll memory", () => { 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(x); + 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(x); + 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", () => { setEntry(10); const { container, rerender } = render(x); diff --git a/frontend/src/components/ui/PageScroller.tsx b/frontend/src/components/ui/PageScroller.tsx index 3b5add0e..92234d85 100644 --- a/frontend/src/components/ui/PageScroller.tsx +++ b/frontend/src/components/ui/PageScroller.tsx @@ -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 // 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). +// 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 { if (top <= 0) { el.scrollTop = 0; @@ -35,11 +41,22 @@ function restoreScroll(el: HTMLElement, top: number): () => void { } let raf = 0; let frames = 0; + let still = 0; + let lastMax = -1; let cancelled = false; const tick = () => { if (cancelled) return; - if (el.scrollHeight - el.clientHeight >= top) el.scrollTop = top; - if (el.scrollTop >= top - 1 || frames++ > 90) return; // reached, or gave up after ~1.5s + const max = el.scrollHeight - el.clientHeight; + 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); diff --git a/frontend/src/components/ui/SidePanel.drawer.dom.test.tsx b/frontend/src/components/ui/SidePanel.drawer.dom.test.tsx index 9d3c35ef..ac7dcf05 100644 --- a/frontend/src/components/ui/SidePanel.drawer.dom.test.tsx +++ b/frontend/src/components/ui/SidePanel.drawer.dom.test.tsx @@ -6,7 +6,7 @@ // 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). 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) => { const actual = await importOriginal(); @@ -105,6 +105,21 @@ describe("SidePanel · the tab under an open card", () => { mount(true); 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", () => { diff --git a/frontend/src/components/ui/SidePanel.tsx b/frontend/src/components/ui/SidePanel.tsx index bb56ba01..54fe4376 100644 --- a/frontend/src/components/ui/SidePanel.tsx +++ b/frontend/src/components/ui/SidePanel.tsx @@ -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("button")?.focus(), + ); + }} // 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. tabIndex={expanded ? -1 : undefined} diff --git a/frontend/src/components/usePlaylistActions.dom.test.tsx b/frontend/src/components/usePlaylistActions.dom.test.tsx index 6ff5cf38..3f88c256 100644 --- a/frontend/src/components/usePlaylistActions.dom.test.tsx +++ b/frontend/src/components/usePlaylistActions.dom.test.tsx @@ -32,6 +32,8 @@ vi.mock("../lib/useMe", () => ({ useMe: () => ({ can_write: h.canWrite }) })); import { usePlaylistActions } from "./usePlaylistActions"; import { HttpError, api } from "../lib/api"; +type Menuable = Parameters["items"]>[0]; + afterEach(() => { cleanup(); vi.restoreAllMocks(); @@ -198,4 +200,30 @@ describe("usePlaylistActions — one push at a time, across menus", () => { }); 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>; + const pending: ((v: Plan) => void)[] = []; + vi.spyOn(api, "playlistPushPlan").mockImplementation( + () => new Promise((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(); + }); }); diff --git a/frontend/src/components/usePlaylistActions.tsx b/frontend/src/components/usePlaylistActions.tsx index 80f0fbc8..c502fb7f 100644 --- a/frontend/src/components/usePlaylistActions.tsx +++ b/frontend/src/components/usePlaylistActions.tsx @@ -25,14 +25,29 @@ import { useCopyItems } from "./copyItems"; * and `PlaylistDetail` (the detail pane), which is why neither is named here. */ type Menuable = Pick; -// Which list a push / revert is running for, so its row is off (and says so) until it is done. ONE -// for the app, not per hook: the rail row's menu and the list page header's "…" are two instances 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. -const inFlight = createStore<{ pushing: number | null; reverting: number | null }>({ - pushing: null, - reverting: null, +// Which lists a push / revert is running for, so their rows are off (and say so) until it is done. +// ONE for the app, not per hook: the rail row's menu and the list page header's "…" are two instances +// at once, and a per-instance flag left the other offering a second push of the same list (E28 review +// C1). Per LIST, not one slot: a single id refused a push of ANOTHER list, silently, from a row that +// looked enabled (round 2, F9) — and the server serializes a list's syncs anyway (`sync_claim`). +// Read with `get()` at the start, so a second start refuses before any render has happened. +type Running = ReadonlySet; +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() { const qc = useQueryClient(); @@ -44,8 +59,6 @@ export function usePlaylistActions() { const { selectedId } = usePlaylists(); const { setSelectedId } = usePlaylistsActions(); 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) => { qc.invalidateQueries({ queryKey: qk.playlists() }); @@ -107,7 +120,7 @@ export function usePlaylistActions() { } async function revert(pl: Menuable) { - if (inFlight.get().reverting != null) return; + if (syncing(pl.id)) return; const ok = await confirm({ title: t("playlists.revertTitle"), message: t("playlists.revertMsg"), @@ -115,8 +128,8 @@ export function usePlaylistActions() { danger: true, }); // Re-read after the dialog: another menu may have started one while this one asked. - if (!ok || inFlight.get().reverting != null) return; - setReverting(pl.id); + if (!ok || syncing(pl.id)) return; + mark("reverting", pl.id, true); try { await api.revertPlaylist(pl.id); refresh(pl.id); @@ -127,14 +140,14 @@ export function usePlaylistActions() { } catch (e) { notifyYouTubeActionError(e, t("playlists.revertFailed")); } finally { - setReverting(null); + mark("reverting", pl.id, false); } } async function push(pl: Menuable) { - if (inFlight.get().pushing != null) return; + if (syncing(pl.id)) return; const name = playlistName(pl, t); - setPushing(pl.id); + mark("pushing", pl.id, true); try { const plan = await api.playlistPushPlan(pl.id); const nothingToDo = @@ -209,7 +222,7 @@ export function usePlaylistActions() { } catch (e) { notifyYouTubeActionError(e, t("playlists.pushFailed")); } finally { - setPushing(null); + mark("pushing", pl.id, false); } } @@ -228,7 +241,7 @@ export function usePlaylistActions() { const lockedNote = builtin ? t("playlists.builtinLocked", { name: t("playlists.watchLater") }) : 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 ? `https://www.youtube.com/playlist?list=${pl.yt_playlist_id}` : null; @@ -250,14 +263,14 @@ export function usePlaylistActions() { { id: "push", label: linked ? t("playlists.pushToYoutube") : t("playlists.exportToYoutube"), - icon: busy(pushing) ? ( + icon: pushing.has(pl.id) ? ( ) : linked ? ( ) : ( ), - disabled: busy(pushing), + disabled: busy, onSelect: () => void push(pl), }, ] @@ -267,9 +280,11 @@ export function usePlaylistActions() { { id: "revert", label: t("playlists.revert"), - icon: , + icon: ( + + ), danger: true, - disabled: busy(reverting), + disabled: busy, onSelect: () => void revert(pl), }, ] diff --git a/frontend/src/components/useVideoActions.dom.test.tsx b/frontend/src/components/useVideoActions.dom.test.tsx index e6a63d9b..269b2c07 100644 --- a/frontend/src/components/useVideoActions.dom.test.tsx +++ b/frontend/src/components/useVideoActions.dom.test.tsx @@ -44,13 +44,14 @@ function downloadRow(state: string | undefined) { } 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"); - 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"); cleanup(); const none = downloadRow(undefined); await waitFor(() => expect(none().label).toBe("downloads.menu.download")); - expect(none().title).toBeUndefined(); + expect(none().note).toBeUndefined(); }); }); diff --git a/frontend/src/components/useVideoActions.tsx b/frontend/src/components/useVideoActions.tsx index bcd1fb2f..2b23bb0a 100644 --- a/frontend/src/components/useVideoActions.tsx +++ b/frontend/src/components/useVideoActions.tsx @@ -74,8 +74,9 @@ export function useVideoActions(handlers: VideoActionHandlers, open: boolean) { const hidden = v.status === "hidden"; const inProgress = v.position_seconds > 0 && !watched; 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 - // label — "Downloaded" read as a status, not as a row that opens something (E28 review J4). + // 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) — and + // not a `title`, which a key or a finger never shows (round 2, F5). const dlState = dl === "done" ? t("downloads.button.downloaded") @@ -143,7 +144,7 @@ export function useVideoActions(handlers: VideoActionHandlers, open: boolean) { { id: "download", label: t("downloads.menu.download"), - title: dlState, + note: dlState, icon: , onSelect: () => setFollowUp({ kind: "download", video: v }), },