From 89000538d47c24a56c5ac4bbc3c61bb07325a67d Mon Sep 17 00:00:00 2001 From: npeter83 Date: Mon, 20 Jul 2026 23:18:46 +0200 Subject: [PATCH 1/6] refactor(table): extract the column header out of DataTable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sort cell, filter funnel/popover and filter predicate move to components/tableHeader.tsx so a non-DataTable body can render under the same headers — the playlist manager's drag-reorderable list needs the headers without inheriting DataTable's internal ordering state. FilterPopover reaches module scope in the process. It was declared inside DataTable's body, so it was a new component type on every render and React remounted it on each keystroke; verified against the pre-change build, where the input's DOM node is replaced on all four keystrokes of a test word. Also picks up two things that live in this markup: aria-sort on the sortable headers, and a real hit target on the filter funnel (it was 14x14). --- frontend/src/components/DataTable.tsx | 230 +++----------------- frontend/src/components/tableHeader.tsx | 267 ++++++++++++++++++++++++ 2 files changed, 291 insertions(+), 206 deletions(-) create mode 100644 frontend/src/components/tableHeader.tsx diff --git a/frontend/src/components/DataTable.tsx b/frontend/src/components/DataTable.tsx index e6cf2ee..09fc18d 100644 --- a/frontend/src/components/DataTable.tsx +++ b/frontend/src/components/DataTable.tsx @@ -1,58 +1,29 @@ import { useEffect, useRef, useState, type ReactNode } from "react"; import { useTranslation } from "react-i18next"; -import { ArrowDown, ArrowUp, ListFilter, X } from "lucide-react"; -import { useDismiss } from "../lib/useDismiss"; -import { useScrollFade } from "../lib/useScrollFade"; import { sortRows, cycleSort, type SortState } from "../lib/columnSort"; import Pager, { hasPagerContent } from "./Pager"; import { PageToolbar } from "./PageShell"; +import { + ColumnHead, + alignClass, + filterRows, + type Column, + type FilterMap, +} from "./tableHeader"; // A reusable, client-side data table: per-column sort + filter (in the header) + pagination, // with its sort/filter/page state optionally persisted to localStorage so a reload (F5) keeps // the view. Columns are declared by the caller; the table is generic over the row type, so -// other modules (e.g. the playlist manager) can reuse it. Below `md` it falls back to a card -// list built from the same column definitions. +// other modules can reuse it. Below `md` it falls back to a card list built from the same column +// definitions. +// +// The header row itself (sort cell + filter funnel/popover + the filter predicate) lives in +// `tableHeader.tsx` — the playlist manager renders its own drag-reorderable body under the same +// headers without inheriting this component's ordering state. -// The multi-select filter's option list, capped and scrollable. Its own component (module level, -// not nested in DataTable) because it owns a hook: FilterPopover is redeclared on every DataTable -// render, so React remounts it — state parked in there would churn. -function FilterOptionList({ children }: { children: ReactNode }) { - const fade = useScrollFade(); - return ( -
- {children} -
- ); -} +// Re-exported so the many `import { type Column } from "./DataTable"` call sites keep working. +export type { Column } from "./tableHeader"; -type ColumnFilter = - | { kind: "text"; get: (row: T) => string } - | { kind: "select"; options: { value: string; label: string }[]; test: (row: T, value: string) => boolean } - | { kind: "multi"; options: { value: string; label: string }[]; test: (row: T, values: string[]) => boolean }; - -export interface Column { - key: string; - header: string; - render: (row: T) => ReactNode; - align?: "left" | "right" | "center"; - width?: string; - sortable?: boolean; - sortValue?: (row: T) => string | number; - filter?: ColumnFilter; - // Keep the cell on one line so the column auto-sizes to its content (table-layout: auto). - nowrap?: boolean; - // Card fallback (below md): `cardPrimary` renders as the card heading (no label); `hideInCard` - // omits the column; `cardLabel:false` shows the value without its header label. - cardPrimary?: boolean; - hideInCard?: boolean; - cardLabel?: boolean; -} - -type FilterMap = Record; interface Persisted { sort: SortState; filters: FilterMap; @@ -71,10 +42,6 @@ function loadPersist(key?: string): Persisted { } } -function isActive(value: string | string[] | undefined): boolean { - return Array.isArray(value) ? value.length > 0 : !!value; -} - export default function DataTable({ rows, columns, @@ -132,8 +99,6 @@ export default function DataTable({ const [filters, setFilters] = useState(initial.filters); const [page, setPage] = useState(initial.page); const [size, setSize] = useState(initial.size ?? pageSize); - const [openFilter, setOpenFilter] = useState(null); - const popRef = useRef(null); useEffect(() => { if (persistKey) localStorage.setItem(persistKey, JSON.stringify({ sort, filters, page, size })); @@ -158,21 +123,7 @@ export default function DataTable({ setPage(0); }, [resetFiltersToken]); - useDismiss(!!openFilter, () => setOpenFilter(null), popRef); - - const filtered = rows.filter((row) => - columns.every((col) => { - const f = col.filter; - if (!f) return true; - const val = filters[col.key]; - if (!isActive(val)) return true; - if (f.kind === "text") return f.get(row).toLowerCase().includes(String(val).toLowerCase()); - if (f.kind === "select") return f.test(row, String(val)); - return f.test(row, val as string[]); - }) - ); - - const sorted = sortRows(filtered, columns, sort); + const sorted = sortRows(filterRows(rows, columns, filters), columns, sort); // size <= 0 means "All" — one page with every row. const allRows = size <= 0; @@ -191,95 +142,6 @@ export default function DataTable({ setPage(0); } - const align = (a?: "left" | "right" | "center") => - a === "right" ? "text-right" : a === "center" ? "text-center" : "text-left"; - - function FilterPopover({ col }: { col: Column }) { - const f = col.filter!; - const val = filters[col.key]; - return ( -
- {f.kind === "text" && ( - applyFilter(col.key, e.target.value)} - placeholder={col.header} - className="w-full bg-card border border-border rounded-md px-2 py-1 text-xs outline-none focus:border-accent" - /> - )} - {f.kind === "select" && ( -
- - {f.options.map((o) => ( - - ))} -
- )} - {f.kind === "multi" && ( - - {f.options.length === 0 && ( - {t("datatable.noOptions")} - )} - {f.options.map((o) => { - const arr = (val as string[]) ?? []; - const on = arr.includes(o.value); - return ( - - ); - })} - - )} - {isActive(val) && ( - - )} -
- ); - } - // The pager + rows-per-page cluster on its own, so it can be rendered a second time at the bottom // (controlsPosition "both") WITHOUT repeating the leading controls (e.g. the status chips) — those // are header controls and belong only at the top. @@ -329,57 +191,13 @@ export default function DataTable({ {/* Wide screens: table. The wrapper isn't clipped so header filter popovers can overflow. */}
- {/* Sticky header row: the column headers stay put at the top of the page scroller while - the rows scroll under them. `bg-bg` on each cell keeps rows from showing through; the - border rides a pseudo-element (box-shadow) since a sticky 's own border scrolls - away with the collapsed table box in some browsers. The `::after` paints a short - fade just BELOW the header so rows dissolve as they slide under it (the header itself - stays crisp — the page scroller's own top fade is off on these pages). */} - - - {columns.map((col) => { - const sorted_ = sort?.key === col.key; - const filterOn = isActive(filters[col.key]); - return ( - - ); - })} - - + {paged.map((row) => ( ({ diff --git a/frontend/src/components/tableHeader.tsx b/frontend/src/components/tableHeader.tsx new file mode 100644 index 0000000..2d7cd3f --- /dev/null +++ b/frontend/src/components/tableHeader.tsx @@ -0,0 +1,267 @@ +import { useRef, useState, type ReactNode, type RefObject } from "react"; +import { useTranslation } from "react-i18next"; +import { ArrowDown, ArrowUp, ListFilter, X } from "lucide-react"; +import { useDismiss } from "../lib/useDismiss"; +import { useScrollFade } from "../lib/useScrollFade"; +import { type SortState } from "../lib/columnSort"; + +// The column-header machinery — the column contract, the sortable header row with its filter +// funnel + popover, and the filter predicate — lifted OUT of DataTable so a non-DataTable body can +// render under the same headers. +// +// Why it lives here and not in DataTable: DataTable owns its filter/sort/page state internally, +// while the playlist manager's body is a drag-reorderable list whose order is the persisted +// server-side one. Two owners of the same ordering fight; composing these pieces instead lets the +// playlist keep its stored order and still get real column headers. Same move as E4 S3's +// ChannelLayoutGrid, which builds a card view straight from the `Column` render closures. + +type ColumnFilter = + | { kind: "text"; get: (row: T) => string } + | { kind: "select"; options: { value: string; label: string }[]; test: (row: T, value: string) => boolean } + | { kind: "multi"; options: { value: string; label: string }[]; test: (row: T, values: string[]) => boolean }; + +export interface Column { + key: string; + header: string; + render: (row: T) => ReactNode; + align?: "left" | "right" | "center"; + width?: string; + sortable?: boolean; + sortValue?: (row: T) => string | number; + filter?: ColumnFilter; + // Keep the cell on one line so the column auto-sizes to its content (table-layout: auto). + nowrap?: boolean; + // Card fallback (below md): `cardPrimary` renders as the card heading (no label); `hideInCard` + // omits the column; `cardLabel:false` shows the value without its header label. + cardPrimary?: boolean; + hideInCard?: boolean; + cardLabel?: boolean; +} + +export type FilterMap = Record; + +export function isActive(value: string | string[] | undefined): boolean { + return Array.isArray(value) ? value.length > 0 : !!value; +} + +/** Apply every column's own filter predicate. A column with no filter, or an empty value, passes. */ +export function filterRows(rows: T[], columns: Column[], filters: FilterMap): T[] { + return rows.filter((row) => + columns.every((col) => { + const f = col.filter; + if (!f) return true; + const val = filters[col.key]; + if (!isActive(val)) return true; + if (f.kind === "text") return f.get(row).toLowerCase().includes(String(val).toLowerCase()); + if (f.kind === "select") return f.test(row, String(val)); + return f.test(row, val as string[]); + }) + ); +} + +export const alignClass = (a?: "left" | "right" | "center") => + a === "right" ? "text-right" : a === "center" ? "text-center" : "text-left"; + +// The multi-select filter's option list, capped and scrollable. Its own component because it owns a +// hook — see the FilterPopover note below for why nesting either of these would remount them. +function FilterOptionList({ children }: { children: ReactNode }) { + const fade = useScrollFade(); + return ( +
+ {children} +
+ ); +} + +// MODULE LEVEL, deliberately. This used to be declared inside DataTable's body, which made it a NEW +// component type on every render: React unmounted and remounted the popover on each keystroke, so +// the caret jumped to the end and IME composition (accents, any CJK input) broke mid-word. The +// `autoFocus` masked it well enough that it read as "the filter is just twitchy". +function FilterPopover({ + col, + value, + onApply, + panelRef, +}: { + col: Column; + value: string | string[] | undefined; + onApply: (key: string, value: string | string[]) => void; + panelRef: RefObject; +}) { + const { t } = useTranslation(); + const f = col.filter!; + return ( +
+ {f.kind === "text" && ( + onApply(col.key, e.target.value)} + placeholder={col.header} + className="w-full bg-card border border-border rounded-md px-2 py-1 text-xs outline-none focus:border-accent" + /> + )} + {f.kind === "select" && ( +
+ + {f.options.map((o) => ( + + ))} +
+ )} + {f.kind === "multi" && ( + + {f.options.length === 0 && ( + {t("datatable.noOptions")} + )} + {f.options.map((o) => { + const arr = (value as string[]) ?? []; + const on = arr.includes(o.value); + return ( + + ); + })} + + )} + {isActive(value) && ( + + )} +
+ ); +} + +/** The sticky ``: one sortable/filterable cell per column. + * + * Sticky-header notes (kept from DataTable, where this look was designed): the header stays put at + * the top of the page scroller while rows scroll under it. `bg-bg` on each cell stops rows showing + * through; the bottom border rides a `box-shadow` because a sticky ``'s own border scrolls away + * with the collapsed table box in some browsers. The `::after` paints a short fade just BELOW the + * header so rows dissolve as they slide under it while the header itself stays crisp — which is why + * pages using this pass `PageShell fadeTop={false}`. */ +export function ColumnHead({ + columns, + sort, + onToggleSort, + filters, + onFilter, +}: { + columns: Column[]; + sort: SortState; + onToggleSort: (key: string) => void; + filters: FilterMap; + onFilter: (key: string, value: string | string[]) => void; +}) { + const { t } = useTranslation(); + const [openFilter, setOpenFilter] = useState(null); + const panelRef = useRef(null); + useDismiss(!!openFilter, () => setOpenFilter(null), panelRef); + + return ( + + + {columns.map((col) => { + const isSorted = sort?.key === col.key; + const filterOn = isActive(filters[col.key]); + return ( + + ); + })} + + + ); +} From 83e673718f15def4e73f238b12a91dad27be8bc9 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Mon, 20 Jul 2026 23:30:14 +0200 Subject: [PATCH 2/6] feat(playlists): hybrid table over the drag list, and guard its destructive paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The column sort and the filters are a VIEW; the playlist's stored order only changes when the user presses "Save this order", which still goes through the undoable path. Picking a sort used to write straight to the server, so merely looking at the list by title rewrote the playlist and marked a YouTube-linked mirror dirty. The table: - playlistColumns.tsx holds the column defs; the body renders them as table rows and fills the grip cell itself, since the handle needs the row's sortable listeners that a render closure can't reach. - The # column shows the STORED position, so a view-sort never renumbers the playlist under the reader. - Saving is withheld while a filter is active (it would drop the hidden items), and drag is disabled under any view, with the reason spelled out rather than silently doing nothing. - applyView/sameOrder live in lib/playlistView.ts with tests: the sort and the channel grouping compose, and getting that wrong produces a plausible order that the save button would then persist. - dnd-kit KeyboardSensor: the grip was focusable and called itself "reorder", but only a pointer could move it. The guards: - Delete is ONE dialog with every outcome. It was two chained confirms where the second one's Cancel/Escape/backdrop meant "delete here only" — dismissing the dialog deleted the playlist. ConfirmProvider gained useChoice() for it. - ConfirmProvider focuses Cancel when an offered action is destructive; the confirm button took autoFocus unconditionally, so a stray Enter on an open dialog ran "Delete user" or "Clear audit log". - Removing an item asks first, and rebases the undo history instead of throwing it away: useUndoable.rebase maps every snapshot, so the reorder history stays valid for the remaining items. - A failed detail query showed "Loading…" forever; it now says so and offers a retry. Rename and reorder failures are surfaced instead of swallowed (reorder was a bare .then(), which also produced an unhandled rejection). - Local reorder/remove no longer require YouTube write scope. - playlistName() replaces the watch-later localizer copied into three files. --- frontend/src/components/AddToPlaylist.tsx | 3 +- frontend/src/components/ConfirmProvider.tsx | 101 ++- frontend/src/components/Playlists.tsx | 818 ++++++++++---------- frontend/src/components/PlaylistsRail.tsx | 4 +- frontend/src/components/playlistColumns.tsx | 128 +++ frontend/src/i18n/locales/en/common.json | 3 +- frontend/src/i18n/locales/en/playlists.json | 28 +- frontend/src/i18n/locales/hu/common.json | 3 +- frontend/src/i18n/locales/hu/playlists.json | 28 +- frontend/src/lib/playlistName.ts | 7 + frontend/src/lib/playlistView.test.ts | 106 +++ frontend/src/lib/playlistView.ts | 42 + frontend/src/lib/useUndoable.ts | 17 + 13 files changed, 851 insertions(+), 437 deletions(-) create mode 100644 frontend/src/components/playlistColumns.tsx create mode 100644 frontend/src/lib/playlistName.ts create mode 100644 frontend/src/lib/playlistView.test.ts create mode 100644 frontend/src/lib/playlistView.ts diff --git a/frontend/src/components/AddToPlaylist.tsx b/frontend/src/components/AddToPlaylist.tsx index 33f3f89..0c161a8 100644 --- a/frontend/src/components/AddToPlaylist.tsx +++ b/frontend/src/components/AddToPlaylist.tsx @@ -6,6 +6,7 @@ import { Check, ListPlus, Plus, Youtube } from "lucide-react"; import { api, type Playlist } from "../lib/api"; import { useDismiss } from "../lib/useDismiss"; import { useScrollFade } from "../lib/useScrollFade"; +import { playlistName } from "../lib/playlistName"; // A small popover (portaled to body, so the feed grid can't clip it) for toggling a // video's membership in the user's playlists, with inline "new playlist" creation. @@ -159,7 +160,7 @@ export default function AddToPlaylist({ {pl.has_video && } - {pl.kind === "watch_later" ? t("playlists.watchLater") : pl.name} + {playlistName(pl, t)} {(pl.source === "youtube" || pl.yt_playlist_id) && ( diff --git a/frontend/src/components/ConfirmProvider.tsx b/frontend/src/components/ConfirmProvider.tsx index 6794a9b..c2d0b03 100644 --- a/frontend/src/components/ConfirmProvider.tsx +++ b/frontend/src/components/ConfirmProvider.tsx @@ -5,6 +5,11 @@ import Modal from "./Modal"; // App-styled, promise-based replacement for window.confirm. Wrap the tree in // once, then anywhere: const confirm = useConfirm(); // if (await confirm({ message: "…", danger: true })) { … } +// +// For a decision with more than two outcomes use `useChoice()` instead — never chain two +// confirms, because the second dialog's Cancel/Escape/backdrop is then indistinguishable from +// a deliberate answer (the playlist delete used to read a dismissal as "delete here only" and +// went ahead and deleted). export interface ConfirmOptions { title?: string; message: ReactNode; @@ -13,62 +18,108 @@ export interface ConfirmOptions { danger?: boolean; } +export interface Choice { + id: string; + label: string; + danger?: boolean; +} + +export interface ChoiceOptions { + title?: string; + message: ReactNode; + cancelLabel?: string; + /** Rendered left→right after Cancel. Resolves to the clicked choice's `id`. */ + choices: Choice[]; +} + +type ChoiceFn = (opts: ChoiceOptions) => Promise; type ConfirmFn = (opts: ConfirmOptions) => Promise; -const ConfirmContext = createContext(() => Promise.resolve(false)); +const ChoiceContext = createContext(() => Promise.resolve(null)); +/** Two outcomes: resolves true on confirm, false on cancel/Escape/backdrop. */ export function useConfirm(): ConfirmFn { - return useContext(ConfirmContext); + const ask = useContext(ChoiceContext); + return useCallback( + async (opts: ConfirmOptions) => { + const picked = await ask({ + title: opts.title, + message: opts.message, + cancelLabel: opts.cancelLabel, + choices: [ + { id: "confirm", label: opts.confirmLabel ?? "", danger: opts.danger }, + ], + }); + return picked === "confirm"; + }, + [ask] + ); +} + +/** Three or more outcomes. Resolves to the chosen id, or **null** when the user cancels, + * presses Escape or clicks the backdrop — dismissal is never one of the actions. */ +export function useChoice(): ChoiceFn { + return useContext(ChoiceContext); } export function ConfirmProvider({ children }: { children: ReactNode }) { const { t } = useTranslation(); const [state, setState] = useState<{ - opts: ConfirmOptions; - resolve: (ok: boolean) => void; + opts: ChoiceOptions; + resolve: (id: string | null) => void; } | null>(null); - const confirm = useCallback( - (opts) => new Promise((resolve) => setState({ opts, resolve })), + const ask = useCallback( + (opts) => new Promise((resolve) => setState({ opts, resolve })), [] ); - function close(ok: boolean) { - state?.resolve(ok); + function close(id: string | null) { + state?.resolve(id); setState(null); } + const choices = state?.opts.choices ?? []; + // Focus Cancel when any offered action is destructive: the confirm button used to take + // autoFocus unconditionally, so a stray Enter on an open dialog executed "Delete user" or + // "Clear audit log" without the user ever looking at it. + const anyDanger = choices.some((c) => c.danger); + return ( - + {children} {state && ( close(false)} - maxWidth="max-w-sm" + onClose={() => close(null)} + maxWidth={choices.length > 1 ? "max-w-md" : "max-w-sm"} >

