From f6192aa6bf411fb130a1abdc6342bbfe5641e072 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 21 Aug 2026 16:51:05 +0200 Subject: [PATCH 01/17] =?UTF-8?q?fix(feed):=20re-measure=20header=20on=20f?= =?UTF-8?q?eed=E2=86=94channel=20Back=20so=20--hdr-h=20can't=20stick=20at?= =?UTF-8?q?=208px?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PageShell is reused (never remounted) across feed↔channel navigation — same component position in App.tsx — so a one-shot useLayoutEffect kept its ResizeObserver bound to the FIRST header node. On channel-open that node detached and fired the observer with a 0 box, rewriting --hdr-h to 0+0+8=8px on the still-mounted content div. Back restored the pt-(--hdr-h) pad class with that stale value, sliding the content toolbar up under the header — stuck until reload. Hold both the header and content nodes in state (callback refs) and key the measurement effect to the live nodes, so it re-runs every time the header mounts/unmounts; drop the stale pad when the header is absent (channel page). Adds a regression test: header removed then restored must re-measure, not 8px. --- .../src/components/PageShell.dom.test.tsx | 23 ++++++++++ frontend/src/components/PageShell.tsx | 42 +++++++++++-------- 2 files changed, 47 insertions(+), 18 deletions(-) diff --git a/frontend/src/components/PageShell.dom.test.tsx b/frontend/src/components/PageShell.dom.test.tsx index 8b526bc4..5d1f2a78 100644 --- a/frontend/src/components/PageShell.dom.test.tsx +++ b/frontend/src/components/PageShell.dom.test.tsx @@ -50,4 +50,27 @@ describe("PageShell --hdr-h (E5 S7)", () => { const { container } = render(content); expect(container.querySelector('[class*="pt-(--hdr-h)"]')).toBeNull(); }); + + it("re-measures when the header is removed then restored (feed→channel→back)", () => { + // PageShell is REUSED across feed↔channel navigation (same component position in App.tsx) — it + // never remounts, so the header DOM node is swapped under a stable PageShell. The measurement + // must re-run on that swap; a one-shot effect kept observing the FIRST node and, when it detached, + // rewrote --hdr-h to a stale 8px that stuck on Back. Regression for the channel-Back header overlap. + const hdr = () =>
H
; + const { container, rerender } = render(content); + const measure = (top: number, h: number) => { + const el = container.querySelector('[data-testid="hdr"]') as HTMLElement; + Object.defineProperty(el, "offsetTop", { value: top, configurable: true }); + Object.defineProperty(el, "offsetHeight", { value: h, configurable: true }); + roCb?.(); + }; + measure(12, 52); + const content = () => container.querySelector('[class*="pt-(--hdr-h)"]') as HTMLElement; + expect(content().style.getPropertyValue("--hdr-h")).toBe("72px"); // 12 + 52 + 8 + + rerender(content); // → channel page: header gone + rerender(content); // → Back: header returns as a NEW node + measure(12, 52); // fires the NEW header's observer + expect(content().style.getPropertyValue("--hdr-h")).toBe("72px"); // re-measured, not the stale 8px + }); }); diff --git a/frontend/src/components/PageShell.tsx b/frontend/src/components/PageShell.tsx index 98d302a7..3e58fe5f 100644 --- a/frontend/src/components/PageShell.tsx +++ b/frontend/src/components/PageShell.tsx @@ -4,7 +4,6 @@ import { isValidElement, useContext, useLayoutEffect, - useRef, useState, type ReactElement, type ReactNode, @@ -65,35 +64,42 @@ export default function PageShell({ const [toolbarSlot, setToolbarSlot] = useState(null); // The floating header pads the content below it by `--hdr-h`. That must equal the header's REAL // height, which grows as the flex-wrap header wraps to 2–3 rows on a narrow window — a fixed pad - // would let the content toolbar slide up under it (E5 S7). Measure the header (a ref forwarded onto - // its root box) and write its height straight onto the content wrapper that consumes `--hdr-h`, so - // the two live together with no assumption about the header's DOM parent. - const headerRef = useRef(null); - const contentRef = useRef(null); + // would let the content toolbar slide up under it (E5 S7). Measure the header and write its height + // straight onto the content wrapper that consumes `--hdr-h`. + // + // Both nodes are held in STATE (callback refs), NOT plain refs behind a one-shot `useLayoutEffect([])`: + // PageShell is reused — never remounted — across feed↔channel navigation (same component position in + // App.tsx), so the header DOM node is swapped out from under a stable PageShell. A one-shot effect keeps + // observing the FIRST header node; when that node detaches on channel-open, its ResizeObserver fires with + // a 0 box and rewrites --hdr-h to `0+0+8`=8px on the (still-mounted) content div. On Back the pad class + // returns with that stale 8px → the content toolbar slides up under the header, stuck until reload. Keying + // the measurement to the live node re-runs it every time the header mounts/unmounts, so it stays correct. + const [headerEl, setHeaderEl] = useState(null); + const [contentEl, setContentEl] = useState(null); useLayoutEffect(() => { - const h = headerRef.current; - const c = contentRef.current; - if (!h || !c) return; + if (!headerEl || !contentEl) { + // No header (channel page) → drop any stale pad so a leftover value can't linger. + contentEl?.style.removeProperty("--hdr-h"); + return; + } // `+ 8` = the header's own gap-y-2 row gap (0.5rem), reused as the gap below it. - const apply = () => c.style.setProperty("--hdr-h", `${h.offsetTop + h.offsetHeight + 8}px`); + const apply = () => + contentEl.style.setProperty("--hdr-h", `${headerEl.offsetTop + headerEl.offsetHeight + 8}px`); apply(); const ro = new ResizeObserver(apply); - ro.observe(h); - return () => { - ro.disconnect(); - c.style.removeProperty("--hdr-h"); - }; - }, []); + ro.observe(headerEl); + return () => ro.disconnect(); + }, [headerEl, contentEl]); const measuredHeader = isValidElement(header) ? cloneElement(header as ReactElement<{ ref?: Ref }>, { - ref: headerRef, + ref: setHeaderEl, }) : header; return ( {measuredHeader}
{banners} From ad0cf5827be8b38bbc306b96fe493de6f7295409 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 21 Aug 2026 17:01:41 +0200 Subject: [PATCH 02/17] feat(feed): use the Combobox picker for Channel/Your tags/Topic/Language filters Replaces the hand-rolled channel search list and the flat tag chip walls with the searchable, ARIA-correct Combobox built for the Plex filter bar (E5), so the YT feed filters get the same input+listbox+chips UX with facet counts. - ChannelPicker now wraps Combobox (selectedLabels keeps picks named); drops its bespoke input + scroll-fade list. - Sidebar language/topic/tags: each Combobox owns only its slice of the shared filters.tags list (setTagSelection replaces that list's ids, leaves the rest). Topic keeps its any/all match toggle, Your tags keeps Manage. TagChip removed. --- frontend/src/components/ChannelPicker.tsx | 92 +++------------- frontend/src/components/Sidebar.tsx | 123 +++++++--------------- 2 files changed, 55 insertions(+), 160 deletions(-) diff --git a/frontend/src/components/ChannelPicker.tsx b/frontend/src/components/ChannelPicker.tsx index e09f037b..3c4386de 100644 --- a/frontend/src/components/ChannelPicker.tsx +++ b/frontend/src/components/ChannelPicker.tsx @@ -1,11 +1,8 @@ -import { useState } from "react"; import { useTranslation } from "react-i18next"; import { useQuery } from "@tanstack/react-query"; import { qk } from "../lib/queryKeys"; -import { X } from "lucide-react"; import { api, type FeedFilters } from "../lib/api"; -import { useScrollFade } from "../lib/useScrollFade"; -import { inputCls } from "./ui/form"; +import Combobox, { type ComboboxOption } from "./Combobox"; // Pick channels to narrow the feed to — several OR together, so every other filter then applies // across all of them. Until this existed the filter was settable only via a shared "?channel=" link: @@ -22,86 +19,29 @@ export default function ChannelPicker({ setFilters: (f: FeedFilters) => void; }) { const { t } = useTranslation(); - const [query, setQuery] = useState(""); - // Same edge-fade affordance the nav rail and the panel body use to hint at more content. - const fade = useScrollFade(); const channelsQuery = useQuery({ queryKey: qk.channels(), queryFn: api.channels, staleTime: 5 * 60_000, }); - const q = query.trim().toLowerCase(); const all = channelsQuery.data ?? []; - const matches = q ? all.filter((c) => (c.title ?? c.id).toLowerCase().includes(q)) : all; - const picked = filters.channelIds; - const nameOf = (id: string) => all.find((c) => c.id === id)?.title ?? id; - const toggle = (id: string) => - setFilters({ - ...filters, - channelIds: picked.includes(id) ? picked.filter((x) => x !== id) : [...picked, id], - }); + // The list order isn't alphabetical — it's the user's channel priority — and Combobox preserves the + // options order, so it stays meaningful. selectedLabels keeps a picked channel named even if a later + // narrowing ever drops it from the visible rows. + const options: ComboboxOption[] = all.map((c) => ({ value: c.id, label: c.title ?? c.id })); + const selectedLabels = Object.fromEntries(all.map((c) => [c.id, c.title ?? c.id])); return ( - <> - {/* The picks sit above the list so they stay visible once the list scrolls away. */} - {picked.length > 0 && ( -
- {picked.map((id) => ( - - ))} -
- )} - setQuery(e.target.value)} - placeholder={t("sidebar.channelSearch")} - aria-label={t("sidebar.channelSearch")} - className={inputCls} - /> - {/* The order isn't arbitrary and isn't alphabetical — say so, or the top of the list looks - random to anyone who hasn't set a priority. Only while unfiltered: a search reorders nothing - but does drop the rows that explain it. */} - {!q && all.length > 0 && ( -
{t("sidebar.channelOrder")}
- )} -
- {matches.length === 0 ? ( -
- {channelsQuery.isLoading ? t("sidebar.loading") : t("sidebar.noChannelMatch")} -
- ) : ( - matches.map((c) => { - const on = picked.includes(c.id); - return ( - - ); - }) - )} -
- + setFilters({ ...filters, channelIds })} + loading={channelsQuery.isLoading} + emptyText={t("sidebar.noChannelMatch")} + selectedLabels={selectedLabels} + /> ); } diff --git a/frontend/src/components/Sidebar.tsx b/frontend/src/components/Sidebar.tsx index d934834a..63a97252 100644 --- a/frontend/src/components/Sidebar.tsx +++ b/frontend/src/components/Sidebar.tsx @@ -15,7 +15,8 @@ import { type PanelItem } from "./PanelGroups"; import { type MediaSourceKind } from "../lib/mediaSource"; import ChannelPicker from "./ChannelPicker"; import SegmentedToggle from "./SegmentedToggle"; -import FilterChipGroup, { filterChipClass } from "./FilterChipGroup"; +import FilterChipGroup from "./FilterChipGroup"; +import Combobox, { type ComboboxOption } from "./Combobox"; import { useFeedFilters, useFeedFiltersActions } from "./FeedFiltersProvider"; import { useChannelsActions } from "./ChannelsProvider"; import { useMe } from "../lib/useMe"; @@ -29,39 +30,6 @@ const DATE_PRESETS: { days: number; key: string }[] = [ { days: 365, key: "1year" }, ]; -function TagChip({ - tag, - count, - active, - onClick, -}: { - tag: Tag; - count: number; - active: boolean; - onClick: () => void; -}) { - const { t } = useTranslation(); - return ( - - ); -} - export default function Sidebar({ layout, setLayout, @@ -113,11 +81,20 @@ export default function Sidebar({ // the panel layout key. `FilterRail` owns the customize `editing` state now. const source: MediaSourceKind = "feed"; - function toggleTag(id: number) { - const has = filters.tags.includes(id); + // The language/topic/tags sections all write into the one `filters.tags` list, so each combobox owns + // only its own slice: its selection replaces the ids that belong to its list, leaving the others. + const tagOption = (tg: Tag): ComboboxOption => ({ + value: String(tg.id), + label: tg.name, + count: chipCount(tg), + }); + const selectedIn = (list: Tag[]): string[] => + filters.tags.filter((id) => list.some((tg) => tg.id === id)).map(String); + function setTagSelection(list: Tag[], next: string[]) { + const inList = new Set(list.map((tg) => tg.id)); setFilters({ ...filters, - tags: has ? filters.tags.filter((t) => t !== id) : [...filters.tags, id], + tags: [...filters.tags.filter((id) => !inList.has(id)), ...next.map(Number)], }); } @@ -216,28 +193,19 @@ export default function Sidebar({ ); } case "language": { - const chips = visibleChips(languages); return ( -
- {chips.map((tg) => ( - toggleTag(tg.id)} - /> - ))} - {facetsReady && chips.length === 0 && ( - {t("sidebar.noMatchingTags")} - )} -
+ setTagSelection(languages, next)} + emptyText={t("sidebar.noMatchingTags")} + /> ); } case "topic": { - const chips = visibleChips(topics); return ( - <> +
setFilters({ ...filters, tagMode })} /> -
- {chips.map((tg) => ( - toggleTag(tg.id)} - /> - ))} - {facetsReady && chips.length === 0 && ( - {t("sidebar.noMatchingTags")} - )} -
- + setTagSelection(topics, next)} + emptyText={t("sidebar.noMatchingTags")} + /> +
); } case "tags": { - const tagChips = visibleChips(userTags); return ( -
- {tagChips.map((tg) => ( - toggleTag(tg.id)} - /> - ))} - {facetsReady && tagChips.length === 0 && ( - {t("sidebar.noMatchingTags")} - )} +
+ setTagSelection(userTags, next)} + emptyText={t("sidebar.noMatchingTags")} + /> - ); - })} -
{/* The Source filter (organic / include-search / search-only) is a global-catalog concept — it selects how videos ENTERED the library. On a channel-scoped view we already show all of this one channel's videos (librarySource is pinned to "all"), so the selector would diff --git a/frontend/src/components/FilterSelect.dom.test.tsx b/frontend/src/components/FilterSelect.dom.test.tsx new file mode 100644 index 00000000..4b6435c5 --- /dev/null +++ b/frontend/src/components/FilterSelect.dom.test.tsx @@ -0,0 +1,92 @@ +// @vitest-environment jsdom +// +// Contract of the labeled-pill FilterSelect. Labels are plain props (no i18n). Built on the floating-ui +// AnchoredPopover platform, so we stub the two observers autoUpdate wires and wrap in a FloatingTree. +// floating-ui never positions in jsdom → the surface stays visibility:hidden, so option/listbox role +// queries pass `{ hidden: true }`. +import { describe, it, expect, vi, afterEach, beforeAll } from "vitest"; +import { render, screen, fireEvent, cleanup } from "@testing-library/react"; +import { FloatingTree } from "@floating-ui/react"; +import FilterSelect, { type FilterSelectOption } from "./FilterSelect"; + +beforeAll(() => { + class Stub { + observe() {} + unobserve() {} + disconnect() {} + takeRecords() { + return []; + } + } + globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver; + globalThis.IntersectionObserver ??= Stub as unknown as typeof IntersectionObserver; +}); + +afterEach(cleanup); + +const STATUS: FilterSelectOption[] = [ + { value: "all", label: "All" }, + { value: "new", label: "New" }, +]; +const TYPES: FilterSelectOption[] = [ + { value: "n", label: "Normal" }, + { value: "s", label: "Shorts" }, +]; +const tree = (ui: React.ReactNode) => render({ui}); + +describe("FilterSelect", () => { + it("renders a labeled trigger `Label: value` and keeps options unmounted until opened", () => { + tree(); + const trigger = screen.getByRole("button", { name: "Status" }); + expect(trigger.textContent).toContain("Status:"); + expect(trigger.textContent).toContain("All"); + expect(screen.queryByRole("option", { hidden: true })).toBeNull(); + }); + + it("single: picking a row replaces the selection and closes", () => { + const onChange = vi.fn(); + tree(); + fireEvent.click(screen.getByRole("button", { name: "Status" })); + expect(screen.getAllByRole("option", { hidden: true }).map((o) => o.textContent)).toEqual([ + "All", + "New", + ]); + fireEvent.click(screen.getAllByRole("option", { hidden: true })[1]!); // New + expect(onChange).toHaveBeenCalledWith(["new"]); + expect(screen.queryByRole("option", { hidden: true })).toBeNull(); + }); + + it("multiple: toggling a row adds it and keeps the panel open", () => { + const onChange = vi.fn(); + tree( + , + ); + fireEvent.click(screen.getByRole("button", { name: "Type" })); + fireEvent.click(screen.getAllByRole("option", { hidden: true })[1]!); // Shorts + expect(onChange).toHaveBeenCalledWith(["n", "s"]); + expect(screen.queryAllByRole("option", { hidden: true }).length).toBeGreaterThan(0); // still open + }); + + it("multiple + requireOne: the last remaining selection can't be turned off", () => { + const onChange = vi.fn(); + tree( + , + ); + fireEvent.click(screen.getByRole("button", { name: "Type" })); + fireEvent.click(screen.getAllByRole("option", { hidden: true })[0]!); // Normal — the only one selected + expect(onChange).not.toHaveBeenCalled(); + }); + + it("multiple: the trigger summarises extra picks as `First +N`", () => { + tree( + , + ); + expect(screen.getByRole("button", { name: "Type" }).textContent).toContain("Normal +1"); + }); +}); diff --git a/frontend/src/components/FilterSelect.tsx b/frontend/src/components/FilterSelect.tsx new file mode 100644 index 00000000..3b1f39fc --- /dev/null +++ b/frontend/src/components/FilterSelect.tsx @@ -0,0 +1,135 @@ +import { useState } from "react"; +import { ChevronDown, Check } from "lucide-react"; +import { PopoverItem, PopoverSurface, useAnchoredPopover } from "./AnchoredPopover"; + +// A compact "labeled pill → dropdown" filter control: the trigger reads `Label: value ▾`, and clicking +// it opens a listbox of options (facet counts optional). It replaces a wall of individual toolbar chips +// with one pill per filter group, in two flavours: +// - single (default): picking a row replaces the choice and closes — always exactly one active. +// - multiple: rows toggle independently and the panel stays open; `requireOne` blocks clearing the last. +// Built on the same AnchoredPopover platform as ActionMenu/Combobox (arrows/typeahead/Escape/anchor-flip); +// reusable anywhere a small, known set of options currently renders as a chip group. See FilterSelect.dom.test. + +export interface FilterSelectOption { + value: string; + label: string; + /** A facet count shown at the row's end. */ + count?: number; + disabled?: boolean; +} + +export interface FilterSelectProps { + /** Visible group name shown as the trigger prefix (`Label: value`) and the accessible name. */ + label: string; + options: readonly FilterSelectOption[]; + selected: string[]; + onChange: (next: string[]) => void; + /** Toggle a subset (default false = pick exactly one). */ + multiple?: boolean; + /** Multi only: keep at least one selected — the last active row can't be turned off. Default true. */ + requireOne?: boolean; + /** Test hook: the trigger gets `data-testid={testId}`; each option gets `${testId}-${value}`. */ + testId?: string; +} + +export default function FilterSelect({ + label, + options, + selected, + onChange, + multiple = false, + requireOne = true, + testId, +}: FilterSelectProps) { + const [open, setOpen] = useState(false); + const pop = useAnchoredPopover({ + open, + onOpenChange: setOpen, + mode: "listbox", + placement: "bottom-start", + }); + + const labelOf = (v: string) => options.find((o) => o.value === v)?.label ?? v; + // Summarise the trigger value. Multi lists the first chosen (in options order, so it's stable) + "+N". + const chosen = options.filter((o) => selected.includes(o.value)); + const first = chosen[0]; + const summary = multiple + ? first + ? `${first.label}${chosen.length > 1 ? ` +${chosen.length - 1}` : ""}` + : "—" + : labelOf(selected[0] ?? options[0]?.value ?? ""); + + const commit = (value: string) => { + if (!multiple) { + onChange([value]); + setOpen(false); + return; + } + const has = selected.includes(value); + if (has) { + if (requireOne && selected.length === 1) return; // don't let the last one go + onChange(selected.filter((x) => x !== value)); + } else { + onChange([...selected, value]); + } + }; + + return ( + <> + + + + {options.map((o) => { + const on = selected.includes(o.value); + return ( + commit(o.value)} + label={o.label} + data-testid={testId ? `${testId}-${o.value}` : undefined} + className={`flex w-full items-center gap-2 rounded-md px-2 py-1.5 text-left text-sm transition ${ + on ? "text-fg" : "text-muted" + }`} + > + {multiple ? ( + + {on && } + + ) : ( + + )} + {o.label} + {o.count != null && ( + {o.count} + )} + + ); + })} + + + ); +} From 6109e0c6f10a9e21ab9bc7abfc32b923dcfb76d8 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 21 Aug 2026 17:36:33 +0200 Subject: [PATCH 04/17] feat(feed): convert the Source filter to the FilterSelect dropdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the native - setFilters({ - ...filters, - librarySource: e.target.value as FeedFilters["librarySource"], - }) - } - title={t("feed.source.tip")} - className="bg-surface border border-border rounded-lg px-2 py-1 text-xs focus:outline-hidden focus:ring-2 focus:ring-accent/40" - > - - - - - + + setFilters({ + ...filters, + librarySource: next[0] as FeedFilters["librarySource"], + }) + } + /> )}
- {t("feed.sortLabel")} {/* Sort moved out of the filter sidebar to the row above the cards (S3), like the Feed. */}
- {t("plex.filter.sort")} { + class Stub { + observe() {} + unobserve() {} + disconnect() {} + takeRecords() { + return []; + } + } + globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver; + globalThis.IntersectionObserver ??= Stub as unknown as typeof IntersectionObserver; +}); + const OPTS: SortOption[] = [ { key: "date", label: "Date", defaultDir: "desc" }, { key: "title", label: "Title" }, @@ -16,25 +32,30 @@ const OPTS: SortOption[] = [ function renderControl(sort: SortState, extra: Record = {}) { const onChange = vi.fn(); render( - , + + + , ); return { onChange }; } +const openPanel = () => fireEvent.click(screen.getByRole("button", { name: "Sort" })); + afterEach(cleanup); describe("SortControl", () => { - it("renders one option per SortOption, no empty option without defaultLabel", () => { + it("renders one option per SortOption, no no-sort row without defaultLabel", () => { renderControl({ key: "title", dir: "asc" }); - expect(screen.getAllByRole("option").map((o) => o.textContent)).toEqual([ + openPanel(); + expect(screen.getAllByRole("option", { hidden: true }).map((o) => o.textContent)).toEqual([ "Date", "Title", "Shuffle", @@ -43,40 +64,44 @@ describe("SortControl", () => { it("adds a leading no-sort option when defaultLabel is set (tables)", () => { renderControl(null, { defaultLabel: "None" }); - const none = screen.getByRole("option", { name: "None" }) as HTMLOptionElement; - expect(none.value).toBe(""); + openPanel(); + expect(screen.getAllByRole("option", { hidden: true }).map((o) => o.textContent)).toEqual([ + "None", + "Date", + "Title", + "Shuffle", + ]); }); - it("falls back to the first option (not a blank select) for an unavailable key without defaultLabel", () => { + it("summarises the trigger with the first option (not blank) for an unavailable key without defaultLabel", () => { renderControl({ key: "ghost", dir: "asc" }); - expect((screen.getByRole("combobox") as HTMLSelectElement).value).toBe("date"); + expect(screen.getByRole("button", { name: "Sort" }).textContent).toContain("Date"); }); it("picking a new option fires onChange at that option's defaultDir", () => { const { onChange } = renderControl({ key: "title", dir: "asc" }); - fireEvent.change(screen.getByRole("combobox"), { target: { value: "date" } }); + openPanel(); + fireEvent.click(screen.getAllByRole("option", { hidden: true })[0]!); // Date expect(onChange).toHaveBeenCalledWith({ key: "date", dir: "desc" }); }); - it("shows the direction toggle for a directional sort and flips it on click", () => { + it("shows the direction segment for a directional sort and sets asc on click", () => { const { onChange } = renderControl({ key: "date", dir: "desc" }); - const toggle = screen.getByRole("button", { name: "Descending" }); - fireEvent.click(toggle); + openPanel(); + fireEvent.click(screen.getByText("Ascending")); expect(onChange).toHaveBeenCalledWith({ key: "date", dir: "asc" }); }); - it("hides the direction toggle for a directionless sort", () => { + it("hides the direction segment for a directionless sort", () => { renderControl({ key: "shuffle", dir: "desc" }); - expect(screen.queryByRole("button")).toBeNull(); + openPanel(); + expect(screen.queryByText("Ascending")).toBeNull(); + expect(screen.queryByText("Descending")).toBeNull(); }); - it("renders the trailing adornment", () => { - renderControl( - { key: "shuffle", dir: "desc" }, - { - trailing: , - }, - ); - expect(screen.getByRole("button", { name: "Reshuffle" })).toBeTruthy(); + it("renders the trailing adornment in the footer", () => { + renderControl({ key: "shuffle", dir: "desc" }, { trailing: }); + openPanel(); + expect(screen.getByText("Reshuffle")).toBeTruthy(); }); }); diff --git a/frontend/src/components/SortControl.tsx b/frontend/src/components/SortControl.tsx index cf3be5c6..19d0d374 100644 --- a/frontend/src/components/SortControl.tsx +++ b/frontend/src/components/SortControl.tsx @@ -1,13 +1,19 @@ -import { type ReactNode } from "react"; -import { ArrowDown, ArrowUp } from "lucide-react"; +import { useState, type ReactNode } from "react"; +import { ArrowDown, ArrowUp, Check, ChevronDown } from "lucide-react"; import { pickSort, type SortOption, type SortState } from "../lib/columnSort"; +import { PopoverItem, PopoverSurface, useAnchoredPopover } from "./AnchoredPopover"; // The one sort control for every content surface (YT feed, Plex library, channel manager + its card -// view, channel discovery): a dropdown of `SortOption`s + an asc/desc toggle, driven by the shared -// `SortState`. Presentation-agnostic to its container — a table feeds column-derived options via -// `ColumnSortControl`, the feed/Plex declare their own vocabulary. `defaultLabel` opts a view into a -// "no sort" choice (tables); omit it for always-sorted views (feed/Plex). `trailing` is an adornment -// slot (the feed's reshuffle button) so the shuffle affordance stays out of the generic control. +// view, channel discovery, playlists): a labeled dropdown pill matching FilterSelect — `Label: key ↓ ▾` +// — that opens a listbox of `SortOption`s with an asc/desc segment in the panel footer. Two-dimensional +// (key + direction) unlike FilterSelect, so it stays its own component. Driven by the shared `SortState`. +// `defaultLabel` opts a view into a "no sort" choice (tables); omit it for always-sorted views +// (feed/Plex). `trailing` is an adornment slot (the feed's reshuffle, playlists' dirty-first toggle) +// rendered in the footer beside the direction segment. +// +// Direction lives in the footer, not on the trigger, on purpose: the trigger stays a single plain +// - )} - {trailing} -
+