feat(design): a playlist's one action list (C1) — the row's "…", Delete + undo, the header's "…" (E28 S3e2)

- Playlist rows: the X column became the row's "…" (play, pin, copy title/link, move, order,
  remove). Remove runs at once, with no confirm; its toast's Undo puts the row back at its index.
  The Delete key removes a keyboard-walked row, and focus moves on to the next.
- POST /api/playlists/{id}/items takes an optional `position`, so an undo is one request that
  renumbers the list around the row.
- A new `listDelete` key scope. Only a list with `onDelete` registers it, so the help sheet does not
  offer Delete where nothing handles it.
- A playlist has one list, `usePlaylistActions.items`: rename, pin, YouTube open/push/revert,
  delete. The rail row's "…" and the page header's "…" both open it. Push and revert moved into
  the hook; the header keeps Play all.
- Phone-width playlist table: "#" and Channel are hidden below sm (`Column.phoneHidden`). Their
  fixed widths had left the title no width.
This commit is contained in:
2026-09-26 04:05:39 +02:00
parent aaf1843b09
commit 7455fff401
24 changed files with 1061 additions and 425 deletions
+26 -3
View File
@@ -4,7 +4,7 @@ mirror the user's own YouTube playlists in (`/sync-youtube`) and push/revert loc
out (`/{id}/push-plan`, `/{id}/revert-youtube`), behind the optional write scope."""
from fastapi import APIRouter, Depends, HTTPException
from pydantic import BaseModel
from pydantic import BaseModel, Field
from sqlalchemy import func, select, and_
from sqlalchemy.exc import IntegrityError
from sqlalchemy.orm import Session, aliased
@@ -507,17 +507,40 @@ def delete_playlist(
return {"deleted": playlist_id}
class AddItemIn(VideoRefIn):
# Where the video lands among the list's items, in their stored order (0 = first). Omitted, it
# is appended. The list page's "Undo" of a removal puts the row back where it was with this —
# ONE request, because append-then-reorder leaves it at the end whenever the second call fails.
position: int | None = Field(default=None, ge=0)
@router.post("/{playlist_id}/items")
def add_item(
playlist_id: int,
body: VideoRefIn,
body: AddItemIn,
user: User = Depends(current_user),
db: Session = Depends(get_db),
) -> dict:
pl = owned_or_404(db, Playlist, playlist_id, user, what="playlist")
video_id = body.video_id
get_or_404(db, Video, video_id, what="video")
_add_item(db, pl, video_id, mark_dirty=True)
added = _add_item(db, pl, video_id, mark_dirty=False)
if added and body.position is not None:
# Renumber the whole list with the new row at `position` (clamped to the end): positions
# can have gaps after removals, so "index N" is only meaningful over the ordered rows.
rows = list(
db.execute(
select(PlaylistItem)
.where(PlaylistItem.playlist_id == pl.id)
.order_by(PlaylistItem.position)
).scalars()
)
new = next(r for r in rows if r.video_id == video_id)
rows.remove(new)
rows.insert(min(body.position, len(rows)), new)
for pos, row in enumerate(rows):
row.position = pos
recompute_dirty(db, pl)
db.commit()
return {"ok": True, "playlist_id": pl.id, "video_id": video_id}
+105
View File
@@ -0,0 +1,105 @@
"""E28 S3e2 — adding a video AT a position: the list page's "Undo" of a removal.
The row has to come back where it was in ONE request. The two-call shape (append, then send the
whole order) leaves it at the end of the list whenever the second call fails, and the reorder would
also overwrite any change another tab made in between. Positions can have gaps after removals, so
the position is an index over the ORDERED rows, and the list is renumbered around it.
"""
from datetime import datetime, timezone
from app.models import Channel, Playlist, PlaylistItem, Video
def _seed(db, user, ids, *, positions=None, **kw):
db.add(Channel(id="UCtest", title="Test"))
now = datetime.now(timezone.utc)
for v in [*ids, "vX"]:
db.add(Video(id=v, channel_id="UCtest", published_at=now))
pl = Playlist(user_id=user.id, name="L", **kw)
db.add(pl)
db.flush()
for i, vid in enumerate(ids):
db.add(PlaylistItem(playlist_id=pl.id, video_id=vid, position=(positions or range(99))[i]))
db.commit()
return pl
def _ids(db, pl_id):
db.expire_all()
return [
r.video_id
for r in db.execute(
PlaylistItem.__table__.select()
.where(PlaylistItem.playlist_id == pl_id)
.order_by(PlaylistItem.position)
).all()
]
def _add(client, pl_id, body):
return client.post(f"/api/playlists/{pl_id}/items", json=body)
def test_a_position_puts_the_video_back_in_its_place(db, make_user, as_user):
user = make_user()
pl = _seed(db, user, ["a", "b", "c"])
r = _add(as_user(user), pl.id, {"video_id": "vX", "position": 1})
assert r.status_code == 200
assert _ids(db, pl.id) == ["a", "vX", "b", "c"]
def test_the_position_counts_rows_not_stored_numbers(db, make_user, as_user):
# A removal leaves a gap (0, 5, 9): index 2 is before the THIRD row, not "position 2".
user = make_user()
pl = _seed(db, user, ["a", "b", "c"], positions=[0, 5, 9])
_add(as_user(user), pl.id, {"video_id": "vX", "position": 2})
assert _ids(db, pl.id) == ["a", "b", "vX", "c"]
def test_a_position_past_the_end_appends(db, make_user, as_user):
user = make_user()
pl = _seed(db, user, ["a", "b"])
_add(as_user(user), pl.id, {"video_id": "vX", "position": 40})
assert _ids(db, pl.id) == ["a", "b", "vX"]
def test_no_position_still_appends(db, make_user, as_user):
user = make_user()
pl = _seed(db, user, ["a", "b"])
_add(as_user(user), pl.id, {"video_id": "vX"})
assert _ids(db, pl.id) == ["a", "b", "vX"]
def test_a_video_already_in_the_list_does_not_move(db, make_user, as_user):
# The add is idempotent; a position must not turn it into a reorder.
user = make_user()
pl = _seed(db, user, ["a", "b", "c"])
_add(as_user(user), pl.id, {"video_id": "c", "position": 0})
assert _ids(db, pl.id) == ["a", "b", "c"]
def test_a_negative_position_is_refused(db, make_user, as_user):
user = make_user()
pl = _seed(db, user, ["a"])
r = _add(as_user(user), pl.id, {"video_id": "vX", "position": -1})
assert r.status_code == 422
assert _ids(db, pl.id) == ["a"]
def test_putting_a_removed_video_back_leaves_a_linked_list_clean(db, make_user, as_user):
# Remove + undo is no change at all: a YouTube-linked list must not stay "unsynced" after it.
user = make_user()
pl = _seed(db, user, ["a", "b", "c"], yt_playlist_id="PLx")
client = as_user(user)
from app.sync.playlists import current_fingerprint
pl.synced_fingerprint = current_fingerprint(db, pl)
pl.dirty = False
db.commit()
client.delete(f"/api/playlists/{pl.id}/items/b")
db.expire_all()
assert db.get(Playlist, pl.id).dirty is True
_add(client, pl.id, {"video_id": "b", "position": 1})
db.expire_all()
assert db.get(Playlist, pl.id).dirty is False
+7 -7
View File
@@ -308,14 +308,14 @@ export default function PlayerModal({
if (q && i < q.length - 1) setPlayingId(q[i + 1]!.id);
};
// "Remove from this playlist", offered right where you are watching the thing you want gone. The
// parent owns the removal (confirm + API + caches); the player owns what happens NEXT, because it
// parent owns the removal (API + caches + the toast whose Undo puts it back); the player owns what happens NEXT, because it
// is the only side that knows which item is playing — its queue is frozen at mount, so the parent
// cannot tell. Step to the item that followed (clamped when the last one goes), and close when the
// list runs out.
//
// Guarded against a double press by a REF, not by disabling the button: a trigger disabled for
// the confirm's lifetime was still disabled when the dialog closed and restored focus to it — a
// no-op — so a keyboard user who cancelled was left on <body>, outside this aria-modal player.
// Guarded against a double press by a REF, not by disabling the button: a trigger disabled for the
// removal's lifetime was still disabled when focus came back to it (it once waited on a confirm) —
// a no-op — so a keyboard user was left on <body>, outside this aria-modal player.
const removingRef = useRef(false);
const removeFromList = async () => {
if (!onRemoveFromList || removingRef.current) return;
@@ -327,9 +327,9 @@ export default function PlayerModal({
} finally {
removingRef.current = false;
}
if (!done) return; // cancelled at the confirm, or the request failed — leave playback alone
if (!done) return; // the request failed — leave playback alone
// Decide from where the player is NOW, not where it was when the button was pressed: prev/next
// and auto-advance keep working under the confirm dialog, and a cursor from before the await
// and auto-advance keep working while the request runs, and a cursor from before the await
// threw the viewer back onto a video they had already moved past.
const { queue: q, index: i } = navRef.current;
if ((q?.[i] ?? video).id !== victim.id) {
@@ -967,7 +967,7 @@ export default function PlayerModal({
variant="outline-danger"
size="md"
onClick={() => void removeFromList()}
// Named with its object, like the row control: a bare "Remove" among Fullscreen /
// Named with its object, like the list's menu row: a bare "Remove" among Fullscreen /
// Download / Mark watched reads as "delete this video".
label={t("playlists.removeItemOf", { title: active.title })}
title={t("playlists.removeItem")}
+181 -2
View File
@@ -9,6 +9,7 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
const h = vi.hoisted(() => ({
notify: vi.fn(),
confirm: vi.fn(async () => true),
selectedId: 1 as number | null,
mounted: false,
mounts: 0,
@@ -35,8 +36,18 @@ vi.mock("../lib/notifications", async (importOriginal) => ({
notify: (...a: unknown[]) => h.notify(...a),
}));
vi.mock("./playlistsContext", () => ({ usePlaylists: () => ({ selectedId: h.selectedId }) }));
vi.mock("./confirmContext", () => ({ useConfirm: () => async () => true }));
vi.mock("./usePlaylistActions", () => ({ usePlaylistActions: () => ({ remove: vi.fn() }) }));
vi.mock("./confirmContext", () => ({ useConfirm: () => h.confirm }));
// The list's own menu is usePlaylistActions' (pinned in its own test); here only its rename row,
// which opens THIS page's title field.
vi.mock("./usePlaylistActions", () => ({
usePlaylistActions: () => ({
remove: vi.fn(),
rename: vi.fn(),
items: (_pl: unknown, o: { onRename: () => void }) => [
{ id: "rename", label: "playlists.rename", onSelect: o.onRename },
],
}),
}));
vi.mock("./PlayerModal", async () => {
const { useEffect } = await import("react");
return {
@@ -131,6 +142,7 @@ beforeEach(() => {
h.mounted = false;
h.mounts = 0;
h.notify.mockClear();
h.confirm.mockClear();
});
afterEach(() => {
cleanup();
@@ -393,6 +405,173 @@ describe("Playlists — a change the server refused is taken back on screen", ()
});
});
// E28 C1: a row's X column went. Remove is in the row's menu and on the Delete key, with no confirm —
// the toast's Undo puts the row back where it stood, in ONE request.
describe("Playlists — removing a row, and undoing it", () => {
// The server's side of a removal, so the refetch after it agrees with it.
const serverRemoves = (lists: Record<number, Video[]>) =>
vi.spyOn(api, "removeFromPlaylist").mockImplementation(async (id: number, videoId: string) => {
lists[id] = lists[id]!.filter((v) => v.id !== videoId);
return undefined as never;
});
// The removal's toast — found by what it says, not by being the latest: a refetch after the
// removal may notify too.
const lastToast = () =>
h.notify.mock.calls
.map((c) => c[0] as { message: string; action?: { label: string; onClick: () => void } })
.filter((n) => n.message?.startsWith("playlists.removedToast"))
.pop()!;
const pressKey = (key: string) =>
act(() => {
(document.activeElement ?? document.body).dispatchEvent(
new KeyboardEvent("keydown", { key, bubbles: true, cancelable: true }),
);
});
it("removes from the menu at once, and the toast's Undo puts it back at its place", async () => {
const lists = { 1: [vid("A", 0), vid("B", 0), vid("C", 0)] };
serverRemoves(lists);
vi.spyOn(api, "addToPlaylist").mockResolvedValue(undefined as never);
mount(lists);
await rowMenu("Video B", "playlists.removeItem");
await waitFor(() => expect(api.removeFromPlaylist).toHaveBeenCalledWith(1, "B"));
expect(rowsOnScreen()).toEqual(["Video A", "Video C"]);
// No confirm: the toast's Undo is the safety net.
expect(h.confirm).not.toHaveBeenCalled();
await waitFor(() => expect(lastToast().message).toBe("playlists.removedToast:Video B"));
expect(lastToast().action?.label).toBe("common.undo");
act(() => lastToast().action!.onClick());
// Index 1 of the STORED order: where it was, not the end of the list.
await waitFor(() => expect(api.addToPlaylist).toHaveBeenCalledWith(1, "B", 1));
});
it("puts it back where it stood in the stored order, whatever the view on screen", async () => {
vi.spyOn(api, "removeFromPlaylist").mockResolvedValue(undefined as never);
vi.spyOn(api, "addToPlaylist").mockResolvedValue(undefined as never);
mount({ 1: [vid("A", 0), vid("B", 0), vid("C", 0)] });
await screen.findByRole("button", { name: "card.play — Video C" });
// Sort the VIEW by title, descending: C, B, A on screen — the stored order is still A, B, C.
fireEvent.click(screen.getByRole("button", { name: /playlists\.colTitle/ }));
fireEvent.click(screen.getByRole("button", { name: /playlists\.colTitle/ }));
await waitFor(() => expect(rowsOnScreen()).toEqual(["Video C", "Video B", "Video A"]));
await rowMenu("Video A", "playlists.removeItem");
await waitFor(() => expect(lastToast()?.action).toBeTruthy());
act(() => lastToast().action!.onClick());
await waitFor(() => expect(api.addToPlaylist).toHaveBeenCalledWith(1, "A", 0));
});
it("says so when the Undo fails", async () => {
vi.spyOn(api, "removeFromPlaylist").mockResolvedValue(undefined as never);
vi.spyOn(api, "addToPlaylist").mockRejectedValue(new TypeError("Failed to fetch"));
mount({ 1: [vid("A", 0), vid("B", 0)] });
await rowMenu("Video A", "playlists.removeItem");
await waitFor(() => expect(lastToast()?.action).toBeTruthy());
act(() => lastToast().action!.onClick());
await waitFor(() =>
expect(h.notify).toHaveBeenCalledWith(
expect.objectContaining({ level: "error", title: "playlists.undoRemoveFailed" }),
),
);
});
it("removes the walked row on Delete, and the focus goes on to the next", async () => {
const lists = { 1: [vid("A", 0), vid("B", 0), vid("C", 0)] };
serverRemoves(lists);
mount(lists);
await screen.findByRole("button", { name: "card.play — Video C" });
pressKey("j");
pressKey("j");
expect(document.activeElement?.textContent).toContain("Video B");
pressKey("Delete");
await waitFor(() => expect(api.removeFromPlaylist).toHaveBeenCalledWith(1, "B"));
expect(rowsOnScreen()).toEqual(["Video A", "Video C"]);
await waitFor(() => expect(document.activeElement?.textContent).toContain("Video C"));
});
it("offers no Delete, no Remove and no X on Watch later's rows", async () => {
vi.spyOn(api, "removeFromPlaylist").mockResolvedValue(undefined as never);
h.selectedId = WATCH_LATER;
mount({ [WATCH_LATER]: [vid("A", 0), vid("B", 0)] });
await screen.findByRole("button", { name: "card.play — Video B" });
pressKey("j");
pressKey("Delete");
await act(async () => void (await new Promise((r) => setTimeout(r, 20))));
expect(api.removeFromPlaylist).not.toHaveBeenCalled();
expect(screen.queryByRole("button", { name: /removeItemOf/ })).toBeNull();
});
it("opens the row's menu from its '…', with the copy rows", async () => {
mount({ 1: [vid("A", 0)] });
await screen.findByRole("button", { name: "card.play — Video A" });
const more = screen.getByRole("button", { name: "playlists.rowMenu" });
expect(more.getAttribute("aria-expanded")).toBe("false");
fireEvent.click(more);
await waitFor(() =>
expect(screen.getAllByRole("menuitem", { hidden: true }).map((b) => b.textContent)).toEqual(
expect.arrayContaining([
"card.play",
"common.copy.title",
"common.copy.link",
"playlists.removeItem",
]),
),
);
expect(
screen.getByRole("button", { name: "playlists.rowMenu" }).getAttribute("aria-expanded"),
).toBe("true");
});
});
// E28 C1: the header keeps Play all; rename, YouTube and delete are the list's menu, behind a "…".
describe("Playlists · the header's '…'", () => {
it("replaces the pencil, the YouTube buttons and the trash, and renames from its menu", async () => {
mount({ 1: [vid("A", 0)] });
await screen.findByRole("button", { name: "card.play — Video A" });
for (const gone of [
"playlists.rename",
"playlists.delete",
"playlists.pushToYoutube",
"playlists.exportToYoutube",
"playlists.openOnYoutube",
])
expect(screen.queryByRole("button", { name: gone })).toBeNull();
const more = screen.getByRole("button", { name: "playlists.railMenu" });
act(() => more.focus());
fireEvent.click(more);
// The surface is visibility:hidden in jsdom (floating-ui never positions): matched on text.
const renameRow = await waitFor(() => {
const row = screen
.getAllByRole("menuitem", { hidden: true })
.find((b) => b.textContent === "playlists.rename");
expect(row).toBeTruthy();
return row!;
});
fireEvent.click(renameRow);
const field = await screen.findByRole("textbox", { name: "playlists.rename" });
// Leaving the field by Escape hands the focus back to the "…" it was opened from.
fireEvent.keyDown(field, { key: "Escape" });
await waitFor(() =>
expect(document.activeElement).toBe(
screen.getByRole("button", { name: "playlists.railMenu" }),
),
);
});
});
describe("Playlists · a phone-width table", () => {
it("leaves out '#' and Channel below sm, header and cells alike, so the title keeps a width", async () => {
mount({ 1: [vid("A", 0)] });
await screen.findByRole("button", { name: "card.play — Video A" });
const phoneHidden = (el: Element) =>
el.classList.contains("hidden") && el.classList.contains("sm:table-cell");
const heads = [...document.querySelectorAll("thead th")];
const cells = [...document.querySelectorAll("tbody tr:first-child td")];
// Columns: grip, #, title, channel, length, "…".
expect(heads.map(phoneHidden)).toEqual([false, true, false, true, false, false]);
expect(cells.map(phoneHidden)).toEqual([false, true, false, true, false, false]);
});
});
describe("Playlists — the rows are a keyboard list", () => {
it("walks the rows with j and plays the focused one on Enter", async () => {
mount({ 1: [vid("A", 0), vid("B", 0)] });
+100 -211
View File
@@ -24,15 +24,10 @@ import {
ArrowUp,
ArrowUpToLine,
Check,
ExternalLink,
FolderInput,
History,
ListPlus,
Pencil,
Play,
RotateCcw,
Trash2,
Upload,
X,
Youtube,
} from "lucide-react";
@@ -47,19 +42,16 @@ import { formatRuntime } from "../lib/format";
import { applyView, sameOrder } from "../lib/playlistView";
const PlayerModal = lazy(() => import("./PlayerModal"));
import { Button } from "./ui/Button";
import { IconButton } from "./ui/IconButton";
import { LoadingState, StateMessage } from "./ui/QueryState";
import { type ActionMenuItem } from "./ui/ActionMenu";
import { useComfortPin, pinVideo } from "./comfortPin";
import { useComfortPin, pinPlaylist, pinVideo } from "./comfortPin";
import { ContextMenu } from "./ui/ContextMenu";
import { useContextMenu } from "./ui/useContextMenu";
import UndoToolbar from "./ui/UndoToolbar";
import NavGrid from "./ui/NavGrid";
import { PageToolbar } from "./ui/PageShell";
import { useConfirm } from "./confirmContext";
import { usePlaylistActions } from "./usePlaylistActions";
import { usePlaylists } from "./playlistsContext";
import { useMe } from "../lib/useMe";
import { usePref } from "../lib/usePref";
import { navItemAttrs } from "../lib/listNav";
import { ColumnHead } from "./ui/tableHeader";
@@ -71,7 +63,8 @@ import { DragHandle } from "./ui/DragHandle";
import { Input } from "./ui/Input";
import { CheckRow } from "./ui/NativeControls";
import { useReturnFocus } from "./ui/useReturnFocus";
import { IconLink } from "./ui/LinkButton";
import { MoreButton } from "./ui/MoreButton";
import { useCopyItems } from "./copyItems";
// The playlist detail is a HYBRID table (E4 S4): real sortable/filterable column headers over a
// body that is still a drag-reorderable @dnd-kit list.
@@ -134,7 +127,8 @@ function Row({
}}
// transition-COLORS, not `transition` — the bare utility animates transform too, which is
// the same lag by another route (150ms behind the pointer, on every row).
className="border-b border-border/50 state-layer transition-colors"
// `group`: the row's "…" reveals on its hover or focus (`.reveal`), and stays on touch.
className="group border-b border-border/50 state-layer transition-colors"
{...navItemAttrs(index)}
{...menuProps}
>
@@ -142,7 +136,7 @@ function Row({
<td
key={col.key}
style={col.width ? { width: col.width } : undefined}
className={`px-2 py-1.5 align-middle ${col.nowrap ? "whitespace-nowrap" : ""} ${alignClass(col.align)}`}
className={`px-2 py-1.5 align-middle ${col.nowrap ? "whitespace-nowrap" : ""} ${alignClass(col.align)}${col.phoneHidden ? " hidden sm:table-cell" : ""}`}
>
{col.key === GRIP_KEY ? (
// The one cell the column defs can't render: the handle needs this row's sortable
@@ -164,14 +158,11 @@ function Row({
}
export default function Playlists() {
const me = useMe();
const canWrite = me.can_write;
// The selected playlist lives in PlaylistsProvider, shared with the PlaylistsRail (which owns the
// list + its rail controls). This detail pane reacts to the selection.
const { selectedId } = usePlaylists();
const { t } = useTranslation();
const qc = useQueryClient();
const confirm = useConfirm();
const plActions = usePlaylistActions();
const [renaming, setRenaming] = useState(false);
const renameFocus = useReturnFocus(renaming);
@@ -267,6 +258,10 @@ export default function Playlists() {
// The comfort-shelf pin composes into the row's existing menu (E24 S3) rather than mounting a
// second one on the same <tr>.
const comfortPin = useComfortPin(rowMenu.open);
const copy = useCopyItems();
// The header's "…": the list's own menu, the one its rail row opens (usePlaylistActions.items).
const headerMenu = useContextMenu<PlaylistDetail>();
const headerPin = useComfortPin(headerMenu.open);
const listQuery = useQuery({
queryKey: qk.playlists(),
@@ -323,17 +318,15 @@ export default function Playlists() {
const builtin = detail?.kind === "watch_later";
// YouTube-mirrored playlists can be edited locally and synced back (the read-sync skips a
// dirty mirror so it won't clobber unpushed edits); the YT-link button marks their origin.
// dirty mirror so it won't clobber unpushed edits); the header's "Unsynced" says when they differ.
const editable = !builtin;
const linked = editable && !!detail?.yt_playlist_id;
// LOCAL editing (reorder, remove) needs no YouTube write scope — `canWrite` gates only the
// push/delete-on-YouTube paths. Folding it in here would have taken drag and remove away from
// LOCAL editing (reorder, remove) needs no YouTube write scope — `can_write` gates only the
// push/delete-on-YouTube paths (usePlaylistActions). Folding it in here would have taken drag and remove away from
// every account without the scope, which they have always had.
const canEdit = editable;
const canEditRef = useRef(canEdit);
canEditRef.current = canEdit;
const [pushing, setPushing] = useState(false);
const [reverting, setReverting] = useState(false);
// Stable (the memoized row callbacks depend on it), so it reads `shown` through the ref rather
// than closing over it.
@@ -350,11 +343,8 @@ export default function Playlists() {
});
}, []);
const onPlay = useCallback((v: Video) => openPlayer(v.id), [openPlayer]);
const onRemove = useCallback((v: Video) => {
void removeRef.current(v);
}, []);
// The player's "remove from this playlist" goes through the SAME removeItem the row menu and the
// inline ✕ use — one confirm, one optimistic update, one undo-stack rebase, no second copy to
// Delete key use — one optimistic update, one undo toast, one undo-stack rebase, no second copy to
// drift. It answers whether the removal actually happened, because the player has to decide
// afterwards whether to step to the next item or close. It removes from the list the player was
// launched from (see `playing.listId`), not from whatever is selected by now.
@@ -376,9 +366,25 @@ export default function Playlists() {
[items],
);
// The row's "…" opens the row menu under itself (E28 C1). Which row's menu is open is part of the
// cell (aria-expanded), so the columns rebuild when it changes — a cheap array of closures.
const { buttonProps: rowMenuButton, open: rowMenuOpen, target: rowMenuTarget } = rowMenu;
const columns = useMemo(
() => playlistColumns({ t, indexOf: storedIndex, onPlay, onRemove, canEdit }),
[t, storedIndex, onPlay, onRemove, canEdit],
() =>
playlistColumns({
t,
indexOf: storedIndex,
onPlay,
more: (v) => (
<MoreButton
label={t("playlists.rowMenu")}
expanded={rowMenuOpen && rowMenuTarget === v}
menuProps={rowMenuButton(v)}
className={rowMenuOpen && rowMenuTarget === v ? undefined : "reveal"}
/>
),
}),
[t, storedIndex, onPlay, rowMenuButton, rowMenuOpen, rowMenuTarget],
);
const filterActive = Object.values(filters).some(isActive);
@@ -437,108 +443,6 @@ export default function Playlists() {
qc.invalidateQueries({ queryKey: qk.playlist(listId) });
}
async function revertToYoutube() {
if (selectedId == null || !detail || reverting) return;
const ok = await confirm({
title: t("playlists.revertTitle"),
message: t("playlists.revertMsg"),
confirmLabel: t("playlists.revertConfirm"),
danger: true,
});
if (!ok) return;
setReverting(true);
try {
await api.revertPlaylist(selectedId);
refreshAll();
notify({ level: "success", message: t("playlists.revertDone", { name: plName(detail) }) });
} catch (e) {
notifyYouTubeActionError(e, t("playlists.revertFailed"));
} finally {
setReverting(false);
}
}
async function pushToYoutube() {
if (selectedId == null || !detail || pushing) return;
setPushing(true);
try {
const plan = await api.playlistPushPlan(selectedId);
const nothingToDo =
plan.action === "update" &&
!plan.add_yt &&
!plan.add_local &&
!plan.remove_yt &&
!plan.remove_local &&
!plan.to_reorder &&
!plan.rename;
if (nothingToDo) {
notify({ level: "info", message: t("playlists.pushUpToDate", { name: plName(detail) }) });
return;
}
if (!plan.affordable) {
notify({
level: "warning",
message: t("playlists.pushNoQuota", {
left: plan.remaining_today,
units: plan.units_estimate,
}),
});
return;
}
let message: string;
if (plan.action === "create") {
message = t("playlists.pushPlanCreate", {
count: plan.add_yt,
units: plan.units_estimate,
left: plan.remaining_today,
});
} else {
// One sentence per side that actually changes; the untouched side gets a short "stays
// unchanged" reassurance instead of a "+0 / −0" the reader has to decode.
const here: string[] = [];
if (plan.add_local) here.push(t("playlists.mergeAddLocal", { count: plan.add_local }));
if (plan.remove_local)
here.push(t("playlists.mergeRemoveLocal", { count: plan.remove_local }));
const yt: string[] = [];
if (plan.add_yt) yt.push(t("playlists.mergeAddYt", { count: plan.add_yt }));
if (plan.remove_yt) yt.push(t("playlists.mergeRemoveYt", { count: plan.remove_yt }));
if (plan.to_reorder) yt.push(t("playlists.mergeReorder", { count: plan.to_reorder }));
if (plan.rename) yt.push(t("playlists.mergeRename"));
const sentences = [...here, ...yt];
if (here.length && !yt.length) sentences.push(t("playlists.mergeYtUnchanged"));
if (yt.length && !here.length) sentences.push(t("playlists.mergeLocalUnchanged"));
const quota =
plan.units_estimate > 0
? t("playlists.mergeQuota", {
units: plan.units_estimate,
left: plan.remaining_today,
})
: t("playlists.mergeQuotaFree");
message = sentences.join(" ") + quota;
}
const ok = await confirm({
title: t("playlists.pushTitle"),
message,
confirmLabel: t("playlists.pushConfirm"),
});
if (!ok) return;
const r = await api.pushPlaylist(selectedId);
refreshAll();
if (r.failures.length) {
notify({
level: "warning",
message: t("playlists.pushPartial", { count: r.failures.length }),
});
} else {
notify({ level: "success", message: t("playlists.pushDone", { name: plName(detail) }) });
}
} catch (e) {
notifyYouTubeActionError(e, t("playlists.pushFailed"));
} finally {
setPushing(false);
}
}
function onDragEnd(e: DragEndEvent) {
const { active: a, over } = e;
if (!over || a.id === over.id || selectedId == null) return;
@@ -548,26 +452,20 @@ export default function Playlists() {
order.set(arrayMove(items, oldIndex, newIndex)); // onApply persists
}
/** @returns whether the item was actually removed — the player needs to know, because it must
/** Takes a row out at once and says so in a toast whose Undo puts it back where it was (E28 C1:
* the X column became the Delete key + this toast; no confirm, because nothing is lost).
* @returns whether the item was actually removed — the player needs to know, because it must
* step to the next item (or close) only when the removal really happened. */
async function removeItem(
video: Video,
listId: number | null = selectedRef.current,
): Promise<boolean> {
if (listId == null) return false;
const ok = await confirm({
title: t("playlists.removeItem"),
message: t("playlists.confirmRemove", { title: video.title }),
confirmLabel: t("playlists.removeItem"),
danger: true,
});
if (!ok) return false;
// Only the list ON SCREEN has rows to take the item out of optimistically — the open player can
// be removing from the list it was launched from while another one is selected. Read after the
// confirm: the selection may have changed while it was open.
// be removing from the list it was launched from while another one is selected.
if (listId === selectedRef.current) {
// A removal isn't itself undoable, but it no longer THROWS AWAY the reorder history: rebasing
// drops the item from every snapshot, so the stack stays valid for what's left.
// A removal isn't itself on the reorder undo stack, but it no longer THROWS AWAY that history:
// rebasing drops the item from every snapshot, so the stack stays valid for what's left.
const next = order.value.filter((v) => v.id !== video.id);
order.rebase((list) => list.filter((v) => v.id !== video.id));
lastSetRef.current = `${listId}:${idsKey(next)}`;
@@ -578,17 +476,19 @@ export default function Playlists() {
// so the guard reset the order: the removed row came back and the undo history the rebase had
// kept went with it. (A failure resyncs below, which puts the row back where it belongs.)
const before = qc.getQueryData<PlaylistDetail>(qk.playlist(listId));
// Where it stood in the STORED order — what the Undo puts it back at (the server's list, so a
// view sort on screen does not decide it).
const at = before?.items.findIndex((v) => v.id === video.id) ?? -1;
qc.setQueryData<PlaylistDetail>(qk.playlist(listId), (old) =>
old ? { ...old, items: old.items.filter((v) => v.id !== video.id) } : old,
);
try {
await api.removeFromPlaylist(listId, video.id);
refreshAll(listId);
return true;
} catch (e) {
// Said here, as moveItemTo does, or a confirmed removal fails in silence (inside the
// full-screen player there is not even a row snapping back to show it) — unless `req` already
// put it in its error dialog, which `notifyActionError` checks.
// Said here, as moveItemTo does, or a removal fails in silence (inside the full-screen player
// there is not even a row snapping back to show it) — unless `req` already put it in its error
// dialog, which `notifyActionError` checks.
notifyActionError(e, t("playlists.removeFailed"));
// …and the row goes back NOW, not when the resync lands: a removal that failed because the
// server was unreachable has a resync that fails too, and the list kept showing the row as
@@ -597,6 +497,24 @@ export default function Playlists() {
refreshAll(listId);
return false;
}
notify({
level: "success",
message: t("playlists.removedToast", { title: video.title }),
action: {
label: t("common.undo"),
// ONE request that puts it back at its index (the server renumbers around it): an append
// plus a whole-list reorder would land it at the end whenever the second call failed.
onClick: () =>
void api
.addToPlaylist(listId, video.id, at < 0 ? undefined : at)
.catch((e: unknown) => notifyActionError(e, t("playlists.undoRemoveFailed")))
.finally(() => {
refreshAll(listId);
qc.invalidateQueries({ queryKey: qk.playlistMembership(video.id) });
}),
},
});
return true;
}
// Kept in a ref so the memoized column callbacks don't rebuild on every state change.
const removeRef = useRef(removeItem);
@@ -659,13 +577,6 @@ export default function Playlists() {
if (name && name !== detail?.name) await plActions.rename(selectedId, name);
}
async function deletePlaylist() {
if (!detail) return;
// The dialog, the "delete on YouTube too?" branch and the re-selection live in
// usePlaylistActions — the rail's context menu deletes through the SAME code.
await plActions.remove(detail);
}
function playerState(id: string, status: string) {
api
.setState(id, status)
@@ -729,6 +640,10 @@ export default function Playlists() {
onSelect: () => openPlayer(ctxRow.id),
},
...comfortPin.item(pinVideo(ctxRow.id), ctxRow.title ?? ""),
...copy(ctxRow.title, ctxRow.watch_url).map((it, i) => ({
...it,
separatorBefore: i === 0,
})),
...(canEdit
? ([
...(recentMoveTarget
@@ -802,6 +717,17 @@ export default function Playlists() {
: []),
];
const headerPl = headerMenu.target;
const headerItems: ActionMenuItem[] = !headerPl
? []
: plActions.items(headerPl, {
onRename: () => {
setRenameValue(headerPl.name);
setRenaming(true);
},
pin: headerPin.item(pinPlaylist(headerPl.id), plName(headerPl)),
});
// The open player, at ONE place in the tree whichever state the page below it is in. Selecting
// another list under an open player (the command palette works over it) puts the page into
// "Loading…" until that list arrives; with the player rendered only in the loaded branch, that
@@ -894,32 +820,7 @@ export default function Playlists() {
aria-label={t("playlists.rename")}
/>
) : (
<>
<h2 className="text-lg font-semibold truncate">{plName(detail)}</h2>
{editable && (
<IconButton
ref={renameFocus.target}
onClick={() => {
setRenameValue(detail.name);
setRenaming(true);
}}
label={t("playlists.rename")}
>
<Pencil />
</IconButton>
)}
{detail.yt_playlist_id && (
<IconLink
href={`https://www.youtube.com/playlist?list=${detail.yt_playlist_id}`}
target="_blank"
rel="noreferrer noopener"
label={t("playlists.openOnYoutube")}
className="shrink-0"
>
<ExternalLink />
</IconLink>
)}
</>
<h2 className="text-lg font-semibold truncate">{plName(detail)}</h2>
)}
<div className="ml-auto flex gap-1.5 items-center shrink-0">
{linked && detail.dirty && (
@@ -939,39 +840,15 @@ export default function Playlists() {
>
{t("playlists.playAll")}
</Button>
{editable && canWrite && (
<IconButton
variant={detail.dirty ? "outline-accent" : "outline"}
onClick={pushToYoutube}
loading={pushing}
label={
linked ? t("playlists.pushToYoutube") : t("playlists.exportToYoutube")
}
>
{linked ? <Youtube /> : <Upload />}
</IconButton>
)}
{linked && detail.dirty && (
<IconButton
label={t("playlists.revert")}
size="sm"
variant="outline"
onClick={revertToYoutube}
disabled={reverting}
>
<RotateCcw className={`w-3.5 h-3.5 ${reverting ? "animate-spin" : ""}`} />
</IconButton>
)}
{editable && (
<IconButton
label={t("playlists.delete")}
size="sm"
variant="outline-danger"
onClick={deletePlaylist}
>
<Trash2 className="w-3.5 h-3.5" />
</IconButton>
)}
{/* Rename, the pin, YouTube (open / push / revert) and Delete: the list's ONE
menu (E28 C1), the same the rail row opens — the header keeps Play all. */}
<MoreButton
ref={renameFocus.target}
variant="outline"
label={t("playlists.railMenu")}
expanded={headerMenu.open}
menuProps={headerMenu.buttonProps(detail)}
/>
</div>
</div>
@@ -1074,7 +951,18 @@ export default function Playlists() {
{/* The rows are a keyboard list: `j`/`k` and the arrows walk them, Enter plays. The grip
keeps Space/Enter, and during a keyboard drag the arrows too (listNav leaves a key
dnd-kit has handled). */}
<NavGrid as="tbody">
<NavGrid
as="tbody"
// Delete on a walked row = the menu's Remove (E28 C1), with the same undo toast.
onDelete={
canEdit
? (i) => {
const v = shownRef.current[i];
if (v) void removeItem(v);
}
: undefined
}
>
{shown.map((v, i) => (
<Row
key={v.id}
@@ -1100,6 +988,7 @@ export default function Playlists() {
{/* One surface for the whole table, outside the rows — it anchors to the pointer, not the row. */}
<ContextMenu menu={rowMenu} ariaLabel={t("playlists.rowMenu")} items={ctxItems} />
<ContextMenu menu={headerMenu} ariaLabel={t("playlists.railMenu")} items={headerItems} />
</div>
{player}
</>
@@ -23,10 +23,13 @@ vi.mock("./comfortPin", () => ({
useComfortPin: () => ({ item: () => [] }),
pinPlaylist: (id: number) => id,
}));
const rename = vi.fn(async () => {});
vi.mock("./usePlaylistActions", () => ({
usePlaylistActions: () => ({ rename, remove: vi.fn() }),
// The real usePlaylistActions builds the menu (the rename row is its first); only the dialogs are
// stubbed, and the write is watched at the API.
vi.mock("./confirmContext", () => ({
useChoice: () => async () => null,
useConfirm: () => async () => false,
}));
const rename = vi.fn(async () => ({}) as never);
// The SidePanel/PanelGroups chrome is not what is under test: render the rail's bodies directly.
vi.mock("./FilterRail", () => ({
default: ({ items }: { items: { id: string; body: ReactNode }[] }) => (
@@ -74,6 +77,7 @@ const alpha = {
async function openRename() {
vi.spyOn(api, "playlists").mockResolvedValue([alpha]);
vi.spyOn(api, "renamePlaylist").mockImplementation(rename);
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
render(
<QueryClientProvider client={qc}>
@@ -117,3 +121,40 @@ describe("PlaylistsRail · rename from the keyboard", () => {
await vi.waitFor(() => expect(document.activeElement).toBe(rowButton()));
});
});
// E28 C1: the row's "…" opens the same menu as its right-click — the list's one action list.
describe("PlaylistsRail · the row's '…'", () => {
it("opens the playlist's menu under the row, and says so", async () => {
vi.spyOn(api, "playlists").mockResolvedValue([alpha]);
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
render(
<QueryClientProvider client={qc}>
<FloatingTree>
<PlaylistsRail
layout={{} as never}
setLayout={() => {}}
collapsed={false}
onToggleCollapse={() => {}}
/>
</FloatingTree>
</QueryClientProvider>,
);
await screen.findByText("alpha");
const more = screen.getByRole("button", { name: "playlists.railMenu" });
expect(more.getAttribute("aria-expanded")).toBe("false");
fireEvent.click(more);
await vi.waitFor(() =>
expect(screen.getAllByRole("menuitem", { hidden: true }).map((b) => b.textContent)).toEqual([
"playlists.rename",
"playlists.exportToYoutube",
"playlists.delete",
]),
);
expect(
screen.getByRole("button", { name: "playlists.railMenu" }).getAttribute("aria-expanded"),
).toBe("true");
// It is not inside the row's own button: a button cannot hold a button.
expect(more.closest("button")).toBe(more);
expect(rowButton()?.contains(more)).toBe(false);
});
});
+59 -69
View File
@@ -2,7 +2,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react";
import { useTranslation } from "react-i18next";
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
import { qk } from "../lib/queryKeys";
import { ListVideo, Pencil, Pin, Plus, RefreshCw, Trash2, Youtube } from "lucide-react";
import { ListVideo, Pin, Plus, RefreshCw, Youtube } from "lucide-react";
import { api, type Playlist } from "../lib/api";
import { formatRuntime } from "../lib/format";
import { notify } from "../lib/notifications";
@@ -26,6 +26,7 @@ import PlaylistLimitsHint from "./PlaylistLimitsHint";
import { IconButton } from "./ui/IconButton";
import { Pressable } from "./ui/Pressable";
import { Input } from "./ui/Input";
import { MoreButton } from "./ui/MoreButton";
type PlSort = {
key: "custom" | "name" | "count" | "duration" | "recent";
@@ -199,43 +200,17 @@ export default function PlaylistsRail({
}
const ctxPl = railMenu.target;
// The built-in Watch later list is renameable/deletable nowhere else in the UI either; the rows
// stay visible and disabled so the menu doesn't change shape from list to list.
const ctxBuiltin = ctxPl?.kind === "watch_later";
// Interpolated, not spelled out: the HU copy said "Watch later", a name the Hungarian UI never
// shows for this list ("Megnézem később") — so the message named a list the reader had never seen
// (E10 review round 1). Passing the localized name makes the two impossible to drift.
const lockedNote = ctxBuiltin
? t("playlists.builtinLocked", { name: t("playlists.watchLater") })
: undefined;
// The list's ONE menu (usePlaylistActions.items), the same the page header's "…" opens; the rail
// renames IN the row that was right-clicked — the action lands where the gesture did.
const railItems: ActionMenuItem[] = !ctxPl
? []
: [
{
id: "rename",
label: t("playlists.rename"),
icon: <Pencil className="w-4 h-4" />,
disabled: ctxBuiltin,
note: lockedNote,
onSelect: () => {
setRenameValue(ctxPl.name);
setRenamingId(ctxPl.id);
},
: plActions.items(ctxPl, {
onRename: () => {
setRenameValue(ctxPl.name);
setRenamingId(ctxPl.id);
},
...comfortPin.item(pinPlaylist(ctxPl.id), plName(ctxPl)),
{
id: "delete",
label: t("playlists.delete"),
icon: <Trash2 className="w-4 h-4" />,
separatorBefore: true,
danger: true,
disabled: ctxBuiltin,
note: lockedNote,
// The same dialog (and the same "what gets selected next" rule) as the detail header's
// trash button — one implementation, two entry points.
onSelect: () => void plActions.remove(ctxPl),
},
];
pin: comfortPin.item(pinPlaylist(ctxPl.id), plName(ctxPl)),
});
const items: PanelItem[] = [
{
@@ -350,44 +325,59 @@ export default function PlaylistsRail({
/>
</div>
) : (
<Pressable
key={pl.id}
ref={(el) => {
if (pl.id === selectedId) selectedRef.current = el;
// Back from the rename field by Enter/Escape — see refocusRowRef.
if (el && refocusRowRef.current === pl.id) {
refocusRowRef.current = null;
el.focus();
}
}}
onClick={() => setSelectedId(pl.id)}
{...railMenu.triggerProps(pl)}
layer={pl.id !== selectedId}
className={`${ROW_BASE} ${
pl.id === selectedId ? "bg-accent/15 border-accent" : "border-transparent"
}`}
>
<div className="w-8.5 h-8.5 rounded-md bg-card overflow-hidden shrink-0">
{pl.cover_thumbnail && (
<img src={pl.cover_thumbnail} alt="" className="w-full h-full object-cover" />
)}
</div>
<div className="min-w-0">
<div className="flex items-center gap-1 text-[0.8125rem] text-fg">
<span className="truncate">{plName(pl)}</span>
{(pl.source === "youtube" || pl.yt_playlist_id) && (
<Youtube
className={`w-3 h-3 shrink-0 ${pl.dirty ? "text-accent" : "text-muted"}`}
// `group`: the row's "…" shows on its hover or focus (`.reveal`), and always on touch.
// A sibling of the row button, not inside it — a button cannot hold a button.
<div key={pl.id} className="group relative" {...railMenu.triggerProps(pl)}>
<Pressable
ref={(el) => {
if (pl.id === selectedId) selectedRef.current = el;
// Back from the rename field by Enter/Escape — see refocusRowRef.
if (el && refocusRowRef.current === pl.id) {
refocusRowRef.current = null;
el.focus();
}
}}
onClick={() => setSelectedId(pl.id)}
layer={pl.id !== selectedId}
// pr-9: the "…" sits over the row's right end, so the name truncates short of it.
className={`${ROW_BASE} pr-9 ${
pl.id === selectedId ? "bg-accent/15 border-accent" : "border-transparent"
}`}
>
<div className="w-8.5 h-8.5 rounded-md bg-card overflow-hidden shrink-0">
{pl.cover_thumbnail && (
<img
src={pl.cover_thumbnail}
alt=""
className="w-full h-full object-cover"
/>
)}
</div>
<div className="text-2xs text-muted">
{t("playlists.itemCount", { count: pl.item_count })}
{pl.total_duration_seconds > 0 &&
` · ${formatRuntime(pl.total_duration_seconds)}`}
<div className="min-w-0">
<div className="flex items-center gap-1 text-[0.8125rem] text-fg">
<span className="truncate">{plName(pl)}</span>
{(pl.source === "youtube" || pl.yt_playlist_id) && (
<Youtube
className={`w-3 h-3 shrink-0 ${pl.dirty ? "text-accent" : "text-muted"}`}
/>
)}
</div>
<div className="text-2xs text-muted">
{t("playlists.itemCount", { count: pl.item_count })}
{pl.total_duration_seconds > 0 &&
` · ${formatRuntime(pl.total_duration_seconds)}`}
</div>
</div>
</div>
</Pressable>
</Pressable>
<MoreButton
label={t("playlists.railMenu")}
expanded={railMenu.open && railMenu.target === pl}
menuProps={railMenu.buttonProps(pl)}
className={`absolute right-1.5 top-1/2 -translate-y-1/2 ${
railMenu.open && railMenu.target === pl ? "" : "reveal"
}`}
/>
</div>
),
)}
</div>
+23 -27
View File
@@ -1,9 +1,9 @@
import { Play, X } from "lucide-react";
import type { ReactNode } from "react";
import { Play } from "lucide-react";
import type { TFunction } from "i18next";
import type { Video } from "../lib/api";
import { formatDuration } from "../lib/format";
import type { Column } from "../lib/tableColumns";
import { IconButton } from "./ui/IconButton";
import { Pressable } from "./ui/Pressable";
// Column definitions for the playlist detail table, in their own file like `channelColumns.tsx`.
@@ -18,16 +18,15 @@ export function playlistColumns({
t,
indexOf,
onPlay,
onRemove,
canEdit,
more,
}: {
t: TFunction;
/** Position in the STORED order (1-based on screen) — not the row's position in the current view,
* so a view-sort never renumbers the playlist under the reader. */
indexOf: (v: Video) => number;
onPlay: (v: Video) => void;
onRemove: (v: Video) => void;
canEdit: boolean;
/** The row's "…" — the visible door to its menu (E28 C1), where "Remove" now lives. */
more: (v: Video) => ReactNode;
}): Column<Video>[] {
const cols: Column<Video>[] = [
{ key: GRIP_KEY, header: "", width: "2.25rem", render: () => null, hideInCard: true },
@@ -38,6 +37,8 @@ export function playlistColumns({
width: "4.5rem",
nowrap: true,
hideInCard: true,
// The position number is the first thing a phone row can spare; the order still shows.
phoneHidden: true,
// Sorting by "#" ascending is the stored order itself — that is what makes it a truthful
// default for the header arrow, and a one-click way back from any other sort.
sortable: true,
@@ -61,12 +62,14 @@ export function playlistColumns({
<Pressable
onClick={() => onPlay(v)}
aria-label={`${t("card.play")} — ${v.title}`}
className="relative w-15.5 h-8.75 rounded-sm overflow-hidden bg-surface shrink-0 group touch-target"
// A NAMED group: the row is a `group` too (its "…" reveals on the row's hover), and a
// plain group-hover here would light the play overlay whenever the row is hovered.
className="relative w-15.5 h-8.75 rounded-sm overflow-hidden bg-surface shrink-0 group/thumb touch-target"
>
{v.thumbnail_url && (
<img src={v.thumbnail_url} alt="" className="w-full h-full object-cover" />
)}
<span className="absolute inset-0 grid place-items-center bg-black/40 opacity-0 group-hover:opacity-100 group-focus-visible:opacity-100 transition">
<span className="absolute inset-0 grid place-items-center bg-black/40 opacity-0 group-hover/thumb:opacity-100 group-focus-visible/thumb:opacity-100 transition">
<Play className="w-4 h-4 text-white fill-current" />
</span>
</Pressable>
@@ -85,6 +88,7 @@ export function playlistColumns({
key: "channel",
header: t("playlists.colChannel"),
width: "12rem",
phoneHidden: true,
nowrap: true,
sortable: true,
sortValue: (v) => v.channel_title ?? "",
@@ -123,25 +127,17 @@ export function playlistColumns({
},
];
if (canEdit) {
cols.push({
key: "actions",
header: "",
align: "right",
width: "2.5rem",
nowrap: true,
render: (v) => (
<IconButton
variant="danger"
onClick={() => onRemove(v)}
label={t("playlists.removeItemOf", { title: v.title })}
title={t("playlists.removeItem")}
>
<X />
</IconButton>
),
});
}
cols.push({
key: "actions",
header: "",
align: "right",
// Room for the "…" at 44px on touch (`.touch-target`), not just its 28px under a mouse.
width: "3.25rem",
nowrap: true,
// The X that stood here went into the menu with the Delete key and an undo toast (E28 C1): a
// one-click destructive button on every row, for an action the toast now makes reversible.
render: more,
});
return cols;
}
@@ -1,3 +1,4 @@
import { type Ref } from "react";
import { MoreHorizontal } from "lucide-react";
import { IconButton } from "./IconButton";
import { type IconButtonSize, type IconButtonVariant } from "./buttonStyles";
@@ -13,6 +14,7 @@ export function MoreButton({
variant = "ghost",
size = "sm",
className,
ref,
}: {
/** The menu's name ("Video actions"): the button's accessible name and tooltip. */
label: string;
@@ -21,9 +23,12 @@ export function MoreButton({
variant?: IconButtonVariant;
size?: IconButtonSize;
className?: string;
/** For a caller that hands the focus back here — an in-place rename the menu opened. */
ref?: Ref<HTMLButtonElement>;
}) {
return (
<IconButton
ref={ref}
variant={variant}
size={size}
label={label}
+4 -1
View File
@@ -10,14 +10,17 @@ import { useListNav } from "../../lib/listNav";
export default function NavGrid({
as: Tag = "div",
className,
onDelete,
children,
}: {
as?: "div" | "tbody";
className?: string;
/** The Delete key takes the focused item out (lib/listNav): its index among the items. */
onDelete?: (index: number) => void;
children: ReactNode;
}) {
const ref = useRef<HTMLDivElement & HTMLTableSectionElement>(null);
useListNav(ref, { enabled: true });
useListNav(ref, { enabled: true, onDelete });
return (
<Tag ref={ref} className={className}>
{children}
+1 -1
View File
@@ -219,7 +219,7 @@ export function ColumnHead<T>({
<th
key={col.key}
style={col.width ? { width: col.width } : undefined}
className={`relative font-medium px-2 py-2 whitespace-nowrap bg-bg ${alignClass(col.align)}`}
className={`relative font-medium px-2 py-2 whitespace-nowrap bg-bg ${alignClass(col.align)}${col.phoneHidden ? " hidden sm:table-cell" : ""}`}
aria-sort={
col.sortable
? isSorted
@@ -8,7 +8,7 @@ import { renderHook, act, cleanup } from "@testing-library/react";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import type { ReactNode } from "react";
const h = vi.hoisted(() => ({ notify: vi.fn(), reportError: vi.fn() }));
const h = vi.hoisted(() => ({ notify: vi.fn(), reportError: vi.fn(), canWrite: true }));
vi.mock("react-i18next", async (importOriginal) => {
const actual = await importOriginal<typeof import("react-i18next")>();
@@ -19,12 +19,15 @@ vi.mock("../lib/notifications", async (importOriginal) => ({
notify: (...a: unknown[]) => h.notify(...a),
}));
vi.mock("../lib/errorDialog", () => ({ reportError: (...a: unknown[]) => h.reportError(...a) }));
vi.mock("./confirmContext", () => ({ useChoice: () => async () => "both" }));
vi.mock("./confirmContext", () => ({
useChoice: () => async () => "both",
useConfirm: () => async () => true,
}));
vi.mock("./playlistsContext", () => ({
usePlaylists: () => ({ selectedId: null }),
usePlaylistsActions: () => ({ setSelectedId: () => {} }),
}));
vi.mock("../lib/useMe", () => ({ useMe: () => ({ can_write: true }) }));
vi.mock("../lib/useMe", () => ({ useMe: () => ({ can_write: h.canWrite }) }));
import { usePlaylistActions } from "./usePlaylistActions";
import { HttpError, api } from "../lib/api";
@@ -34,6 +37,7 @@ afterEach(() => {
vi.restoreAllMocks();
h.notify.mockReset();
h.reportError.mockReset();
h.canWrite = true;
});
function mount() {
@@ -44,7 +48,7 @@ function mount() {
return renderHook(() => usePlaylistActions(), { wrapper });
}
const linked = { id: 4, name: "Mix", kind: "user", yt_playlist_id: "PL1" } as const;
const linked = { id: 4, name: "Mix", kind: "user", yt_playlist_id: "PL1", dirty: false } as const;
describe("usePlaylistActions — deleting a list", () => {
it("does not toast a failure the request layer already put on screen", async () => {
@@ -85,3 +89,69 @@ describe("usePlaylistActions — deleting a list", () => {
);
});
});
// E28 C1: ONE list per playlist, which the rail row and the page header's "…" both open — rename,
// the pin, YouTube (open / push / revert) and delete, grouped, the destructive rows last.
describe("usePlaylistActions — a playlist's menu", () => {
const pin = [{ id: "pin", label: "pin", onSelect: () => {} }];
const rows = (pl: Parameters<ReturnType<typeof usePlaylistActions>["items"]>[0]) => {
const { result } = mount();
return result.current.items(pl, { onRename: () => {}, pin });
};
const ids = (items: { id: string }[]) => items.map((i) => i.id);
it("lists a YouTube-linked list with unpushed edits in full", () => {
const items = rows({ ...linked, dirty: true });
expect(ids(items)).toEqual(["rename", "pin", "open-youtube", "push", "revert", "delete"]);
expect(items.find((i) => i.id === "push")!.label).toBe("playlists.pushToYoutube");
// One separator opens the YouTube group, one the delete.
expect(items.filter((i) => i.separatorBefore).map((i) => i.id)).toEqual([
"open-youtube",
"delete",
]);
});
it("offers a local list an export and nothing to open or revert", () => {
const items = rows({ id: 5, name: "Mine", kind: "user", yt_playlist_id: null, dirty: false });
expect(ids(items)).toEqual(["rename", "pin", "push", "delete"]);
expect(items.find((i) => i.id === "push")!.label).toBe("playlists.exportToYoutube");
});
it("has no push without the YouTube write scope, and no revert for a list already in step", () => {
h.canWrite = false;
expect(ids(rows({ ...linked, dirty: false }))).toEqual([
"rename",
"pin",
"open-youtube",
"delete",
]);
});
it("keeps Watch later's rename and delete in place, off, with the reason under them", () => {
const items = rows({
id: 9,
name: "",
kind: "watch_later",
yt_playlist_id: null,
dirty: false,
});
expect(ids(items)).toEqual(["rename", "pin", "delete"]);
for (const id of ["rename", "delete"]) {
const it = items.find((i) => i.id === id)!;
expect(it.disabled).toBe(true);
expect(it.note).toBe("playlists.builtinLocked");
}
});
it("opens the list on YouTube inside the click, in a new tab", () => {
const open = vi.spyOn(window, "open").mockReturnValue(null);
const row = rows(linked).find((i) => i.id === "open-youtube")!;
expect("inGesture" in row && row.inGesture).toBe(true);
row.onSelect!();
expect(open).toHaveBeenCalledWith(
"https://www.youtube.com/playlist?list=PL1",
"_blank",
"noopener,noreferrer",
);
});
});
@@ -1,86 +0,0 @@
import { useQueryClient } from "@tanstack/react-query";
import { useTranslation } from "react-i18next";
import { qk } from "../lib/queryKeys";
import { api, type Playlist } from "../lib/api";
import { notifyActionError } from "../lib/actionErrors";
import { notifyYouTubeActionError } from "../lib/youtubeErrors";
import { playlistName } from "../lib/playlistName";
import { useChoice } from "./confirmContext";
import { usePlaylists, usePlaylistsActions } from "./playlistsContext";
import { useMe } from "../lib/useMe";
// The two playlist-level mutations that now have TWO entry points each: the detail header's
// pencil/trash buttons, and (E10 S2) the rail row's context menu. They live here rather than in
// either caller because the delete is not one API call — it is a dialog whose SHAPE depends on
// whether the list is linked to YouTube, plus the "what gets selected next" dance — and this
// codebase's recurring bug is the second copy of such a flow drifting from the first.
/** The subset of a playlist these actions need — satisfied by both `Playlist` (the rail's rows)
* and `PlaylistDetail` (the detail pane), which is why neither is named here. */
type Deletable = Pick<Playlist, "id" | "name" | "kind" | "yt_playlist_id">;
export function usePlaylistActions() {
const qc = useQueryClient();
const { t } = useTranslation();
const choose = useChoice();
const canWrite = useMe().can_write;
const { selectedId } = usePlaylists();
const { setSelectedId } = usePlaylistsActions();
async function rename(id: number, name: string): Promise<void> {
const next = name.trim();
if (!next) return;
try {
await api.renamePlaylist(id, next);
} catch (e) {
notifyYouTubeActionError(e, t("playlists.renameFailed"));
}
qc.invalidateQueries({ queryKey: qk.playlists() });
qc.invalidateQueries({ queryKey: qk.playlist(id) });
}
async function remove(pl: Deletable): Promise<void> {
if (pl.kind === "watch_later") return; // the built-in list has no delete anywhere in the UI
// ONE dialog with every outcome. It used to be two chained confirms, where the second one's
// Cancel/Escape/backdrop meant "delete here only" — dismissing the dialog deleted the playlist.
const offerYoutube = !!pl.yt_playlist_id && canWrite;
const name = playlistName(pl, t);
const picked = await choose({
title: t("playlists.delete"),
message: offerYoutube
? t("playlists.confirmDeleteLinked", { name })
: t("playlists.confirmDelete", { name }),
choices: offerYoutube
? [
{ id: "here", label: t("playlists.deleteHereOnly"), danger: true },
{ id: "both", label: t("playlists.deleteBoth"), danger: true },
]
: [{ id: "here", label: t("playlists.delete"), danger: true }],
});
if (picked == null) return;
const onYoutube = picked === "both";
// Pick the next selection from what's left *now* (don't go via null, or the rail's auto-select
// effect would re-pick the just-deleted id from the stale list). Only when we deleted the list
// the user is looking at — deleting some other row from its context menu must not move them.
if (selectedId === pl.id) {
const listed = qc.getQueryData<Playlist[]>(qk.playlists()) ?? [];
setSelectedId(listed.find((p) => p.id !== pl.id)?.id ?? null);
}
try {
await api.deletePlaylist(pl.id, onYoutube);
} catch (err) {
// Prefer the server's specific reason (e.g. a local vs YouTube-side failure) over a blanket
// "couldn't delete on YouTube" that misleads when the failure wasn't the YouTube side — unless
// `req` already put it in its error dialog: not twice (notifyActionError checks).
notifyActionError(
err,
onYoutube ? t("playlists.deleteYtFailed") : t("playlists.deleteFailed"),
"warning",
);
}
qc.removeQueries({ queryKey: qk.playlist(pl.id) });
qc.invalidateQueries({ queryKey: qk.playlists() });
}
return { rename, remove };
}
@@ -0,0 +1,293 @@
import { useState } from "react";
import { useQueryClient } from "@tanstack/react-query";
import { useTranslation } from "react-i18next";
import { ExternalLink, Pencil, RotateCcw, Trash2, Upload, Youtube } from "lucide-react";
import { qk } from "../lib/queryKeys";
import { api, type Playlist } from "../lib/api";
import { notify } from "../lib/notifications";
import { notifyActionError } from "../lib/actionErrors";
import { notifyYouTubeActionError } from "../lib/youtubeErrors";
import { playlistName } from "../lib/playlistName";
import { useChoice, useConfirm } from "./confirmContext";
import { usePlaylists, usePlaylistsActions } from "./playlistsContext";
import { useMe } from "../lib/useMe";
import { type ActionMenuItem } from "./ui/ActionMenu";
// A playlist's ONE action list (E28 C1) and the mutations behind it. Two entry points open it — the
// rail row (right-click, long-press, its "…") and the list page header's "…" — so it lives here,
// not in either caller: the delete is not one API call but a dialog whose SHAPE depends on whether
// the list is linked to YouTube, plus the "what gets selected next" dance, and the push is a plan, a
// quota check and a confirm. This codebase's recurring bug is the second copy of such a flow
// drifting from the first.
/** The subset of a playlist these actions need — satisfied by both `Playlist` (the rail's rows)
* and `PlaylistDetail` (the detail pane), which is why neither is named here. */
type Menuable = Pick<Playlist, "id" | "name" | "kind" | "yt_playlist_id" | "dirty">;
export function usePlaylistActions() {
const qc = useQueryClient();
const { t } = useTranslation();
const choose = useChoice();
const confirm = useConfirm();
const canWrite = useMe().can_write;
const { selectedId } = usePlaylists();
const { setSelectedId } = usePlaylistsActions();
// Which list a push / revert is running for, so its row is off (and says so) until it is done.
const [pushing, setPushing] = useState<number | null>(null);
const [reverting, setReverting] = useState<number | null>(null);
const refresh = (id: number) => {
qc.invalidateQueries({ queryKey: qk.playlists() });
qc.invalidateQueries({ queryKey: qk.playlist(id) });
};
async function rename(id: number, name: string): Promise<void> {
const next = name.trim();
if (!next) return;
try {
await api.renamePlaylist(id, next);
} catch (e) {
notifyYouTubeActionError(e, t("playlists.renameFailed"));
}
refresh(id);
}
async function remove(pl: Pick<Menuable, "id" | "name" | "kind" | "yt_playlist_id">) {
if (pl.kind === "watch_later") return; // the built-in list has no delete anywhere in the UI
// ONE dialog with every outcome. It used to be two chained confirms, where the second one's
// Cancel/Escape/backdrop meant "delete here only" — dismissing the dialog deleted the playlist.
const offerYoutube = !!pl.yt_playlist_id && canWrite;
const name = playlistName(pl, t);
const picked = await choose({
title: t("playlists.delete"),
message: offerYoutube
? t("playlists.confirmDeleteLinked", { name })
: t("playlists.confirmDelete", { name }),
choices: offerYoutube
? [
{ id: "here", label: t("playlists.deleteHereOnly"), danger: true },
{ id: "both", label: t("playlists.deleteBoth"), danger: true },
]
: [{ id: "here", label: t("playlists.delete"), danger: true }],
});
if (picked == null) return;
const onYoutube = picked === "both";
// Pick the next selection from what's left *now* (don't go via null, or the rail's auto-select
// effect would re-pick the just-deleted id from the stale list). Only when we deleted the list
// the user is looking at — deleting some other row from its context menu must not move them.
if (selectedId === pl.id) {
const listed = qc.getQueryData<Playlist[]>(qk.playlists()) ?? [];
setSelectedId(listed.find((p) => p.id !== pl.id)?.id ?? null);
}
try {
await api.deletePlaylist(pl.id, onYoutube);
} catch (err) {
// Prefer the server's specific reason (e.g. a local vs YouTube-side failure) over a blanket
// "couldn't delete on YouTube" that misleads when the failure wasn't the YouTube side — unless
// `req` already put it in its error dialog: not twice (notifyActionError checks).
notifyActionError(
err,
onYoutube ? t("playlists.deleteYtFailed") : t("playlists.deleteFailed"),
"warning",
);
}
qc.removeQueries({ queryKey: qk.playlist(pl.id) });
qc.invalidateQueries({ queryKey: qk.playlists() });
}
async function revert(pl: Menuable) {
if (reverting != null) return;
const ok = await confirm({
title: t("playlists.revertTitle"),
message: t("playlists.revertMsg"),
confirmLabel: t("playlists.revertConfirm"),
danger: true,
});
if (!ok) return;
setReverting(pl.id);
try {
await api.revertPlaylist(pl.id);
refresh(pl.id);
notify({
level: "success",
message: t("playlists.revertDone", { name: playlistName(pl, t) }),
});
} catch (e) {
notifyYouTubeActionError(e, t("playlists.revertFailed"));
} finally {
setReverting(null);
}
}
async function push(pl: Menuable) {
if (pushing != null) return;
const name = playlistName(pl, t);
setPushing(pl.id);
try {
const plan = await api.playlistPushPlan(pl.id);
const nothingToDo =
plan.action === "update" &&
!plan.add_yt &&
!plan.add_local &&
!plan.remove_yt &&
!plan.remove_local &&
!plan.to_reorder &&
!plan.rename;
if (nothingToDo) {
notify({ level: "info", message: t("playlists.pushUpToDate", { name }) });
return;
}
if (!plan.affordable) {
notify({
level: "warning",
message: t("playlists.pushNoQuota", {
left: plan.remaining_today,
units: plan.units_estimate,
}),
});
return;
}
let message: string;
if (plan.action === "create") {
message = t("playlists.pushPlanCreate", {
count: plan.add_yt,
units: plan.units_estimate,
left: plan.remaining_today,
});
} else {
// One sentence per side that actually changes; the untouched side gets a short "stays
// unchanged" reassurance instead of a "+0 / −0" the reader has to decode.
const here: string[] = [];
if (plan.add_local) here.push(t("playlists.mergeAddLocal", { count: plan.add_local }));
if (plan.remove_local)
here.push(t("playlists.mergeRemoveLocal", { count: plan.remove_local }));
const yt: string[] = [];
if (plan.add_yt) yt.push(t("playlists.mergeAddYt", { count: plan.add_yt }));
if (plan.remove_yt) yt.push(t("playlists.mergeRemoveYt", { count: plan.remove_yt }));
if (plan.to_reorder) yt.push(t("playlists.mergeReorder", { count: plan.to_reorder }));
if (plan.rename) yt.push(t("playlists.mergeRename"));
const sentences = [...here, ...yt];
if (here.length && !yt.length) sentences.push(t("playlists.mergeYtUnchanged"));
if (yt.length && !here.length) sentences.push(t("playlists.mergeLocalUnchanged"));
const quota =
plan.units_estimate > 0
? t("playlists.mergeQuota", {
units: plan.units_estimate,
left: plan.remaining_today,
})
: t("playlists.mergeQuotaFree");
message = sentences.join(" ") + quota;
}
const ok = await confirm({
title: t("playlists.pushTitle"),
message,
confirmLabel: t("playlists.pushConfirm"),
});
if (!ok) return;
const r = await api.pushPlaylist(pl.id);
refresh(pl.id);
if (r.failures.length) {
notify({
level: "warning",
message: t("playlists.pushPartial", { count: r.failures.length }),
});
} else {
notify({ level: "success", message: t("playlists.pushDone", { name }) });
}
} catch (e) {
notifyYouTubeActionError(e, t("playlists.pushFailed"));
} finally {
setPushing(null);
}
}
/** The playlist's menu rows. `onRename` opens the caller's own rename field (the rail renames in
* the row, the header in the title); `pin` is the caller's comfort-shelf row, whose "is it
* pinned?" read follows its own menu's open state. */
function items(
pl: Menuable,
{ onRename, pin }: { onRename: () => void; pin: ActionMenuItem[] },
): ActionMenuItem[] {
const builtin = pl.kind === "watch_later";
const linked = !builtin && !!pl.yt_playlist_id;
// Interpolated, not spelled out: the HU copy said "Watch later", a name the Hungarian UI never
// shows for this list ("Megnézem később") — so the message named a list the reader had never seen
// (E10 review round 1). Passing the localized name makes the two impossible to drift.
const lockedNote = builtin
? t("playlists.builtinLocked", { name: t("playlists.watchLater") })
: undefined;
const busy = (id: number | null) => id === pl.id;
const youtube: ActionMenuItem[] = [
...(pl.yt_playlist_id
? [
{
id: "open-youtube",
label: t("playlists.openOnYoutube"),
icon: <ExternalLink className="w-4 h-4" />,
// A new tab only opens inside the click (a popup blocker refuses one a task later).
inGesture: true,
onSelect: () =>
void window.open(
`https://www.youtube.com/playlist?list=${pl.yt_playlist_id}`,
"_blank",
"noopener,noreferrer",
),
},
]
: []),
...(!builtin && canWrite
? [
{
id: "push",
label: linked ? t("playlists.pushToYoutube") : t("playlists.exportToYoutube"),
icon: busy(pushing) ? (
<Upload className="w-4 h-4 animate-pulse" />
) : linked ? (
<Youtube className="w-4 h-4" />
) : (
<Upload className="w-4 h-4" />
),
disabled: busy(pushing),
onSelect: () => void push(pl),
},
]
: []),
...(linked && pl.dirty
? [
{
id: "revert",
label: t("playlists.revert"),
icon: <RotateCcw className={`w-4 h-4${busy(reverting) ? " animate-spin" : ""}`} />,
danger: true,
disabled: busy(reverting),
onSelect: () => void revert(pl),
},
]
: []),
];
return [
{
id: "rename",
label: t("playlists.rename"),
icon: <Pencil className="w-4 h-4" />,
disabled: builtin,
note: lockedNote,
onSelect: onRename,
},
...pin,
...youtube.map((it, i) => ({ ...it, separatorBefore: i === 0 })),
{
id: "delete",
label: t("playlists.delete"),
icon: <Trash2 className="w-4 h-4" />,
separatorBefore: true,
danger: true,
disabled: builtin,
note: lockedNote,
onSelect: () => void remove(pl),
},
];
}
return { rename, remove, items };
}
+1
View File
@@ -35,6 +35,7 @@
"listStep": "Next / previous item",
"gridMove": "Move around the list",
"open": "Open the selected item",
"listRemove": "Remove the selected item from the list",
"cap": {
"space": "Space",
"menu": "Menu"
+2 -1
View File
@@ -75,6 +75,8 @@
"moveNoTargets": "There is no other playlist to move it to yet.",
"movedToast": "“{{title}}” moved to “{{name}}” ✓",
"moveFailed": "Couldn't move the video.",
"removedToast": "“{{title}}” removed from the playlist",
"undoRemoveFailed": "Couldn't put the video back.",
"removeFailed": "Couldn't remove the video.",
"errors": {
"name_required": "Give the playlist a name.",
@@ -97,7 +99,6 @@
"builtinLocked": "The built-in {{name}} list can’t be renamed or deleted.",
"removeItem": "Remove",
"removeItemOf": "Remove “{{title}}” from the playlist",
"confirmRemove": "Remove “{{title}}” from this playlist?",
"dragHandleOf": "Reorder “{{title}}”",
"dragBlockedSort": "Reordering by hand is off while a column sort or grouping is applied.",
"dragBlockedFilter": "Reordering by hand is off while a filter is applied.",
+1
View File
@@ -35,6 +35,7 @@
"listStep": "Következő / előző elem",
"gridMove": "Mozgás a listában",
"open": "A kijelölt elem megnyitása",
"listRemove": "A kijelölt elem eltávolítása a listából",
"cap": {
"space": "Szóköz",
"menu": "Menü"
+2 -1
View File
@@ -75,6 +75,8 @@
"moveNoTargets": "Még nincs másik lejátszási lista, ahová áthelyezhetnéd.",
"movedToast": "„{{title}}” áthelyezve ide: „{{name}}” ✓",
"moveFailed": "Nem sikerült áthelyezni a videót.",
"removedToast": "„{{title}}” eltávolítva a listáról",
"undoRemoveFailed": "Nem sikerült visszatenni a videót.",
"removeFailed": "Nem sikerült eltávolítani a videót.",
"errors": {
"name_required": "Adj nevet a lejátszási listának.",
@@ -97,7 +99,6 @@
"builtinLocked": "A beépített „{{name}}” lista nem nevezhető át és nem törölhető.",
"removeItem": "Eltávolítás",
"removeItemOf": "„{{title}}” eltávolítása a listáról",
"confirmRemove": "Eltávolítod a(z) „{{title}}” videót erről a listáról?",
"dragHandleOf": "„{{title}}” átrendezése",
"dragBlockedSort": "A kézi átrendezés ki van kapcsolva, amíg oszloprendezés vagy csoportosítás van érvényben.",
"dragBlockedFilter": "A kézi átrendezés ki van kapcsolva, amíg szűrő van érvényben.",
+4 -2
View File
@@ -68,10 +68,12 @@ export const playlistsApi = {
req(`/api/playlists/${id}`, { method: "PATCH", body: JSON.stringify({ name }) }),
deletePlaylist: (id: number, onYoutube = false) =>
req(`/api/playlists/${id}${onYoutube ? "?on_youtube=true" : ""}`, { method: "DELETE" }),
addToPlaylist: (id: number, videoId: string) =>
// `position` (an index in the stored order) puts it back where a removal took it from — the
// list page's Undo; omitted, it is appended.
addToPlaylist: (id: number, videoId: string, position?: number) =>
req(`/api/playlists/${id}/items`, {
method: "POST",
body: JSON.stringify({ video_id: videoId }),
body: JSON.stringify({ video_id: videoId, ...(position != null && { position }) }),
}),
removeFromPlaylist: (id: number, videoId: string) =>
req(`/api/playlists/${id}/items/${videoId}`, { method: "DELETE" }),
+13 -2
View File
@@ -31,13 +31,19 @@ export type KeyScope =
| "page"
| "search"
| "list"
| "listDelete"
| "undo"
| "contextMenu"
| "ytPlayer"
| "plexPlayer"
| "plexBrowse";
const PAGE_LEVEL: ReadonlySet<KeyScope> = new Set<KeyScope>(["page", "search", "list"]);
const PAGE_LEVEL: ReadonlySet<KeyScope> = new Set<KeyScope>([
"page",
"search",
"list",
"listDelete",
]);
/** True for a scope that acts on the page beneath the overlays (see `KeyScope`). */
export const isPageLevelScope = (scope: KeyScope): boolean => PAGE_LEVEL.has(scope);
@@ -132,6 +138,9 @@ export const SHORTCUTS = [
{ id: "up", scope: "list", keys: ["ArrowUp"], labelKey: "keys.gridMove" },
{ id: "down", scope: "list", keys: ["ArrowDown"], labelKey: "keys.gridMove" },
{ id: "open", scope: "list", keys: ["Enter"], labelKey: "keys.open" },
// Its own scope beside `list`: only a list that can take an item out (a playlist's rows) registers
// it, so the sheet does not promise Delete on the feed or a Plex grid, where nothing answers it.
{ id: "remove", scope: "listDelete", keys: ["Delete"], labelKey: "keys.listRemove" },
// --- the row context menu (wherever useContextMenu is wired) ---------------------------------
// Its OWN scope, not `global`: only the surfaces that actually call `useContextMenu` register it,
// so the sheet stops advertising "Open the context menu" on the feed, Plex, Downloads and Settings
@@ -412,7 +421,9 @@ export function useActiveScopes(): KeyScope[] {
// registers `page` in its own effect, which React runs after every child's — so by mount order
// the module jumps would head the sheet, above the list keys of the page you are looking at.
const surfaces = live.filter((s) => !isPageLevelScope(s));
const pageLevel = (["list", "search", "page"] as const).filter((s) => live.includes(s));
const pageLevel = (["list", "listDelete", "search", "page"] as const).filter((s) =>
live.includes(s),
);
return [...surfaces, ...pageLevel, "global"];
}
+53
View File
@@ -562,3 +562,56 @@ describe("useListNav · leaving the list, and naming the item (deep-review E11 r
);
});
});
describe("useListNav · Delete (E28 S3e2: a playlist row's X became the Delete key)", () => {
function Deletable({ onDelete }: { onDelete?: boolean }) {
const [ids, setIds] = useState([0, 1, 2]);
const ref = useRef<HTMLDivElement>(null);
useListNav(ref, {
enabled: true,
onDelete:
onDelete === false ? undefined : (i) => setIds((xs) => xs.filter((_, j) => j !== i)),
});
return (
<div ref={ref}>
{ids.map((id, i) => (
<div key={id} {...navItemAttrs(i)} data-testid={`d${id}`}>
<button data-testid={`d${id}-action`}>action</button>
</div>
))}
</div>
);
}
const gone = (id: string) => screen.queryByTestId(id) === null;
it("takes the focused item out and moves the focus to the one now in its slot", async () => {
render(<Deletable />);
press("j");
press("j");
expect(focused()).toBe("d1");
const e = press("Delete");
expect(e.defaultPrevented).toBe(true);
expect(gone("d1")).toBe(true);
await waitFor(() => expect(focused()).toBe("d2"));
// …so a second Delete takes the next row.
press("Delete");
expect(gone("d2")).toBe(true);
await waitFor(() => expect(focused()).toBe("d0"));
});
it("leaves Delete to a control inside the item", () => {
render(<Deletable />);
act(() => screen.getByTestId("d1-action").focus());
const e = press("Delete");
expect(e.defaultPrevented).toBe(false);
expect(gone("d1")).toBe(false);
});
it("does nothing in a list that cannot remove", () => {
render(<Deletable onDelete={false} />);
press("j");
const e = press("Delete");
expect(e.defaultPrevented).toBe(false);
expect(gone("d0")).toBe(false);
});
});
+16 -4
View File
@@ -16,9 +16,9 @@ function Scopes() {
return <output data-testid="scopes">{useActiveScopes().join(" ")}</output>;
}
function Queue({ count }: { count: number }) {
function Queue({ count, onDelete }: { count: number; onDelete?: (i: number) => void }) {
const ref = useRef<HTMLDivElement>(null);
useListNav(ref, { enabled: true });
useListNav(ref, { enabled: true, onDelete });
return (
<div ref={ref}>
{count === 0 ? (
@@ -34,10 +34,10 @@ function Queue({ count }: { count: number }) {
);
}
const page = (count: number) => (
const page = (count: number, onDelete?: (i: number) => void) => (
<>
<Scopes />
<Queue count={count} />
<Queue count={count} onDelete={onDelete} />
</>
);
const scopes = () => screen.getByTestId("scopes").textContent!.split(" ");
@@ -51,4 +51,16 @@ describe("useListNav · the list key scope", () => {
rerender(page(0));
await waitFor(() => expect(scopes()).not.toContain("list"));
});
// E28 S3e2: Delete is on the sheet only where a list can take an item out — a playlist's rows, not
// the feed or a Plex grid, where nothing answers it.
it("puts Delete on the sheet only for a list that removes items", async () => {
const { rerender } = render(page(2));
await waitFor(() => expect(scopes()).toContain("list"));
expect(scopes()).not.toContain("listDelete");
rerender(page(2, () => {}));
await waitFor(() => expect(scopes()).toContain("listDelete"));
rerender(page(0, () => {}));
await waitFor(() => expect(scopes()).not.toContain("listDelete"));
});
});
+44 -1
View File
@@ -33,6 +33,8 @@ interface NavList {
readonly count: () => number;
readonly cols: () => number;
readonly reveal: (index: number) => void;
/** Takes the item at `index` out of the list (Delete), or null for a list that cannot. */
readonly remove: () => ((index: number) => void) | null;
}
const lists = new Map<HTMLElement, NavList>();
@@ -316,6 +318,38 @@ function onKey(e: KeyboardEvent): void {
target.click();
},
});
dispatch(e, "listDelete", {
// On the item root, like Enter — and only in a list that registered a remover.
remove: (ev) => {
const cur = current();
const remove = cur?.list.remove();
if (!cur || !remove || document.activeElement !== cur.item) return;
ev.preventDefault();
remove(cur.index);
refocusSlot(cur.list, cur.index, cur.item);
},
});
}
// How long a removal gets to take its row out of the DOM before the focus gives up following it.
const REMOVE_FRAMES = 30;
/** After the focused item was removed, focus the item now in its slot (the next one; the new last
* when it was the last), so a second Delete takes the next row — the way a mail list behaves. Only
* while nothing else took the focus meanwhile: the removal fell back to <body>, nothing more. */
function refocusSlot(list: NavList, index: number, removed: HTMLElement): void {
let frames = 0;
const wait = () => {
if (removed.isConnected) {
if (++frames < REMOVE_FRAMES) requestAnimationFrame(wait);
return;
}
const active = document.activeElement;
if (active && active !== document.body && !isStandInFocus(active)) return;
const n = list.count();
if (n > 0) focusIndex(list, Math.min(index, n - 1));
};
requestAnimationFrame(wait);
}
// ── The item a view was left from ────────────────────────────────────────────────────────────────
@@ -417,7 +451,14 @@ export function restoreNavPosition(pos: NavPosition): () => void {
* the count and the columns are then measured from the DOM at the moment a key is pressed. */
export function useListNav(
ref: RefObject<HTMLElement | null>,
opts: { enabled: boolean; count?: number; cols?: number; reveal?: (index: number) => void },
opts: {
enabled: boolean;
count?: number;
cols?: number;
reveal?: (index: number) => void;
/** A list whose items the Delete key takes out (a playlist's rows): the item's index. */
onDelete?: (index: number) => void;
},
): void {
const optsRef = useRef(opts);
optsRef.current = opts;
@@ -426,6 +467,7 @@ export function useListNav(
// arrows and Enter over nothing (keymap: a scope is pushed only while its keys really do something).
const [hasItems, setHasItems] = useState(false);
useKeyScope("list", opts.enabled && hasItems);
useKeyScope("listDelete", opts.enabled && hasItems && !!opts.onDelete);
useEffect(() => {
const el = ref.current;
if (!opts.enabled || !el) return;
@@ -434,6 +476,7 @@ export function useListNav(
count: () => optsRef.current.count ?? ownItems(el).length,
cols: () => optsRef.current.cols ?? measuredCols(el),
reveal: (i) => optsRef.current.reveal?.(i),
remove: () => optsRef.current.onDelete ?? null,
});
el.setAttribute(LIST, "");
const check = () => setHasItems(el.querySelector(`[${INDEX}]`) !== null);
+3
View File
@@ -37,6 +37,9 @@ export interface Column<T> {
cardPrimary?: boolean;
hideInCard?: boolean;
cardLabel?: boolean;
// A column a table WITHOUT the card fallback (the playlist's drag-reorderable hybrid) leaves out
// below `sm`: on a phone its fixed widths alone overran the screen and left the title no width.
phoneHidden?: boolean;
}
export type FilterMap = Record<string, string | string[]>;