{state.opts.message}

-
+
- + {choices.map((c, i) => ( + + ))}
)} - + ); } diff --git a/frontend/src/components/Playlists.tsx b/frontend/src/components/Playlists.tsx index 5e2c9fc..aabe7f8 100644 --- a/frontend/src/components/Playlists.tsx +++ b/frontend/src/components/Playlists.tsx @@ -1,8 +1,9 @@ -import { lazy, Suspense, useEffect, useRef, useState } from "react"; +import { lazy, Suspense, useCallback, useEffect, useMemo, useRef, useState } from "react"; import { useTranslation } from "react-i18next"; import { useQuery, useQueryClient } from "@tanstack/react-query"; import { DndContext, + KeyboardSensor, PointerSensor, closestCenter, useSensor, @@ -12,13 +13,14 @@ import { import { SortableContext, arrayMove, + sortableKeyboardCoordinates, useSortable, verticalListSortingStrategy, } from "@dnd-kit/sortable"; import { CSS } from "@dnd-kit/utilities"; import { - ArrowDown, - ArrowUp, + AlertTriangle, + Check, ExternalLink, GripVertical, ListPlus, @@ -28,66 +30,36 @@ import { RotateCcw, Trash2, Upload, - X, Youtube, } from "lucide-react"; import { api, type Video } from "../lib/api"; -import { formatDuration } from "../lib/format"; import { notify } from "../lib/notifications"; import { notifyYouTubeActionError } from "../lib/youtubeErrors"; +import { playlistName } from "../lib/playlistName"; import { useUndoable } from "../lib/useUndoable"; +import { cycleSort, type SortState } from "../lib/columnSort"; +import { applyView, sameOrder } from "../lib/playlistView"; const PlayerModal = lazy(() => import("./PlayerModal")); import UndoToolbar from "./UndoToolbar"; import { PageToolbar } from "./PageShell"; -import { useConfirm } from "./ConfirmProvider"; +import { useChoice, useConfirm } from "./ConfirmProvider"; import { usePlaylists, usePlaylistsActions } from "./PlaylistsProvider"; import { useMe } from "../lib/useMe"; +import { ColumnHead, alignClass, filterRows, isActive, type Column, type FilterMap } from "./tableHeader"; +import { GRIP_KEY, playlistColumns } from "./playlistColumns"; -type SortKey = "manual" | "title" | "duration" | "channel"; -type SortDir = "asc" | "desc"; - -function comparator(key: SortKey, dir: SortDir): ((a: Video, b: Video) => number) | null { - if (key === "manual") return null; - const sign = dir === "asc" ? 1 : -1; - if (key === "duration") - return (a, b) => { - const av = a.duration_seconds; - const bv = b.duration_seconds; - if (av == null && bv == null) return 0; - if (av == null) return 1; // nulls always last, both directions - if (bv == null) return -1; - return sign * (av - bv); - }; - return (a, b) => { - const av = (key === "channel" ? a.channel_title : a.title) ?? ""; - const bv = (key === "channel" ? b.channel_title : b.title) ?? ""; - return sign * av.localeCompare(bv); - }; -} - -// Sort a playlist's items. When `group` is on, items are grouped by channel (groups ordered -// by channel name in the chosen direction) and the chosen sort is applied within each group; -// with key "manual" the within-group order is left as-is. Returns the same array reference -// when nothing would change, so it doesn't create a spurious undo entry. -function sortItems(items: Video[], key: SortKey, dir: SortDir, group: boolean): Video[] { - const cmp = comparator(key, dir); - if (!group) return cmp ? [...items].sort(cmp) : items; - const groups = new Map(); - for (const v of items) { - const k = v.channel_title ?? ""; - const g = groups.get(k); - if (g) g.push(v); - else groups.set(k, [v]); - } - const sign = dir === "asc" ? 1 : -1; - const keys = [...groups.keys()].sort((a, b) => sign * a.localeCompare(b)); - const out: Video[] = []; - for (const k of keys) { - const g = groups.get(k)!; - out.push(...(cmp ? [...g].sort(cmp) : g)); - } - return out; -} +// 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. +// +// The distinction that shapes the whole module: **the column sort and the filters are a VIEW**, +// while the playlist's own order is stored data. Before S4 the sort control wrote straight through +// to the server, so picking "sort by title" silently rewrote the playlist and marked a +// YouTube-linked mirror dirty. Now nothing is written until the user presses "Save this order", +// which still goes through the undoable path — the capability is intact, it just stopped being a +// side effect of looking at the list. +// +// It composes `tableHeader`'s pieces instead of rendering a `DataTable`, because DataTable owns +// its own filter/sort/page state and the stored order needs a single owner. const idsKey = (items: Video[]) => items @@ -97,86 +69,56 @@ const idsKey = (items: Video[]) => function Row({ video, - index, - readOnly, - onPlay, - onRemove, + columns, + dragDisabled, + dragHint, }: { video: Video; - index: number; - readOnly?: boolean; - onPlay: () => void; - onRemove: () => void; + columns: Column
 + ))} + ); } @@ -190,31 +132,44 @@ export default function Playlists() { const { t } = useTranslation(); const qc = useQueryClient(); const confirm = useConfirm(); + const choose = useChoice(); const [renaming, setRenaming] = useState(false); const [renameValue, setRenameValue] = useState(""); - // The item order is undoable: drag, sort and group all go through `order.set`, so each is - // reversible (buttons + Ctrl+Z/Y). onApply persists the new order to the server. + // The stored order is undoable: drag and "save this order" both go through `order.set`, so each + // is reversible (buttons + Ctrl+Z/Y). onApply persists the new order to the server. const order = useUndoable([], { onApply: (next) => { if (selectedId == null) return; - api.reorderPlaylist(selectedId, next.map((v) => v.id)).then(() => { - // Refresh the sidebar (cover may change) and the detail (so the now-dirty state, - // and the Reset/Unsynced indicators, show without an F5). The membership guard - // keeps the refetch from wiping the undo history on a pure reorder. - qc.invalidateQueries({ queryKey: ["playlists"] }); - qc.invalidateQueries({ queryKey: ["playlist", selectedId] }); - }); + api + .reorderPlaylist( + selectedId, + next.map((v) => v.id) + ) + .then(() => { + // Refresh the sidebar (cover may change) and the detail (so the now-dirty state, + // and the Reset/Unsynced indicators, show without an F5). The membership guard + // keeps the refetch from wiping the undo history on a pure reorder. + qc.invalidateQueries({ queryKey: ["playlists"] }); + qc.invalidateQueries({ queryKey: ["playlist", selectedId] }); + }) + .catch((e) => { + // Was a bare .then(): a failed reorder left the UI showing an order the server had + // rejected, and produced an unhandled rejection. Say so and resync from the server. + notifyYouTubeActionError(e, t("playlists.reorderFailed")); + qc.invalidateQueries({ queryKey: ["playlist", selectedId] }); + }); }, }); const items = order.value; - const [sortKey, setSortKey] = useState("manual"); - const [sortDir, setSortDir] = useState("asc"); + // VIEW state — never written to the server on its own. + const [viewSort, setViewSort] = useState(null); + const [filters, setFilters] = useState({}); const [groupBy, setGroupBy] = useState(false); // Tracks the membership (id set) currently loaded into `order`, so a refetch that only // re-confirms our own reorder doesn't wipe the undo history — we reset history only when // the actual set of items changes (different playlist, or an add/remove). const lastSetRef = useRef(""); - // The open player, as an index into `items` (the queue); null = closed. + // The open player, as an index into the DISPLAYED rows (the queue); null = closed. const [playingIndex, setPlayingIndex] = useState(null); const listQuery = useQuery({ @@ -244,43 +199,85 @@ export default function Playlists() { // eslint-disable-next-line react-hooks/exhaustive-deps }, [detail]); - // Reset the sort controls when switching playlists (the order itself is per-playlist). + // Reset the view when switching playlists (the stored order itself is per-playlist). useEffect(() => { - setSortKey("manual"); - setSortDir("asc"); + setViewSort(null); + setFilters({}); setGroupBy(false); }, [selectedId]); - function applySort(key: SortKey, dir: SortDir, group: boolean) { - order.set(sortItems(items, key, dir, group)); - } - const undo = () => { - setSortKey("manual"); - setGroupBy(false); - order.undo(); - }; - const redo = () => { - setSortKey("manual"); - setGroupBy(false); - order.redo(); - }; - - const sensors = useSensors( - useSensor(PointerSensor, { activationConstraint: { distance: 4 } }) - ); - - // Watch later is a built-in playlist: show a localized name and no rename/delete. - const plName = (p: { kind: string; name: string }) => - p.kind === "watch_later" ? t("playlists.watchLater") : p.name; - 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. 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 + // every account without the scope, which they have always had. + const canEdit = editable; const [pushing, setPushing] = useState(false); const [reverting, setReverting] = useState(false); + const onPlay = useCallback((v: Video) => { + setPlayingIndex((_) => shownRef.current.findIndex((x) => x.id === v.id)); + }, []); + const onRemove = useCallback((v: Video) => { + removeRef.current(v); + }, []); + + const storedIndex = useMemo(() => { + const m = new Map(items.map((v, i) => [v.id, i])); + return (v: Video) => m.get(v.id) ?? 0; + }, [items]); + + const columns = useMemo( + () => playlistColumns({ t, indexOf: storedIndex, onPlay, onRemove, canEdit }), + [t, storedIndex, onPlay, onRemove, canEdit] + ); + + const filterActive = Object.values(filters).some(isActive); + const shown = useMemo( + () => applyView(filterRows(items, columns, filters), columns, viewSort, groupBy), + [items, columns, filters, viewSort, groupBy] + ); + // Read by the memoized onPlay callback without making it depend on the list. + const shownRef = useRef(shown); + shownRef.current = shown; + + const viewActive = viewSort !== null || groupBy || filterActive; + // Saving a FILTERED view would drop every hidden item from the playlist, so the button only + // offers itself when the view is a pure reordering of the whole list. + const canSaveOrder = canEdit && !filterActive && !sameOrder(shown, items); + + function toggleSort(key: string) { + setViewSort((s) => cycleSort(s, key)); + } + function applyFilter(key: string, value: string | string[]) { + setFilters((f) => ({ ...f, [key]: value })); + } + function clearView() { + setViewSort(null); + setFilters({}); + setGroupBy(false); + } + function saveOrder() { + order.set(shown); // onApply persists it + clearView(); + } + + const sensors = useSensors( + useSensor(PointerSensor, { activationConstraint: { distance: 4 } }), + // The grip was focusable and called itself "reorder", but only a pointer could move it. + useSensor(KeyboardSensor, { coordinateGetter: sortableKeyboardCoordinates }) + ); + + const plName = (p: { kind: string; name: string }) => playlistName(p, t); + + function refreshAll() { + qc.invalidateQueries({ queryKey: ["playlists"] }); + qc.invalidateQueries({ queryKey: ["playlist", selectedId] }); + } + async function revertToYoutube() { if (selectedId == null || !detail || reverting) return; const ok = await confirm({ @@ -345,7 +342,10 @@ export default function Playlists() { const r = await api.pushPlaylist(selectedId); refreshAll(); if (r.failures.length) { - notify({ level: "warning", message: t("playlists.pushPartial", { count: 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) }) }); } @@ -356,32 +356,31 @@ export default function Playlists() { } } - function refreshAll() { - qc.invalidateQueries({ queryKey: ["playlists"] }); - qc.invalidateQueries({ queryKey: ["playlist", selectedId] }); - } - function onDragEnd(e: DragEndEvent) { const { active: a, over } = e; if (!over || a.id === over.id || selectedId == null) return; const oldIndex = items.findIndex((v) => v.id === a.id); const newIndex = items.findIndex((v) => v.id === over.id); if (oldIndex < 0 || newIndex < 0) return; - // A manual drag breaks any active sort/grouping; reflect that in the controls. - setSortKey("manual"); - setGroupBy(false); order.set(arrayMove(items, oldIndex, newIndex)); // onApply persists } - async function removeItem(videoId: string) { + async function removeItem(video: Video) { if (selectedId == null) return; - // Membership changes aren't undoable: reset the order to the new set (clearing history) - // and keep lastSetRef in sync so the follow-up refetch doesn't reset again. - const next = items.filter((v) => v.id !== videoId); - order.reset(next); + const ok = await confirm({ + title: t("playlists.removeItem"), + message: t("playlists.confirmRemove", { title: video.title }), + confirmLabel: t("playlists.removeItem"), + danger: true, + }); + if (!ok) return; + // 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. + const next = items.filter((v) => v.id !== video.id); + order.rebase((list) => list.filter((v) => v.id !== video.id)); lastSetRef.current = idsKey(next); try { - await api.removeFromPlaylist(selectedId, videoId); + await api.removeFromPlaylist(selectedId, video.id); refreshAll(); } catch { // The optimistic removal above didn't stick — resync from the server to restore the row. @@ -389,37 +388,43 @@ export default function Playlists() { refreshAll(); } } + // Kept in a ref so the memoized column callbacks don't rebuild on every state change. + const removeRef = useRef(removeItem); + removeRef.current = removeItem; async function saveRename() { if (selectedId == null) return; const name = renameValue.trim(); setRenaming(false); if (name && name !== detail?.name) { - await api.renamePlaylist(selectedId, name); + try { + await api.renamePlaylist(selectedId, name); + } catch (e) { + notifyYouTubeActionError(e, t("playlists.renameFailed")); + } refreshAll(); } } async function deletePlaylist() { if (selectedId == null || !detail) return; - const ok = await confirm({ + // 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 = linked && canWrite; + const picked = await choose({ title: t("playlists.delete"), - message: t("playlists.confirmDelete", { name: detail.name }), - confirmLabel: t("playlists.delete"), - danger: true, + message: offerYoutube + ? t("playlists.confirmDeleteLinked", { name: plName(detail) }) + : t("playlists.confirmDelete", { name: plName(detail) }), + choices: offerYoutube + ? [ + { id: "here", label: t("playlists.deleteHereOnly"), danger: true }, + { id: "both", label: t("playlists.deleteOnYoutubeConfirm"), danger: true }, + ] + : [{ id: "here", label: t("playlists.delete"), danger: true }], }); - if (!ok) return; - // For a YouTube-linked playlist, offer to delete it on YouTube too (Cancel = here only). - let onYoutube = false; - if (linked && canWrite) { - onYoutube = await confirm({ - title: t("playlists.deleteOnYoutubeTitle"), - message: t("playlists.deleteOnYoutubeMsg", { name: detail.name }), - confirmLabel: t("playlists.deleteOnYoutubeConfirm"), - cancelLabel: t("playlists.deleteHereOnly"), - danger: true, - }); - } + if (picked == null) return; + const onYoutube = picked === "both"; const deletedId = selectedId; // Pick the next selection from what's left *now* (don't go via null, or the // auto-select effect would re-pick the just-deleted id from the stale list). @@ -428,7 +433,10 @@ export default function Playlists() { try { await api.deletePlaylist(deletedId, onYoutube); } catch { - notify({ level: "warning", message: t("playlists.deleteYtFailed") }); + notify({ + level: "warning", + message: onYoutube ? t("playlists.deleteYtFailed") : t("playlists.deleteFailed"), + }); } qc.removeQueries({ queryKey: ["playlist", deletedId] }); qc.invalidateQueries({ queryKey: ["playlists"] }); @@ -441,237 +449,261 @@ export default function Playlists() { }); } + // Stable identity: PlayerModal's keydown/scroll-lock effect depends on it, so a fresh closure + // every render re-subscribed those listeners and re-ran focusModal() under the open player. + const closePlayer = useCallback(() => setPlayingIndex(null), []); + + const dragHint = filterActive + ? t("playlists.dragBlockedFilter") + : t("playlists.dragBlockedSort"); + + if (selectedId == null) { + return
{t("playlists.pickOne")}
; + } + // The page used to render "Loading…" forever when the detail query failed — an error read as a + // slow network that never arrived. + if (detailQuery.isError) { + return ( +
+ + + {t("playlists.loadFailed")} + + +
+ ); + } + if (!detail) { + return
{t("playlists.loading")}
; + } + return (
- {selectedId == null || !detail ? ( -
- {selectedId == null ? t("playlists.pickOne") : t("playlists.loading")} -
- ) : ( - <> - {/* Header (title, actions, sort/group) is fixed chrome — it portals into the shell's - fixed band, so only the video list scrolls. */} - -
-
-
- {items[0]?.thumbnail_url && ( - - )} -
-
- {renaming ? ( - setRenameValue(e.target.value)} - onBlur={saveRename} - onKeyDown={(e) => { - if (e.key === "Enter") saveRename(); - if (e.key === "Escape") setRenaming(false); - }} - className="text-lg font-semibold bg-card border border-border rounded-md px-2 py-0.5 outline-none focus:border-accent" - /> - ) : ( -
-

{plName(detail)}

- {editable && ( - - )} - {detail.yt_playlist_id && ( - - - - )} -
- )} -
- {t("playlists.itemCount", { count: items.length })} -
-
- - {editable && canWrite && ( - - )} - {linked && detail.dirty && ( - - )} + {/* Header (title, actions, view controls) is fixed chrome — it portals into the shell's + fixed band, so only the video list scrolls. */} + +
+
+
+ {items[0]?.thumbnail_url && ( + + )} +
+
+ {renaming ? ( + setRenameValue(e.target.value)} + onBlur={saveRename} + onKeyDown={(e) => { + if (e.key === "Enter") saveRename(); + if (e.key === "Escape") setRenaming(false); + }} + aria-label={t("playlists.rename")} + className="text-lg font-semibold bg-card border border-border rounded-md px-2 py-0.5 outline-none focus:border-accent" + /> + ) : ( +
+

{plName(detail)}

{editable && ( )} - {linked && detail.dirty && ( - - - {t("playlists.unsynced")} - + + )}
+ )} +
+ {t("playlists.itemCount", { count: items.length })} + {filterActive && ` · ${t("playlists.filteredCount", { count: shown.length })}`} +
+
+ + {editable && canWrite && ( + + )} + {linked && detail.dirty && ( + + )} + {editable && ( + + )} + {linked && detail.dirty && ( + + + {t("playlists.unsynced")} + + )}
+
- {editable && items.length > 1 && ( -
- {t("playlists.sortLabel")} - + {items.length > 1 && ( +
+ + {viewActive && ( - + )} + {canSaveOrder && ( + + )} + {viewActive && ( + {dragHint} + )} + {canEdit && (
-
- )} + )}
- + )} +
+ - {items.length === 0 ? ( -
-
- - {t("playlists.empty")} -
-
- ) : ( - +
+ + {t("playlists.empty")} +
+
+ ) : ( + + {/* Not clipped, so the header's filter popovers can overflow the table box. */} +
+
- - - {col.filter && ( - - )} - - {openFilter === col.key && col.filter && } -
{col.render(row)}
+ + + {col.filter && ( + + )} + + {openFilter === col.key && col.filter && ( + + )} +
- - - )} - {index + 1} - - - {video.duration_seconds != null ? ( - - {formatDuration(video.duration_seconds)} - - ) : video.live_status === "live" || video.live_status === "upcoming" ? ( - - {t(`card.${video.live_status}`)} - - ) : null} - {!readOnly && ( - - )} - + {col.key === GRIP_KEY ? ( + // The one cell the column defs can't render: the handle needs this row's sortable + // attributes/listeners. Disabled (not hidden) under a view, so the affordance stays + // put and can explain itself instead of silently doing nothing. + + ) : ( + col.render(video) + )} +
+ + v.id)} + strategy={verticalListSortingStrategy} > - v.id)} - strategy={verticalListSortingStrategy} - > -
- {items.map((v, i) => ( - setPlayingIndex(i)} - onRemove={() => removeItem(v.id)} - /> - ))} -
- - + + {shown.map((v) => ( + + ))} + + +
+ {shown.length === 0 && ( +
+ {t("playlists.noneMatchFilter")} +
)} - - )} +
+ + )} - {playingIndex != null && items[playingIndex] && ( + {playingIndex != null && shown[playingIndex] && ( setPlayingIndex(null)} + onClose={closePlayer} onState={playerState} /> diff --git a/frontend/src/components/PlaylistsRail.tsx b/frontend/src/components/PlaylistsRail.tsx index d7f09d2..db9d4fe 100644 --- a/frontend/src/components/PlaylistsRail.tsx +++ b/frontend/src/components/PlaylistsRail.tsx @@ -12,6 +12,7 @@ import SidePanel from "./SidePanel"; import PanelGroups, { type PanelItem } from "./PanelGroups"; import { usePlaylists, usePlaylistsActions } from "./PlaylistsProvider"; import { useMe } from "../lib/useMe"; +import { playlistName } from "../lib/playlistName"; type PlSort = { key: "custom" | "name" | "count" | "duration"; dir: "asc" | "desc"; dirtyFirst: boolean }; const PL_SORT_DEFAULT: PlSort = { key: "custom", dir: "asc", dirtyFirst: false }; @@ -69,8 +70,7 @@ export default function PlaylistsRail({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [playlists, selectedId, listQuery.data]); - const plName = (p: { kind: string; name: string }) => - p.kind === "watch_later" ? t("playlists.watchLater") : p.name; + const plName = (p: { kind: string; name: string }) => playlistName(p, t); const sortedPlaylists = useMemo(() => { const sign = plSort.dir === "asc" ? 1 : -1; diff --git a/frontend/src/components/playlistColumns.tsx b/frontend/src/components/playlistColumns.tsx new file mode 100644 index 0000000..720e03a --- /dev/null +++ b/frontend/src/components/playlistColumns.tsx @@ -0,0 +1,128 @@ +import { Play, X } from "lucide-react"; +import type { TFunction } from "i18next"; +import type { Video } from "../lib/api"; +import { formatDuration } from "../lib/format"; +import type { Column } from "./tableHeader"; + +// Column definitions for the playlist detail table, in their own file like `channelColumns.tsx`. +// +// The `grip` column carries no renderer: the drag handle needs the `useSortable` attributes that +// only exist inside the row component, which a `render(row)` closure can't reach. It stays in the +// list anyway so ONE column array drives both the header and the body — the row renderer fills +// that cell itself. Everything else is an ordinary cell. +export const GRIP_KEY = "grip"; + +export function playlistColumns({ + t, + indexOf, + onPlay, + onRemove, + canEdit, +}: { + 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; +}): Column