From e644b4f7d7c76079ac207cad3807fd73b5ce8eeb Mon Sep 17 00:00:00 2001 From: npeter83 Date: Tue, 28 Jul 2026 23:49:44 +0200 Subject: [PATCH 01/18] feat(popover): add the Floating UI anchored-popover platform + migrate TagsCell pilot (R8 S1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce useAnchoredPopover + PopoverSurface over @floating-ui/react — one primitive replacing the hand-rolled measured engines: flip() (two-way flip), shift() (width clamp), hide() (hide-and-follow when the trigger leaves its scroller), autoUpdate (scroll follow), FloatingPortal (escape ancestor stacking contexts). Supports menu + hover modes; focus management lands in S2. Wrap the app and the public watch page in so Escape / outside-press dispatch is topmost-only across nested layers (S3 folds in the Modal + players). Migrate the richest engine, TagsCell, as the pilot — ~140 lines of bespoke positioning (coords state, flip layout-effect, scroll rAF loop, useDismiss, Overlay) collapse into one hook call. Verified in-app: click opens, portals to , positions below the trigger, role=menu + aria-expanded/haspopup, Escape and outside-click close, no console errors. --- frontend/package-lock.json | 60 +++++++ frontend/package.json | 1 + frontend/src/components/AnchoredPopover.tsx | 174 ++++++++++++++++++ frontend/src/components/Channels.tsx | 186 +++++--------------- frontend/src/main.tsx | 48 ++--- 5 files changed, 307 insertions(+), 162 deletions(-) create mode 100644 frontend/src/components/AnchoredPopover.tsx diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 301f023b..18c044c1 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -9,6 +9,7 @@ "@dnd-kit/core": "^6.3.1", "@dnd-kit/sortable": "^10.0.0", "@dnd-kit/utilities": "^3.2.2", + "@floating-ui/react": "^0.27.20", "@tanstack/react-query": "^5.51.0", "@tanstack/react-virtual": "^3.14.3", "clsx": "^2.1.1", @@ -995,6 +996,59 @@ "node": "^18.18.0 || ^20.9.0 || >=21.1.0" } }, + "node_modules/@floating-ui/core": { + "version": "1.8.0", + "resolved": "https://registry.npmjs.org/@floating-ui/core/-/core-1.8.0.tgz", + "integrity": "sha512-0CIZ5itps/8x7BG8dEIhs53BvCUH2PCoogtakwRTut+Arm58sJooJ0AuZhLw2HJYIR5cMLNPBSS728sPho2khQ==", + "license": "MIT", + "dependencies": { + "@floating-ui/utils": "^0.2.12" + } + }, + "node_modules/@floating-ui/dom": { + "version": "1.8.0", + "resolved": "https://registry.npmjs.org/@floating-ui/dom/-/dom-1.8.0.tgz", + "integrity": "sha512-yXSrzeHZBTZadLOlfyhCkJHNeLJnHRnRInwdZ40L7ZiaAtrBwoYlsDrX3v5zB1Utk7CLfzcOVnVVWoXEky7Ceg==", + "license": "MIT", + "dependencies": { + "@floating-ui/core": "^1.8.0", + "@floating-ui/utils": "^0.2.12" + } + }, + "node_modules/@floating-ui/react": { + "version": "0.27.20", + "resolved": "https://registry.npmjs.org/@floating-ui/react/-/react-0.27.20.tgz", + "integrity": "sha512-CMqMy7OaXl9W0eq1Uy7L7i2Y/anPvHmFmESd2CEw0t5YvZhcVCeo4MBevAmswRllX7Y2dEidA4ozGPunLSTQpw==", + "license": "MIT", + "dependencies": { + "@floating-ui/react-dom": "^2.1.9", + "@floating-ui/utils": "^0.2.12", + "tabbable": "^6.0.0" + }, + "peerDependencies": { + "react": ">=17.0.0", + "react-dom": ">=17.0.0" + } + }, + "node_modules/@floating-ui/react-dom": { + "version": "2.1.9", + "resolved": "https://registry.npmjs.org/@floating-ui/react-dom/-/react-dom-2.1.9.tgz", + "integrity": "sha512-JDjEFGCpImxDCA7JJKviA0M9+RtmJdj0m/NVU5IMgBK+AmZouAQQ7/+2GLH0GXXY0YMw9oXPB8hKdbPYg5QLYg==", + "license": "MIT", + "dependencies": { + "@floating-ui/dom": "^1.8.0" + }, + "peerDependencies": { + "react": ">=16.8.0", + "react-dom": ">=16.8.0" + } + }, + "node_modules/@floating-ui/utils": { + "version": "0.2.12", + "resolved": "https://registry.npmjs.org/@floating-ui/utils/-/utils-0.2.12.tgz", + "integrity": "sha512-HpCo8tmWzLVad5s2d19EhAz5zqrrQ6s69qd6moPMQvkOuSwDT1YgRfWSVuc4ennqrgv3OHppiOGMQ7oC13yIww==", + "license": "MIT" + }, "node_modules/@humanfs/core": { "version": "0.19.2", "resolved": "https://registry.npmjs.org/@humanfs/core/-/core-0.19.2.tgz", @@ -5294,6 +5348,12 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/tabbable": { + "version": "6.5.0", + "resolved": "https://registry.npmjs.org/tabbable/-/tabbable-6.5.0.tgz", + "integrity": "sha512-wieBHXygIm7OyQOu5hQlkk62/WyCFYGlWg7L6/ZCUZwx0o398Zkn4pVmMyfYhfMG8kGrj/Krt8eIk6UKC6VzwA==", + "license": "MIT" + }, "node_modules/tailwindcss": { "version": "3.4.19", "resolved": "https://registry.npmjs.org/tailwindcss/-/tailwindcss-3.4.19.tgz", diff --git a/frontend/package.json b/frontend/package.json index f29efae6..dfa964a5 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -22,6 +22,7 @@ "@dnd-kit/core": "^6.3.1", "@dnd-kit/sortable": "^10.0.0", "@dnd-kit/utilities": "^3.2.2", + "@floating-ui/react": "^0.27.20", "@tanstack/react-query": "^5.51.0", "@tanstack/react-virtual": "^3.14.3", "clsx": "^2.1.1", diff --git a/frontend/src/components/AnchoredPopover.tsx b/frontend/src/components/AnchoredPopover.tsx new file mode 100644 index 00000000..3422f9f7 --- /dev/null +++ b/frontend/src/components/AnchoredPopover.tsx @@ -0,0 +1,174 @@ +import { type CSSProperties, type ReactNode, type Ref } from "react"; +import { + useFloating, + autoUpdate, + offset as offsetMiddleware, + flip, + shift, + size, + hide, + useClick, + useHover, + useFocus, + useDismiss, + useRole, + useInteractions, + useMergeRefs, + useFloatingNodeId, + safePolygon, + FloatingNode, + FloatingPortal, + FloatingFocusManager, + type Placement, + type UseRoleProps, +} from "@floating-ui/react"; + +// R8 popover platform. ONE anchored-popover primitive over @floating-ui/react, replacing the ~9 +// hand-rolled positioning engines (measured flip / width clamp / rAF scroll-follow / scroller-relative +// visibility) that had each re-derived the same maths. Floating UI's middleware does all four: +// - flip() → measured two-way flip (top/bottom) when a side runs out of room +// - shift() → slides back into view horizontally (the old MEASURED-width clamp) +// - hide() → sets referenceHidden when the trigger is scrolled out of its clipping scroller, so we +// hide-and-follow instead of the old "close on leave scroller" (auto-reappears on scroll back) +// - autoUpdate → follows the trigger on scroll/resize (the old rAF loop), coalesced by the library +// Positioning is portalled through FloatingPortal (escapes ancestor stacking contexts — same job the +// house did for TagsCell). Every popover is a node in the app's so Escape / +// outside-press dispatch is topmost-only across nested layers (S3 folds the Modal + players in too). +// Focus management (trap + return-to-trigger, roving nav) arrives in S2 via `focusManaged`. + +// Internal to the platform; call sites pass the string literal via `mode`, they don't name the type. +type PopoverMode = "menu" | "listbox" | "hover" | "dialog"; + +export interface AnchoredPopoverOptions { + open: boolean; + onOpenChange: (open: boolean, event?: Event) => void; + /** "menu"/"listbox"/"dialog" open on click; "hover" opens on hover+focus (tooltips, hover cards). */ + mode?: PopoverMode; + placement?: Placement; + /** Gap between trigger and panel, px. Default 4. */ + offset?: number; + /** Hover mode only: open/close intent delays, ms. */ + hoverDelay?: number | { open?: number; close?: number }; + /** ARIA role for the panel; defaults to "tooltip" in hover mode, else "menu". */ + role?: UseRoleProps["role"]; + /** Cap the panel height to the available space (adds the `size` middleware). Off by default so a + * caller's own max-height class wins; turn on for long lists that should shrink to fit. */ + fitHeight?: boolean; +} + +export type AnchoredPopover = ReturnType; + +export function useAnchoredPopover(opts: AnchoredPopoverOptions) { + const mode = opts.mode ?? "menu"; + const isHover = mode === "hover"; + // null when not inside a — the hook still works, just without nested-layer awareness. + const nodeId = useFloatingNodeId(); + + const { refs, floatingStyles, context, middlewareData } = useFloating({ + nodeId, + open: opts.open, + onOpenChange: opts.onOpenChange, + placement: opts.placement ?? "bottom-start", + whileElementsMounted: autoUpdate, + middleware: [ + offsetMiddleware(opts.offset ?? 4), + flip({ padding: 8 }), + shift({ padding: 8 }), + ...(opts.fitHeight + ? [ + size({ + padding: 8, + apply({ availableHeight, elements }) { + elements.floating.style.maxHeight = `${availableHeight}px`; + }, + }), + ] + : []), + hide({ padding: 8 }), + ], + }); + + const click = useClick(context, { enabled: !isHover }); + const hover = useHover(context, { + enabled: isHover, + move: false, + delay: opts.hoverDelay ?? { open: 120, close: 140 }, + handleClose: safePolygon(), + }); + const focus = useFocus(context, { enabled: isHover }); + const dismiss = useDismiss(context); + const role = useRole(context, { role: opts.role ?? (isHover ? "tooltip" : "menu") }); + + const interactions = useInteractions([click, hover, focus, dismiss, role]); + + return { + nodeId, + mode, + open: opts.open, + refs, + floatingStyles, + context, + referenceHidden: middlewareData.hide?.referenceHidden ?? false, + ...interactions, + }; +} + +/** Renders the popover panel: the FloatingTree node + portal + the positioned surface. Merges the + * library's floating ref/props with the caller's className/style (and an optional extra ref, e.g. a + * scroll-fade callback ref). `focusManaged` wraps it in a FloatingFocusManager (S2). */ +export function PopoverSurface({ + popover, + className, + style, + children, + floatingRef, + focusManaged = false, + initialFocus, +}: { + popover: AnchoredPopover; + className?: string; + style?: CSSProperties; + children: ReactNode; + /** Extra ref merged onto the floating node (e.g. a useScrollFade callback ref). */ + floatingRef?: Ref; + focusManaged?: boolean; + initialFocus?: number | React.MutableRefObject; +}) { + const { context, refs, floatingStyles, getFloatingProps, nodeId, referenceHidden, open } = + popover; + const mergedRef = useMergeRefs([refs.setFloating, floatingRef ?? null]); + const floatingProps = getFloatingProps(); + + if (!open) return null; + + const surface = ( +
+ {children} +
+ ); + + return ( + + + {focusManaged ? ( + + {surface} + + ) : ( + surface + )} + + + ); +} diff --git a/frontend/src/components/Channels.tsx b/frontend/src/components/Channels.tsx index b8afee54..9bb41e8d 100644 --- a/frontend/src/components/Channels.tsx +++ b/frontend/src/components/Channels.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useLayoutEffect, useRef, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { useTranslation } from "react-i18next"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { qk } from "../lib/queryKeys"; @@ -21,7 +21,6 @@ import { import { api, type ManagedChannel, type Tag } from "../lib/api"; import { notifyYouTubeActionError } from "../lib/youtubeErrors"; import { accountKey, LS } from "../lib/storage"; -import { useDismiss } from "../lib/useDismiss"; import { useScrollFade } from "../lib/useScrollFade"; import { formatEta, formatTotalHours, relativeTime } from "../lib/format"; import { subsColumn } from "./channelColumns"; @@ -30,7 +29,7 @@ import Tooltip from "./Tooltip"; import DataTable, { type Column } from "./DataTable"; import ChannelLayoutGrid from "./ChannelLayoutGrid"; import { LoadingState, StateMessage } from "./QueryState"; -import Overlay from "./Overlay"; +import { useAnchoredPopover, PopoverSurface } from "./AnchoredPopover"; import ViewSwitcher, { type ViewOption } from "./ViewSwitcher"; import ColumnSortControl from "./ColumnSortControl"; import Pager from "./Pager"; @@ -1037,114 +1036,26 @@ function TagsCell({ }) { const { t } = useTranslation(); const [open, setOpen] = useState(false); - // Anchor for the portalled menu. It renders through (i.e. at ) rather than - // `absolute` inside the cell, because in the CARD layout every VirtualGrid row carries a - // `transform` — which makes it a stacking context and traps a z-indexed child, so the menu drew - // UNDER the next card. Same hazard, same blessed fix as the players in E3. - const [coords, setCoords] = useState<{ left: number; top?: number; bottom?: number } | null>( - null, - ); const fade = useScrollFade(); - const ref = useRef(null); - const popRef = useRef(null); - const btnRef = useRef(null); - // The menu is no longer a DOM descendant of `ref`, so dismiss has to watch both. - useDismiss(open, () => setOpen(false), [ref, popRef]); - // Keep the scroll-fade hook's callback ref working alongside our own. - const setPopRef = useCallback( - (node: HTMLDivElement | null) => { - popRef.current = node; - fade.ref(node); - }, - [fade.ref], - ); + // R8 anchored-popover platform: positioning (measured flip via flip(), horizontal clamp via shift(), + // scroll-follow via autoUpdate, hide-on-leave-scroller via hide()), body portal, dismiss and + // topmost Escape all come from the shared hook — replacing this cell's former hand-rolled engine. + // The portal is what lets the CARD layout's per-row `transform` (a stacking context) NOT trap the + // menu under the next card. Focus management (roving nav / return-to-trigger) arrives in S2. + const pop = useAnchoredPopover({ + open, + onOpenChange: setOpen, + mode: "menu", + placement: "bottom-start", + }); - // ONE placement rule, so the position the menu opens at and the one it keeps while scrolling can't - // drift apart. `flip` keeps it anchored above the trigger once it has been flipped; the horizontal - // clamp uses the MEASURED width (`w-44` is rem — it grows with the text-size slider, so a px guess - // reads short at 1.3). Before first paint the menu isn't measurable yet; the layout effect below - // re-runs this once it is. - const coordsFrom = useCallback((r: DOMRect, flip: boolean) => { - const w = popRef.current?.offsetWidth ?? 0; - const left = - w && r.left + w > window.innerWidth - 8 ? Math.max(8, window.innerWidth - 8 - w) : r.left; - return flip ? { left, bottom: window.innerHeight - r.top + 4 } : { left, top: r.bottom + 4 }; - }, []); - - const place = useCallback(() => { - const r = btnRef.current?.getBoundingClientRect(); - if (!r) return false; - setCoords(coordsFrom(r, false)); - return true; - }, [coordsFrom]); - const toggle = () => { - if (open) setOpen(false); - else if (place()) setOpen(true); - }; - - // Correct the placement against the MEASURED box: flip above when it would run off the bottom (and - // there is room above), and apply the width clamp now that the menu can actually be measured. - useLayoutEffect(() => { - const pop = popRef.current; - const r = btnRef.current?.getBoundingClientRect(); - if (!open || !pop || !coords || !r) return; - const h = pop.offsetHeight; // the width is read inside coordsFrom, off the same node - const fitsBelow = r.bottom + 4 + h <= window.innerHeight - 8; - const fitsAbove = r.top - 4 - h >= 8; - // Decide BOTH ways, every time. Prefer the side it is already on (no flapping while scrolling), - // but leave it as soon as that side stops fitting and the other one does — a one-way flip would - // keep an above-anchored menu above a row scrolling toward the top and push it off the screen. - const above = coords.bottom !== undefined; - const flip = above ? fitsAbove || !fitsBelow : !fitsBelow && fitsAbove; - const next = coordsFrom(r, flip); - if (next.left !== coords.left || next.top !== coords.top || next.bottom !== coords.bottom) { - setCoords(next); - } - }, [open, coords, coordsFrom]); - - // A fixed menu doesn't travel with its row, so FOLLOW the trigger on scroll rather than closing: - // in the table the menu used to scroll with the row, and closing on every wheel tick would make - // attaching several tags a reopen-per-tag chore. Closes only once the trigger leaves the SCROLLER - // (not the window — the page scroller starts below ~230px of fixed chrome, so a row hidden behind - // the toolbar is still "on screen" by window coords and the menu would float over the chrome). - // Scrolling INSIDE the menu must not move it. - useEffect(() => { - if (!open) return; - let raf = 0; - const reposition = () => { - raf = 0; - const r = btnRef.current?.getBoundingClientRect(); - const scroller = document.querySelector("main")?.getBoundingClientRect(); - const topEdge = scroller ? scroller.top : 0; - const bottomEdge = scroller ? scroller.bottom : window.innerHeight; - if (!r || r.bottom < topEdge || r.top > bottomEdge) { - setOpen(false); - return; - } - // Keep whichever side it is anchored to, so the layout effect has nothing left to correct — - // resetting to "below" every frame would make it flip back and forth, two renders per frame. - setCoords((cur) => (cur ? coordsFrom(r, cur.bottom !== undefined) : cur)); - }; - const onScroll = (e: Event) => { - if (popRef.current && e.target instanceof Node && popRef.current.contains(e.target)) return; - if (!raf) raf = requestAnimationFrame(reposition); // coalesce a scroll burst into one move - }; - const onResize = () => setOpen(false); - window.addEventListener("scroll", onScroll, true); - window.addEventListener("resize", onResize); - return () => { - if (raf) cancelAnimationFrame(raf); - window.removeEventListener("scroll", onScroll, true); - window.removeEventListener("resize", onResize); - }; - }, [open, coordsFrom]); if (userTags.length === 0) return null; // Show only the tags actually attached to this channel; the “+” opens a per-channel // picker to attach/detach (kept the convenient "kirakás" without listing every tag // on every row). Tag create/delete still lives in the top "YOUR TAGS" row. const attached = userTags.filter((tg) => c.tag_ids.includes(tg.id)); return ( -
+
{attached.map((tg) => ( - {open && coords && ( - -
- {userTags.map((tg) => { - const on = c.tag_ids.includes(tg.id); - return ( - - ); - })} -
- - )} + + {userTags.map((tg) => { + const on = c.tag_ids.includes(tg.id); + return ( + + ); + })} +
); } diff --git a/frontend/src/main.tsx b/frontend/src/main.tsx index 9fe4bca4..328b17bb 100644 --- a/frontend/src/main.tsx +++ b/frontend/src/main.tsx @@ -1,6 +1,7 @@ import React, { lazy, Suspense } from "react"; import { createRoot } from "react-dom/client"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { FloatingTree } from "@floating-ui/react"; import ErrorBoundary from "./components/ErrorBoundary"; import { ConfirmProvider } from "./components/ConfirmProvider"; import { NavigationProvider } from "./components/NavigationProvider"; @@ -43,29 +44,36 @@ const root = ) : path === "/terms" ? ( ) : path.startsWith("/watch/") ? ( - + // FloatingTree so the public player's own popovers dispatch Escape/outside-press topmost-only. + + + ) : ( - - - - - - - - - - - - - - - - - - - + {/* One tree for the whole app: every anchored popover registers as a node so Escape and + outside-press act only on the topmost layer (S3 folds the Modal + players in). */} + + + + + + + + + + + + + + + + + + + + + ); From cfa8be950d3be66f1096889b62e4ec79d63f48af Mon Sep 17 00:00:00 2001 From: npeter83 Date: Wed, 29 Jul 2026 00:40:33 +0200 Subject: [PATCH 02/18] feat(channels): fixed-width tag cell with "+N" overflow + inline create (R8 S1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Redesign the channel tag cell (the popover pilot's first real consumer) to fix two UAT-reported bugs and two UX gaps: - Jumping editor (bug 1): the "+" edit trigger now sits in a STABLE slot at the end, so toggling a tag (which grows the chip row) no longer moves the anchor — the editor popover stays put (verified dx/dy = 0). - Whole-table reflow (bug 2): the cell is width-bounded (max-w inside the cell, since table-layout:auto ignores the column width — the documented channel/About trap) and single-line, collapsing overflow into a hoverable "+N" pill. Rows no longer wrap and reflow (verified uniform row heights). - The "+" is ALWAYS present (even with zero tags), and its menu can CREATE a tag inline (type + Enter → create + attach) — tagging no longer requires the top-of-page dialog. New shared ChannelTagsControl (used by table + cards; the detail page is Part 2) over the R8 popover platform: a `menu` editor popover + a `hover` reveal-all popover. Greedy rem-safe fit via ResizeObserver. `api.createTag` now returns Promise. --- frontend/src/components/ChannelTags.tsx | 260 +++++++++++++++++++++ frontend/src/components/Channels.tsx | 115 ++------- frontend/src/i18n/locales/en/channels.json | 6 +- frontend/src/i18n/locales/hu/channels.json | 6 +- frontend/src/lib/api/tags.ts | 2 +- 5 files changed, 294 insertions(+), 95 deletions(-) create mode 100644 frontend/src/components/ChannelTags.tsx diff --git a/frontend/src/components/ChannelTags.tsx b/frontend/src/components/ChannelTags.tsx new file mode 100644 index 00000000..19d4c3d0 --- /dev/null +++ b/frontend/src/components/ChannelTags.tsx @@ -0,0 +1,260 @@ +import { useLayoutEffect, useRef, useState } from "react"; +import { Check, Plus } from "lucide-react"; +import { useTranslation } from "react-i18next"; +import { type Tag } from "../lib/api"; +import { useAnchoredPopover, PopoverSurface } from "./AnchoredPopover"; +import { useScrollFade } from "../lib/useScrollFade"; + +// The channel tag surface, shared by the channel manager (table + cards) and the channel detail +// page. Built on the R8 popover platform. Two design goals it fixes over the old inline TagsCell: +// 1. Stable edit anchor — the "+" edit trigger sits in a FIXED slot at the end, so toggling tags +// (which grows the chip row) never moves it, and the editor popover doesn't drift. +// 2. Fixed footprint — the chip row lives in a width-bounded, single-line, overflow-hidden box and +// collapses the excess into a "+N" pill (hover reveals the full set). The cell therefore never +// wraps to a 2nd line, so it can't widen its table column and reflow every other row. +// The "+" is ALWAYS present (even with zero tags), and its menu can CREATE a tag inline — tagging is +// no longer gated on the top-of-page "Manage tags" dialog. + +interface TagChipProps { + tag: Tag; + onClick?: () => void; + title?: string; +} + +function TagChip({ tag, onClick, title }: TagChipProps) { + const cls = + "shrink-0 text-[10px] px-1.5 py-0.5 rounded-full bg-accent text-accent-fg border border-accent max-w-[9rem] truncate"; + return onClick ? ( + + ) : ( + {tag.name} + ); +} + +/** The editor body: toggle the user's existing tags on/off for this channel, plus an inline + * "create + attach" input. Rendered inside an anchored `menu` popover. (Exported for the channel + * detail page in Part 2; internal for now.) */ +function TagEditorMenu({ + userTags, + attachedIds, + onToggle, + onCreate, +}: { + userTags: Tag[]; + attachedIds: number[]; + onToggle: (tagId: number) => void; + onCreate: (name: string) => void; +}) { + const { t } = useTranslation(); + const [draft, setDraft] = useState(""); + const attached = new Set(attachedIds); + const submit = () => { + const name = draft.trim(); + if (!name) return; + onCreate(name); + setDraft(""); + }; + return ( +
+
+ {userTags.map((tg) => { + const on = attached.has(tg.id); + return ( + + ); + })} + {userTags.length === 0 && ( +

{t("channels.tags.noneYet")}

+ )} +
+
+ setDraft(e.target.value)} + onKeyDown={(e) => { + if (e.key === "Enter") { + e.preventDefault(); + submit(); + } + }} + maxLength={64} + placeholder={t("channels.tags.createPlaceholder")} + className="flex-1 min-w-0 bg-transparent text-xs px-2 py-1 rounded-md border border-border focus:border-accent outline-none" + /> + +
+
+ ); +} + +/** The full tag control for a channel: a width-bounded chip row with "+N" overflow (hover reveals + * all), followed by a stable "+" trigger that opens the shared editor. Fills its container width, + * so a table cell bounds it via the column width and a card bounds it via the card. */ +export function ChannelTagsControl({ + userTags, + attachedIds, + onToggle, + onCreate, + onChipClick, +}: { + userTags: Tag[]; + attachedIds: number[]; + onToggle: (tagId: number) => void; + onCreate: (name: string) => void; + /** Manager only: clicking a chip filters the feed by that tag. Omit on the detail page. */ + onChipClick?: (tagId: number, name: string) => void; +}) { + const { t } = useTranslation(); + const attached = userTags.filter((tg) => attachedIds.includes(tg.id)); + + // Greedy single-line fit: start showing all, shrink one chip at a time until the row (including + // the "+N" pill it then needs) fits its bounded width. Re-expands and re-fits on width/scale + // change via ResizeObserver — measurement is off the real rem-rendered chips, not a px guess. + const rowRef = useRef(null); + const [maxWidth, setMaxWidth] = useState(0); + const [visible, setVisible] = useState(attached.length); + + // Track the bounded width so a font-scale/viewport change re-fits — measured off the real box, + // never a px guess (widths here are rem and grow with the text-size slider). + useLayoutEffect(() => { + const el = rowRef.current; + if (!el || typeof ResizeObserver === "undefined") return; + const ro = new ResizeObserver((entries) => { + const w = entries[0]?.contentRect.width; + if (w != null) setMaxWidth(w); + }); + ro.observe(el); + return () => ro.disconnect(); + }, []); + + // Reset to "show all" whenever the tag set or the available width changes… + useLayoutEffect(() => { + setVisible(attached.length); + }, [attached.length, maxWidth]); + + // …then drop one chip at a time until the row (with the "+N" pill it then needs) fits its width. + useLayoutEffect(() => { + const el = rowRef.current; + if (el && el.scrollWidth > el.clientWidth + 1 && visible > 0) setVisible((v) => v - 1); + }, [visible, maxWidth, attached.length]); + + const hidden = attached.length - visible; + + const [editorOpen, setEditorOpen] = useState(false); + const editPop = useAnchoredPopover({ + open: editorOpen, + onOpenChange: setEditorOpen, + mode: "menu", + placement: "bottom-start", + }); + + const [overflowOpen, setOverflowOpen] = useState(false); + const overflowPop = useAnchoredPopover({ + open: overflowOpen, + onOpenChange: setOverflowOpen, + mode: "hover", + placement: "bottom", + }); + const fade = useScrollFade(); + + return ( + // max-w caps the cell's MAX-CONTENT so `table-layout:auto` can't ignore the column width and let + // an over-tagged row stretch the whole table (the documented channel/About-column trap). The chip + // row then greedily fits within it and collapses the rest into "+N". +
+
+ {attached.slice(0, visible).map((tg) => ( + onChipClick(tg.id, tg.name) : undefined} + title={onChipClick ? t("channels.row.filterFeedByTag", { name: tg.name }) : undefined} + /> + ))} + {hidden > 0 && ( + + )} +
+ + + + {/* Hover: reveal every attached tag when some are collapsed into "+N". */} + + {attached.map((tg) => ( + onChipClick(tg.id, tg.name) : undefined} + title={onChipClick ? t("channels.row.filterFeedByTag", { name: tg.name }) : undefined} + /> + ))} + + + {/* Click: the shared editor (toggle existing + create inline). */} + + + +
+ ); +} diff --git a/frontend/src/components/Channels.tsx b/frontend/src/components/Channels.tsx index 9bb41e8d..b0138fe5 100644 --- a/frontend/src/components/Channels.tsx +++ b/frontend/src/components/Channels.tsx @@ -12,16 +12,14 @@ import { History, LayoutGrid, Pencil, - Plus, RefreshCw, Table, UserMinus, X, } from "lucide-react"; -import { api, type ManagedChannel, type Tag } from "../lib/api"; +import { api, type ManagedChannel } from "../lib/api"; import { notifyYouTubeActionError } from "../lib/youtubeErrors"; import { accountKey, LS } from "../lib/storage"; -import { useScrollFade } from "../lib/useScrollFade"; import { formatEta, formatTotalHours, relativeTime } from "../lib/format"; import { subsColumn } from "./channelColumns"; import { notify } from "../lib/notifications"; @@ -29,7 +27,7 @@ import Tooltip from "./Tooltip"; import DataTable, { type Column } from "./DataTable"; import ChannelLayoutGrid from "./ChannelLayoutGrid"; import { LoadingState, StateMessage } from "./QueryState"; -import { useAnchoredPopover, PopoverSurface } from "./AnchoredPopover"; +import { ChannelTagsControl } from "./ChannelTags"; import ViewSwitcher, { type ViewOption } from "./ViewSwitcher"; import ColumnSortControl from "./ColumnSortControl"; import Pager from "./Pager"; @@ -212,6 +210,19 @@ export default function Channels() { const call = hasTag ? api.detachChannelTag(id, tagId) : api.attachChannelTag(id, tagId); call.catch(() => qc.invalidateQueries({ queryKey: qk.channels() })); }; + + // Create a brand-new personal tag from the row/card editor and attach it to this channel in one + // step (so tagging no longer requires the top-of-page "Manage tags" dialog first). Refresh the tag + // catalog so the new tag appears everywhere; attach optimistically via toggleTag. + const createAndAttachTag = (channelId: string, name: string) => { + api + .createTag({ name }) + .then((tag) => { + qc.invalidateQueries({ queryKey: qk.tags() }); + toggleTag(channelId, tag.id, false); + }) + .catch(() => notify({ level: "error", message: t("channels.tags.createFailed") })); + }; const syncSubs = useMutation({ mutationFn: () => api.syncSubscriptions(), onSuccess: (r: { subscriptions?: number }) => { @@ -452,17 +463,21 @@ export default function Channels() { key: "tags", header: t("channels.cols.tags"), cardLabel: false, + // Bounded width so the cell can't widen the column and reflow the whole table; the control + // fills it and collapses overflow into "+N" (see ChannelTagsControl). + width: "15rem", filter: { kind: "multi", options: userTags.map((tg) => ({ value: String(tg.id), label: tg.name })), test: (c, values) => values.some((v) => c.tag_ids.includes(Number(v))), }, render: (c) => ( - toggleTag(c.id, tagId, c.tag_ids.includes(tagId))} - onTagClick={onFilterByTag} + attachedIds={c.tag_ids} + onToggle={(tagId) => toggleTag(c.id, tagId, c.tag_ids.includes(tagId))} + onCreate={(name) => createAndAttachTag(c.id, name)} + onChipClick={onFilterByTag} /> ), }, @@ -1023,90 +1038,6 @@ function SyncCell({ c }: { c: ManagedChannel }) { ); } -function TagsCell({ - c, - userTags, - onToggleTag, - onTagClick, -}: { - c: ManagedChannel; - userTags: Tag[]; - onToggleTag: (tagId: number) => void; - onTagClick: (tagId: number, name: string) => void; -}) { - const { t } = useTranslation(); - const [open, setOpen] = useState(false); - const fade = useScrollFade(); - // R8 anchored-popover platform: positioning (measured flip via flip(), horizontal clamp via shift(), - // scroll-follow via autoUpdate, hide-on-leave-scroller via hide()), body portal, dismiss and - // topmost Escape all come from the shared hook — replacing this cell's former hand-rolled engine. - // The portal is what lets the CARD layout's per-row `transform` (a stacking context) NOT trap the - // menu under the next card. Focus management (roving nav / return-to-trigger) arrives in S2. - const pop = useAnchoredPopover({ - open, - onOpenChange: setOpen, - mode: "menu", - placement: "bottom-start", - }); - - if (userTags.length === 0) return null; - // Show only the tags actually attached to this channel; the “+” opens a per-channel - // picker to attach/detach (kept the convenient "kirakás" without listing every tag - // on every row). Tag create/delete still lives in the top "YOUR TAGS" row. - const attached = userTags.filter((tg) => c.tag_ids.includes(tg.id)); - return ( -
- {attached.map((tg) => ( - - ))} - - - {userTags.map((tg) => { - const on = c.tag_ids.includes(tg.id); - return ( - - ); - })} - -
- ); -} - function ActionsCell({ c, isAdmin, diff --git a/frontend/src/i18n/locales/en/channels.json b/frontend/src/i18n/locales/en/channels.json index e9249120..756fdd3c 100644 --- a/frontend/src/i18n/locales/en/channels.json +++ b/frontend/src/i18n/locales/en/channels.json @@ -46,7 +46,11 @@ "tags": { "yourTags": "Your tags", "manage": "Manage tags", - "yourTagsHint": "Your personal labels. Attach them to channels below, then filter the feed by tag from the sidebar. (Separate from the automatic language/topic tags.)" + "yourTagsHint": "Your personal labels. Attach them to channels below, then filter the feed by tag from the sidebar. (Separate from the automatic language/topic tags.)", + "noneYet": "No tags yet — create one below.", + "create": "Create tag", + "createPlaceholder": "New tag…", + "createFailed": "Could not create the tag." }, "loading": "Loading channels…", "empty": "No channels.", diff --git a/frontend/src/i18n/locales/hu/channels.json b/frontend/src/i18n/locales/hu/channels.json index 55e3a113..4748812c 100644 --- a/frontend/src/i18n/locales/hu/channels.json +++ b/frontend/src/i18n/locales/hu/channels.json @@ -46,7 +46,11 @@ "tags": { "yourTags": "Címkéid", "manage": "Címkék kezelése", - "yourTagsHint": "Saját személyes címkéid. Csatold őket az alábbi csatornákhoz, majd szűrd a hírfolyamot címke szerint az oldalsávból. (Az automatikus nyelvi/téma címkéktől függetlenül.)" + "yourTagsHint": "Saját személyes címkéid. Csatold őket az alábbi csatornákhoz, majd szűrd a hírfolyamot címke szerint az oldalsávból. (Az automatikus nyelvi/téma címkéktől függetlenül.)", + "noneYet": "Még nincs címke — hozz létre egyet lent.", + "create": "Címke létrehozása", + "createPlaceholder": "Új címke…", + "createFailed": "A címke létrehozása nem sikerült." }, "loading": "Csatornák betöltése…", "empty": "Nincsenek csatornák.", diff --git a/frontend/src/lib/api/tags.ts b/frontend/src/lib/api/tags.ts index f9eb575d..afb17edd 100644 --- a/frontend/src/lib/api/tags.ts +++ b/frontend/src/lib/api/tags.ts @@ -12,7 +12,7 @@ export interface Tag { export const tagsApi = { tags: (): Promise => req("/api/tags"), - createTag: (t: { name: string; color?: string; category?: string }) => + createTag: (t: { name: string; color?: string; category?: string }): Promise => req("/api/tags", { method: "POST", body: JSON.stringify(t) }), updateTag: (id: number, patch: { name?: string; color?: string }) => req(`/api/tags/${id}`, { method: "PATCH", body: JSON.stringify(patch) }), From 0fd95c15a1c726f9dfa15678e06df116bd535090 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Wed, 29 Jul 2026 01:05:58 +0200 Subject: [PATCH 03/18] fix(channels): stable tag column width, no popover flash, no sync wrap (R8 S1 UAT) Three UAT refinements on the tag cell + popover platform: - Fixed tag-column width (w-[15rem], was max-w): under table-layout:auto a content- driven cell re-solved every column's width on each chip added, so the table visibly shifted (and the "+N" count flickered) until the cap was hit. A fixed footprint makes adding/removing tags move nothing (verified: column x unchanged, "+N" count stable). - No popover flash: gate the floating surface's visibility on Floating UI's isPositioned so it never paints at (0,0) top-left for a frame before the transform lands. - Sync cell no longer wraps: flex-nowrap so the recent/full badges stay on one line (the column grows to fit them instead of breaking to two rows). --- frontend/src/components/AnchoredPopover.tsx | 23 ++++++++++++++++----- frontend/src/components/ChannelTags.tsx | 10 +++++---- frontend/src/components/Channels.tsx | 2 +- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/frontend/src/components/AnchoredPopover.tsx b/frontend/src/components/AnchoredPopover.tsx index 3422f9f7..8f112d63 100644 --- a/frontend/src/components/AnchoredPopover.tsx +++ b/frontend/src/components/AnchoredPopover.tsx @@ -64,7 +64,7 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) { // null when not inside a — the hook still works, just without nested-layer awareness. const nodeId = useFloatingNodeId(); - const { refs, floatingStyles, context, middlewareData } = useFloating({ + const { refs, floatingStyles, context, middlewareData, isPositioned } = useFloating({ nodeId, open: opts.open, onOpenChange: opts.onOpenChange, @@ -108,6 +108,7 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) { refs, floatingStyles, context, + isPositioned, referenceHidden: middlewareData.hide?.referenceHidden ?? false, ...interactions, }; @@ -134,8 +135,16 @@ export function PopoverSurface({ focusManaged?: boolean; initialFocus?: number | React.MutableRefObject; }) { - const { context, refs, floatingStyles, getFloatingProps, nodeId, referenceHidden, open } = - popover; + const { + context, + refs, + floatingStyles, + getFloatingProps, + nodeId, + referenceHidden, + isPositioned, + open, + } = popover; const mergedRef = useMergeRefs([refs.setFloating, floatingRef ?? null]); const floatingProps = getFloatingProps(); @@ -149,8 +158,12 @@ export function PopoverSurface({ style={{ ...(floatingProps.style as CSSProperties | undefined), ...floatingStyles, - // Trigger scrolled out of its clipping scroller → hide-and-follow rather than float over chrome. - ...(referenceHidden ? { visibility: "hidden", pointerEvents: "none" } : null), + // Hide until (a) the first position is measured — else it flashes at top-left (0,0) before + // the transform lands — and (b) while the trigger is scrolled out of its clipping scroller + // (hide-and-follow rather than float over the chrome). + ...(!isPositioned || referenceHidden + ? { visibility: "hidden", pointerEvents: "none" } + : null), ...style, }} > diff --git a/frontend/src/components/ChannelTags.tsx b/frontend/src/components/ChannelTags.tsx index 19d4c3d0..cade20b0 100644 --- a/frontend/src/components/ChannelTags.tsx +++ b/frontend/src/components/ChannelTags.tsx @@ -190,10 +190,12 @@ export function ChannelTagsControl({ const fade = useScrollFade(); return ( - // max-w caps the cell's MAX-CONTENT so `table-layout:auto` can't ignore the column width and let - // an over-tagged row stretch the whole table (the documented channel/About-column trap). The chip - // row then greedily fits within it and collapses the rest into "+N". -
+ // FIXED width (not just max-w): under `table-layout:auto` a content-driven cell re-solves the + // whole table's column widths every time a chip is added — the table visibly shifts until the cap + // is hit. A fixed width makes the cell's footprint constant, so adding/removing tags never moves + // any column; the chip row greedily fits within it and collapses the rest into "+N". (max-w-full + // lets a narrower card shrink it.) +
{attached.slice(0, visible).map((tg) => ( +
Date: Wed, 29 Jul 2026 01:37:29 +0200 Subject: [PATCH 04/18] fix(channels): center table, kill popover top-left flash, exact "+N" count (R8 S1 UAT) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second UAT pass on the tag cell + popover platform: - Popover top-left flash (real root cause): the popIn keyframe animates `transform`, which overrode Floating UI's positioning `transform: translate(x,y)` for the animation's duration, parking the popover at (0,0). Move the animation to an INNER wrapper so the positioned element's transform stays clean (verified: menu is at the correct spot from frame 0). The isPositioned visibility gate is kept as a belt-and-suspenders. - Table shifted right / not centered: my previous fixed-15rem tags column + nowrap Sync pushed the table past the max-w-[96rem] wrapper so it overflowed. Trim to 12rem tags and shorten the long "full history queued/coming" sync labels to "queued"/"coming" so the table fits the wrapper (1594 <= 1628) and centers again. - "+N" count wrong/missing while adding tags: replace the iterative shrink (which lagged a render and could clip the pill's digits) with a deterministic measure — a hidden measurer lays out all chips + a sample pill, and we compute the exact fit in one pass. The count is always correct now (verified across varied rows), and a bare "+" with no number is structurally impossible. --- frontend/src/components/AnchoredPopover.tsx | 34 ++++---- frontend/src/components/ChannelTags.tsx | 97 +++++++++++++-------- frontend/src/components/Channels.tsx | 2 +- frontend/src/i18n/locales/en/channels.json | 4 +- frontend/src/i18n/locales/hu/channels.json | 4 +- 5 files changed, 83 insertions(+), 58 deletions(-) diff --git a/frontend/src/components/AnchoredPopover.tsx b/frontend/src/components/AnchoredPopover.tsx index 8f112d63..95f5d0c6 100644 --- a/frontend/src/components/AnchoredPopover.tsx +++ b/frontend/src/components/AnchoredPopover.tsx @@ -1,4 +1,4 @@ -import { type CSSProperties, type ReactNode, type Ref } from "react"; +import { type CSSProperties, type ReactNode } from "react"; import { useFloating, autoUpdate, @@ -13,7 +13,6 @@ import { useDismiss, useRole, useInteractions, - useMergeRefs, useFloatingNodeId, safePolygon, FloatingNode, @@ -122,7 +121,6 @@ export function PopoverSurface({ className, style, children, - floatingRef, focusManaged = false, initialFocus, }: { @@ -130,8 +128,6 @@ export function PopoverSurface({ className?: string; style?: CSSProperties; children: ReactNode; - /** Extra ref merged onto the floating node (e.g. a useScrollFade callback ref). */ - floatingRef?: Ref; focusManaged?: boolean; initialFocus?: number | React.MutableRefObject; }) { @@ -144,30 +140,38 @@ export function PopoverSurface({ referenceHidden, isPositioned, open, + mode, } = popover; - const mergedRef = useMergeRefs([refs.setFloating, floatingRef ?? null]); - const floatingProps = getFloatingProps(); + const { style: floatingPropStyle, ...floatingProps } = getFloatingProps() as { + style?: CSSProperties; + } & Record; if (!open) return null; + const zClass = mode === "hover" ? "z-tooltip" : "z-popover"; + const surface = (
- {children} + {/* Inner wrapper carries the visual styling + pop-in animation. Keeping the ANIMATION off the + positioned element is essential: the popIn keyframe animates `transform`, which would + otherwise override Floating UI's `transform: translate(x,y)` and park the popover at (0,0) + for the animation's duration — the "flash in the top-left corner" bug. */} +
+ {children} +
); diff --git a/frontend/src/components/ChannelTags.tsx b/frontend/src/components/ChannelTags.tsx index cade20b0..5c70a382 100644 --- a/frontend/src/components/ChannelTags.tsx +++ b/frontend/src/components/ChannelTags.tsx @@ -3,7 +3,6 @@ import { Check, Plus } from "lucide-react"; import { useTranslation } from "react-i18next"; import { type Tag } from "../lib/api"; import { useAnchoredPopover, PopoverSurface } from "./AnchoredPopover"; -import { useScrollFade } from "../lib/useScrollFade"; // The channel tag surface, shared by the channel manager (table + cards) and the channel detail // page. Built on the R8 popover platform. Two design goals it fixes over the old inline TagsCell: @@ -139,36 +138,47 @@ export function ChannelTagsControl({ const { t } = useTranslation(); const attached = userTags.filter((tg) => attachedIds.includes(tg.id)); - // Greedy single-line fit: start showing all, shrink one chip at a time until the row (including - // the "+N" pill it then needs) fits its bounded width. Re-expands and re-fits on width/scale - // change via ResizeObserver — measurement is off the real rem-rendered chips, not a px guess. + // Deterministic single-line fit: a hidden measurer lays out ALL chips + a sample "+N" pill at their + // natural widths; we read each chip's right edge and compute EXACTLY how many fit, reserving the + // pill's width whenever any overflow. One pass (no iterative re-render), so the "+N" count is always + // right as tags are added/removed. Re-measures on width/scale change via ResizeObserver — off real + // rem boxes, never a px guess. const rowRef = useRef(null); - const [maxWidth, setMaxWidth] = useState(0); + const measureRef = useRef(null); const [visible, setVisible] = useState(attached.length); + const idsKey = attached.map((tg) => tg.id).join(","); - // Track the bounded width so a font-scale/viewport change re-fits — measured off the real box, - // never a px guess (widths here are rem and grow with the text-size slider). useLayoutEffect(() => { - const el = rowRef.current; - if (!el || typeof ResizeObserver === "undefined") return; - const ro = new ResizeObserver((entries) => { - const w = entries[0]?.contentRect.width; - if (w != null) setMaxWidth(w); - }); - ro.observe(el); + const row = rowRef.current; + const measure = measureRef.current; + if (!row || !measure) return; + const compute = () => { + const nodes = Array.from(measure.children) as HTMLElement[]; + const pill = nodes[nodes.length - 1]; + if (!pill) return; + const pillW = pill.offsetWidth; + const chips = nodes.slice(0, -1); + const GAP = 4; // gap-1 + const avail = row.clientWidth; + let count = chips.length; + for (let i = 0; i < chips.length; i++) { + const chip = chips[i]; + if (!chip) break; + const right = chip.offsetLeft + chip.offsetWidth; // right edge within the measurer + const moreAfter = i < chips.length - 1; + if (right > avail - (moreAfter ? pillW + GAP : 0)) { + count = i; + break; + } + } + setVisible(count); + }; + compute(); + if (typeof ResizeObserver === "undefined") return; + const ro = new ResizeObserver(compute); + ro.observe(row); return () => ro.disconnect(); - }, []); - - // Reset to "show all" whenever the tag set or the available width changes… - useLayoutEffect(() => { - setVisible(attached.length); - }, [attached.length, maxWidth]); - - // …then drop one chip at a time until the row (with the "+N" pill it then needs) fits its width. - useLayoutEffect(() => { - const el = rowRef.current; - if (el && el.scrollWidth > el.clientWidth + 1 && visible > 0) setVisible((v) => v - 1); - }, [visible, maxWidth, attached.length]); + }, [idsKey]); const hidden = attached.length - visible; @@ -187,15 +197,28 @@ export function ChannelTagsControl({ mode: "hover", placement: "bottom", }); - const fade = useScrollFade(); + + const pillClass = + "shrink-0 text-[10px] px-1.5 py-0.5 rounded-full bg-card text-muted border border-border"; return ( - // FIXED width (not just max-w): under `table-layout:auto` a content-driven cell re-solves the - // whole table's column widths every time a chip is added — the table visibly shifts until the cap - // is hit. A fixed width makes the cell's footprint constant, so adding/removing tags never moves - // any column; the chip row greedily fits within it and collapses the rest into "+N". (max-w-full - // lets a narrower card shrink it.) -
+ // FIXED width so the cell's footprint is constant under `table-layout:auto`: a content-driven cell + // re-solves EVERY column's width as chips are added, visibly shifting the whole table. A fixed + // width keeps the table stable and centered; the chip row fits what it can and collapses the rest + // into "+N". (max-w-full lets a narrower card shrink it.) +
+ {/* Hidden measurer: all chips + a sample pill, laid out unbounded to read their true widths. */} +
+ {attached.map((tg) => ( + + ))} + +{attached.length} +
+
{attached.slice(0, visible).map((tg) => ( +{hidden} @@ -231,7 +254,7 @@ export function ChannelTagsControl({ {/* Hover: reveal every attached tag when some are collapsed into "+N". */} {attached.map((tg) => ( ({ value: String(tg.id), label: tg.name })), diff --git a/frontend/src/i18n/locales/en/channels.json b/frontend/src/i18n/locales/en/channels.json index 756fdd3c..7ec4d8ca 100644 --- a/frontend/src/i18n/locales/en/channels.json +++ b/frontend/src/i18n/locales/en/channels.json @@ -107,8 +107,8 @@ "backfillThisHint": "Fetch this channel's full back-catalog (older videos + complete search). Click again to cancel the request.", "resetBackfill": "Reset & re-fetch this channel", "resetHint": "Admin: re-fetch this channel from scratch (recent + full back-catalog), regardless of its current sync state.", - "fullHistoryQueued": "full history queued", - "fullHistoryComing": "full history coming", + "fullHistoryQueued": "queued", + "fullHistoryComing": "coming", "queuedRequestedHint": "Full history requested — this channel's whole back-catalog will backfill as the shared quota allows. Click to cancel your request.", "queuedByOtherHint": "Another subscriber already requested this channel's full history, so its whole back-catalog is on its way to everyone — nothing to do here.", "hiddenHint": "Hidden — this channel's videos are kept out of your feed. Click to show them again.", diff --git a/frontend/src/i18n/locales/hu/channels.json b/frontend/src/i18n/locales/hu/channels.json index 4748812c..e61d737c 100644 --- a/frontend/src/i18n/locales/hu/channels.json +++ b/frontend/src/i18n/locales/hu/channels.json @@ -107,8 +107,8 @@ "backfillThisHint": "A csatorna teljes archívumának letöltése (régebbi videók + teljes keresés). Újra kattintva visszavonod a kérést.", "resetBackfill": "Csatorna resetelése és újraletöltése", "resetHint": "Admin: a csatorna teljes újraletöltése a nulláról (legutóbbi + teljes archívum), a jelenlegi szinkron-állapottól függetlenül.", - "fullHistoryQueued": "teljes előzmény sorban", - "fullHistoryComing": "teljes előzmény úton", + "fullHistoryQueued": "sorban", + "fullHistoryComing": "úton", "queuedRequestedHint": "Teljes előzmény kérve — a csatorna teljes archívuma letöltődik, ahogy a megosztott kvóta engedi. Kattints a kérés visszavonásához.", "queuedByOtherHint": "Egy másik feliratkozó már kérte ennek a csatornának a teljes előzményét, így a teljes archívuma már úton van mindenkihez — itt nincs teendőd.", "hiddenHint": "Elrejtve — a csatorna videói nem jelennek meg a hírfolyamodban. Kattints, hogy újra láthatóak legyenek.", From 2b70ba7a1f68d8e5d3e91000c549a97a910dab3c Mon Sep 17 00:00:00 2001 From: npeter83 Date: Wed, 29 Jul 2026 01:50:58 +0200 Subject: [PATCH 05/18] =?UTF-8?q?fix(channels):=20tag-cell=20review=20foll?= =?UTF-8?q?ow-ups=20=E2=80=94=20rename-aware=20fit,=20measured=20gap,=20cl?= =?UTF-8?q?ose=20reveal-all?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the /code-review findings on the tag cell: - key the overflow-fit measurement on tag id+name so a RENAME (which changes chip width) re-runs the fit, not just add/remove; - reserve the '+N' pill space using the MEASURED gap (gap-1 is rem, scales with the font slider) instead of a px constant; - force the reveal-all hover popover's state closed when nothing is collapsed, so it can't linger open against an unmounted '+N' trigger. --- frontend/src/components/ChannelTags.tsx | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/frontend/src/components/ChannelTags.tsx b/frontend/src/components/ChannelTags.tsx index 5c70a382..b2a3cb97 100644 --- a/frontend/src/components/ChannelTags.tsx +++ b/frontend/src/components/ChannelTags.tsx @@ -1,4 +1,4 @@ -import { useLayoutEffect, useRef, useState } from "react"; +import { useEffect, useLayoutEffect, useRef, useState } from "react"; import { Check, Plus } from "lucide-react"; import { useTranslation } from "react-i18next"; import { type Tag } from "../lib/api"; @@ -146,7 +146,8 @@ export function ChannelTagsControl({ const rowRef = useRef(null); const measureRef = useRef(null); const [visible, setVisible] = useState(attached.length); - const idsKey = attached.map((tg) => tg.id).join(","); + // Key on id AND name so a RENAME (which changes a chip's width) re-runs the fit, not just add/remove. + const tagsKey = attached.map((tg) => `${tg.id}:${tg.name}`).join("|"); useLayoutEffect(() => { const row = rowRef.current; @@ -158,7 +159,12 @@ export function ChannelTagsControl({ if (!pill) return; const pillW = pill.offsetWidth; const chips = nodes.slice(0, -1); - const GAP = 4; // gap-1 + // Measure the real rendered gap (gap-1 is rem — it scales with the font slider) off the box + // between the last chip and the sample pill, rather than assuming a px constant. + const lastChip = chips[chips.length - 1]; + const gap = lastChip + ? Math.max(0, pill.offsetLeft - (lastChip.offsetLeft + lastChip.offsetWidth)) + : 0; const avail = row.clientWidth; let count = chips.length; for (let i = 0; i < chips.length; i++) { @@ -166,7 +172,7 @@ export function ChannelTagsControl({ if (!chip) break; const right = chip.offsetLeft + chip.offsetWidth; // right edge within the measurer const moreAfter = i < chips.length - 1; - if (right > avail - (moreAfter ? pillW + GAP : 0)) { + if (right > avail - (moreAfter ? pillW + gap : 0)) { count = i; break; } @@ -178,7 +184,7 @@ export function ChannelTagsControl({ const ro = new ResizeObserver(compute); ro.observe(row); return () => ro.disconnect(); - }, [idsKey]); + }, [tagsKey]); const hidden = attached.length - visible; @@ -197,6 +203,11 @@ export function ChannelTagsControl({ mode: "hover", placement: "bottom", }); + // If tags change so nothing is collapsed anymore, the "+N" trigger unmounts — force the reveal-all + // popover's state closed so it can't linger open against a gone reference. + useEffect(() => { + if (hidden === 0 && overflowOpen) setOverflowOpen(false); + }, [hidden, overflowOpen]); const pillClass = "shrink-0 text-[10px] px-1.5 py-0.5 rounded-full bg-card text-muted border border-border"; From 56f95fa6269fc47fa241f08ba7dce0664216965b Mon Sep 17 00:00:00 2001 From: npeter83 Date: Wed, 29 Jul 2026 02:04:18 +0200 Subject: [PATCH 06/18] feat(channels): tag editing on the channel detail page (R8 S1 Part 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bring the same tag control to the channel page, next to Subscribe: - backend: GET /api/channels/{id} now returns this user's `tag_ids` for the channel (added to _channel_detail_dict + the ChannelDetail interface), matching the manager list shape; - ChannelPage renders the shared ChannelTagsControl, wired to attach/detach/create with an optimistic update on the channel-detail cache (reverting on failure) and a manager list invalidation so a tag added here shows there too. Reuses the R8 popover platform + ChannelTagsControl — no new tag UI. --- backend/app/routes/channels.py | 12 +++++++ frontend/src/components/ChannelPage.tsx | 44 ++++++++++++++++++++++++- frontend/src/lib/api/channels.ts | 1 + 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/backend/app/routes/channels.py b/backend/app/routes/channels.py index 3325d509..2bb2c1ba 100644 --- a/backend/app/routes/channels.py +++ b/backend/app/routes/channels.py @@ -230,6 +230,7 @@ def _channel_detail_dict( blocked: bool, stored: int, deep_requested: bool, + tag_ids: list[int], ) -> dict: return { "id": channel.id, @@ -257,6 +258,9 @@ def _channel_detail_dict( "backfill_done": channel.backfill_done, "from_explore": channel.from_explore, "details_synced": channel.details_synced_at is not None, + # This user's personal tag links for the channel (same shape as the manager list), so the + # channel page can offer the same tag editor next to Subscribe. + "tag_ids": tag_ids, } @@ -297,6 +301,13 @@ def channel_detail( ).first() is not None blocked = _is_blocked(db, user, channel_id) stored = db.scalar(select(func.count(Video.id)).where(Video.channel_id == channel_id)) or 0 + tag_ids = list( + db.scalars( + select(ChannelTag.tag_id).where( + ChannelTag.user_id == user.id, ChannelTag.channel_id == channel_id + ) + ).all() + ) return _channel_detail_dict( channel, subscribed=subscribed, @@ -304,6 +315,7 @@ def channel_detail( blocked=blocked, stored=int(stored), deep_requested=bool(sub.deep_requested) if sub else False, + tag_ids=tag_ids, ) diff --git a/frontend/src/components/ChannelPage.tsx b/frontend/src/components/ChannelPage.tsx index a8bef922..e3ce76d4 100644 --- a/frontend/src/components/ChannelPage.tsx +++ b/frontend/src/components/ChannelPage.tsx @@ -22,9 +22,10 @@ import { useFeedFilters } from "./FeedFiltersProvider"; import { useConfirm } from "./ConfirmProvider"; import { notify } from "../lib/notifications"; import { notifyYouTubeActionError } from "../lib/youtubeErrors"; -import { api, type FeedFilters, type Me } from "../lib/api"; +import { api, type FeedFilters, type Me, type ChannelDetail } from "../lib/api"; import type { FeedView } from "../lib/feedView"; import { channelYouTubeUrl, formatViews } from "../lib/format"; +import { ChannelTagsControl } from "./ChannelTags"; // A dedicated channel page: an "About"-style header (banner, avatar, stats, subscribe) over the // channel's videos (the catalog filtered to this channel). For an un-subscribed channel it @@ -166,6 +167,39 @@ export default function ChannelPage({ onError: (err) => notifyYouTubeActionError(err, t("channel.fullHistoryFailed")), }); + // Personal tags, editable right here (the same control the channel manager uses). Optimistic on + // this channel's detail cache; a failed attach/detach reverts by refetching. Invalidate the manager + // list too so a tag added here shows there. createTag mirrors the manager's create-and-attach. + const tagsQuery = useQuery({ queryKey: qk.tags(), queryFn: api.tags }); + const userTags = (tagsQuery.data ?? []).filter((tg) => !tg.system); + const toggleTag = (tagId: number) => { + if (!ch) return; + const hasTag = ch.tag_ids.includes(tagId); + qc.setQueryData(qk.channel(channelId), (old) => + old + ? { + ...old, + tag_ids: hasTag ? old.tag_ids.filter((x) => x !== tagId) : [...old.tag_ids, tagId], + } + : old, + ); + const call = hasTag + ? api.detachChannelTag(channelId, tagId) + : api.attachChannelTag(channelId, tagId); + call + .then(() => qc.invalidateQueries({ queryKey: qk.channels() })) + .catch(() => qc.invalidateQueries({ queryKey: qk.channel(channelId) })); + }; + const createAndAttachTag = (name: string) => { + api + .createTag({ name }) + .then((tag) => { + qc.invalidateQueries({ queryKey: qk.tags() }); + toggleTag(tag.id); + }) + .catch(() => notify({ level: "error", message: t("channels.tags.createFailed") })); + }; + const onUnsubscribe = async () => { const ok = await confirm({ title: t("channel.unsubTitle"), @@ -411,6 +445,14 @@ export default function ChannelPage({ {t("channel.subscribe")} ))} + {ch && !me.is_demo && ( + + )}
diff --git a/frontend/src/lib/api/channels.ts b/frontend/src/lib/api/channels.ts index 0e49fe8a..1ed889b2 100644 --- a/frontend/src/lib/api/channels.ts +++ b/frontend/src/lib/api/channels.ts @@ -31,6 +31,7 @@ export interface ChannelDetail { backfill_done: boolean; from_explore: boolean; details_synced: boolean; + tag_ids: number[]; } export interface ExploreResult { From 3352d0095c4a01b25b59fbf141d02f07954c1eac Mon Sep 17 00:00:00 2001 From: npeter83 Date: Wed, 29 Jul 2026 02:10:00 +0200 Subject: [PATCH 07/18] fix(channels): only show the detail-page tag editor on subscribed channels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit attach_tag/detach_tag call _user_subscription, which 404s for a channel you don't follow — but the tag control was rendered for any non-demo channel, so tagging an un-subscribed/explored channel popped a global error + reverted the optimistic add. Gate the control on ch.subscribed (personal tags are for your subscriptions anyway). --- frontend/src/components/ChannelPage.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frontend/src/components/ChannelPage.tsx b/frontend/src/components/ChannelPage.tsx index e3ce76d4..52f590f8 100644 --- a/frontend/src/components/ChannelPage.tsx +++ b/frontend/src/components/ChannelPage.tsx @@ -445,7 +445,10 @@ export default function ChannelPage({ {t("channel.subscribe")} ))} - {ch && !me.is_demo && ( + {/* Tagging is a subscription-scoped action (attach_tag → _user_subscription 404s + otherwise), and personal tags are for your subscriptions — so only offer it on a + channel you actually follow. */} + {ch?.subscribed && !me.is_demo && ( Date: Wed, 29 Jul 2026 02:25:17 +0200 Subject: [PATCH 08/18] refactor(channels): shared tag-mutation hook + a channel_detail tag_ids test (R8 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the two remaining review findings on the tag work: - extract useChannelTagActions — the optimistic attach/detach + create-and-attach flow that the channel manager and the channel detail page had copy-pasted; the cache-shape-specific optimistic update is injected (manager maps a ManagedChannel[]; detail patches one ChannelDetail), so the two stay in sync; - add a DB-lane test asserting GET /api/channels/{id} returns exactly this user's personal tag links — not another user's, not the auto/system (user_id NULL) links. --- backend/tests/test_channel_tags.py | 54 ++++++++++++++++++++++++ frontend/src/components/ChannelPage.tsx | 51 +++++++++------------- frontend/src/components/Channels.tsx | 43 +++++++------------ frontend/src/lib/useChannelTagActions.ts | 43 +++++++++++++++++++ 4 files changed, 132 insertions(+), 59 deletions(-) create mode 100644 backend/tests/test_channel_tags.py create mode 100644 frontend/src/lib/useChannelTagActions.ts diff --git a/backend/tests/test_channel_tags.py b/backend/tests/test_channel_tags.py new file mode 100644 index 00000000..025d7bba --- /dev/null +++ b/backend/tests/test_channel_tags.py @@ -0,0 +1,54 @@ +"""DB-backed regression (R8 Part 2): GET /api/channels/{id} returns exactly THIS user's personal +tag links for the channel — not another user's, and not the auto/system links (user_id NULL) — +so the channel detail page can reuse the manager's tag editor. Would ship broken if the tag_ids +query ever dropped its user_id filter. + +Uses the DB lane (`db` fixture) — skipped when no Postgres is reachable. +""" +from datetime import datetime, timezone + +from fastapi.testclient import TestClient + +import app.state as state +from app.auth import current_user +from app.db import get_db +from app.main import app +from app.models import Channel, ChannelTag, Tag, User + + +def test_channel_detail_returns_only_this_users_tag_ids(db, monkeypatch): + # published_at set so channel_detail skips the YouTube-enrich path (no quota / network in tests). + ch = Channel( + id="UC_tags_test", + title="Tag Test Channel", + published_at=datetime(2020, 1, 1, tzinfo=timezone.utc), + ) + db.add(ch) + me = User(email="me@example.com") + other = User(email="other@example.com") + db.add_all([me, other]) + db.flush() + mine = Tag(user_id=me.id, name="Mine", category="other") + theirs = Tag(user_id=other.id, name="Theirs", category="other") + auto = Tag(user_id=None, name="English", category="language") # a system/auto tag + db.add_all([mine, theirs, auto]) + db.flush() + # My personal link, another user's link, and a system/auto link on the same channel — only MINE + # must come back for `me`. + db.add(ChannelTag(channel_id=ch.id, tag_id=mine.id, user_id=me.id, source="user")) + db.add(ChannelTag(channel_id=ch.id, tag_id=theirs.id, user_id=other.id, source="user")) + db.add(ChannelTag(channel_id=ch.id, tag_id=auto.id, user_id=None, source="auto")) + db.commit() + + # The request gate 503s an unconfigured instance; flip the process-level flag (auto-restored). + monkeypatch.setattr(state, "_configured_cache", True) + app.dependency_overrides[get_db] = lambda: db + app.dependency_overrides[current_user] = lambda: me + try: + client = TestClient(app) # no context-manager: skip lifespan (scheduler/HLS sweep) + resp = client.get(f"/api/channels/{ch.id}") + finally: + app.dependency_overrides.clear() + + assert resp.status_code == 200, resp.text + assert resp.json()["tag_ids"] == [mine.id] diff --git a/frontend/src/components/ChannelPage.tsx b/frontend/src/components/ChannelPage.tsx index 52f590f8..d898d2de 100644 --- a/frontend/src/components/ChannelPage.tsx +++ b/frontend/src/components/ChannelPage.tsx @@ -26,6 +26,7 @@ import { api, type FeedFilters, type Me, type ChannelDetail } from "../lib/api"; import type { FeedView } from "../lib/feedView"; import { channelYouTubeUrl, formatViews } from "../lib/format"; import { ChannelTagsControl } from "./ChannelTags"; +import { useChannelTagActions } from "../lib/useChannelTagActions"; // A dedicated channel page: an "About"-style header (banner, avatar, stats, subscribe) over the // channel's videos (the catalog filtered to this channel). For an un-subscribed channel it @@ -167,38 +168,24 @@ export default function ChannelPage({ onError: (err) => notifyYouTubeActionError(err, t("channel.fullHistoryFailed")), }); - // Personal tags, editable right here (the same control the channel manager uses). Optimistic on - // this channel's detail cache; a failed attach/detach reverts by refetching. Invalidate the manager - // list too so a tag added here shows there. createTag mirrors the manager's create-and-attach. + // Personal tags, editable right here (the same control + flow the channel manager uses). Optimistic + // on this channel's detail cache; a failed attach/detach reverts by refetching, and a success + // invalidates the manager list so a tag added here shows there too. const tagsQuery = useQuery({ queryKey: qk.tags(), queryFn: api.tags }); const userTags = (tagsQuery.data ?? []).filter((tg) => !tg.system); - const toggleTag = (tagId: number) => { - if (!ch) return; - const hasTag = ch.tag_ids.includes(tagId); - qc.setQueryData(qk.channel(channelId), (old) => - old - ? { - ...old, - tag_ids: hasTag ? old.tag_ids.filter((x) => x !== tagId) : [...old.tag_ids, tagId], - } - : old, - ); - const call = hasTag - ? api.detachChannelTag(channelId, tagId) - : api.attachChannelTag(channelId, tagId); - call - .then(() => qc.invalidateQueries({ queryKey: qk.channels() })) - .catch(() => qc.invalidateQueries({ queryKey: qk.channel(channelId) })); - }; - const createAndAttachTag = (name: string) => { - api - .createTag({ name }) - .then((tag) => { - qc.invalidateQueries({ queryKey: qk.tags() }); - toggleTag(tag.id); - }) - .catch(() => notify({ level: "error", message: t("channels.tags.createFailed") })); - }; + const { toggleTag, createAndAttachTag } = useChannelTagActions({ + optimisticToggle: (id, tagId, attach) => + qc.setQueryData(qk.channel(id), (old) => + old + ? { + ...old, + tag_ids: attach ? [...old.tag_ids, tagId] : old.tag_ids.filter((x) => x !== tagId), + } + : old, + ), + onSuccess: () => qc.invalidateQueries({ queryKey: qk.channels() }), + onError: (id) => qc.invalidateQueries({ queryKey: qk.channel(id) }), + }); const onUnsubscribe = async () => { const ok = await confirm({ @@ -452,8 +439,8 @@ export default function ChannelPage({ toggleTag(channelId, tagId, ch.tag_ids.includes(tagId))} + onCreate={(name) => createAndAttachTag(channelId, name)} /> )}
diff --git a/frontend/src/components/Channels.tsx b/frontend/src/components/Channels.tsx index 6b7e98f3..fdefb734 100644 --- a/frontend/src/components/Channels.tsx +++ b/frontend/src/components/Channels.tsx @@ -28,6 +28,7 @@ import DataTable, { type Column } from "./DataTable"; import ChannelLayoutGrid from "./ChannelLayoutGrid"; import { LoadingState, StateMessage } from "./QueryState"; import { ChannelTagsControl } from "./ChannelTags"; +import { useChannelTagActions } from "../lib/useChannelTagActions"; import ViewSwitcher, { type ViewOption } from "./ViewSwitcher"; import ColumnSortControl from "./ColumnSortControl"; import Pager from "./Pager"; @@ -195,34 +196,22 @@ export default function Channels() { }, []); // Tagging is updated optimistically in place (no refetch of the whole channel list, which - // made the toggle feel ~2s slow); only revert by refetching if the server call fails. - const toggleTag = (id: string, tagId: number, hasTag: boolean) => { - qc.setQueryData(qk.channels(), (old) => - (old ?? []).map((ch) => - ch.id === id - ? { - ...ch, - tag_ids: hasTag ? ch.tag_ids.filter((x) => x !== tagId) : [...ch.tag_ids, tagId], - } - : ch, + // made the toggle feel ~2s slow); only revert by refetching if the server call fails. The + // attach/detach + create-and-attach flow is shared with the channel detail page. + const { toggleTag, createAndAttachTag } = useChannelTagActions({ + optimisticToggle: (id, tagId, attach) => + qc.setQueryData(qk.channels(), (old) => + (old ?? []).map((ch) => + ch.id === id + ? { + ...ch, + tag_ids: attach ? [...ch.tag_ids, tagId] : ch.tag_ids.filter((x) => x !== tagId), + } + : ch, + ), ), - ); - const call = hasTag ? api.detachChannelTag(id, tagId) : api.attachChannelTag(id, tagId); - call.catch(() => qc.invalidateQueries({ queryKey: qk.channels() })); - }; - - // Create a brand-new personal tag from the row/card editor and attach it to this channel in one - // step (so tagging no longer requires the top-of-page "Manage tags" dialog first). Refresh the tag - // catalog so the new tag appears everywhere; attach optimistically via toggleTag. - const createAndAttachTag = (channelId: string, name: string) => { - api - .createTag({ name }) - .then((tag) => { - qc.invalidateQueries({ queryKey: qk.tags() }); - toggleTag(channelId, tag.id, false); - }) - .catch(() => notify({ level: "error", message: t("channels.tags.createFailed") })); - }; + onError: () => qc.invalidateQueries({ queryKey: qk.channels() }), + }); const syncSubs = useMutation({ mutationFn: () => api.syncSubscriptions(), onSuccess: (r: { subscriptions?: number }) => { diff --git a/frontend/src/lib/useChannelTagActions.ts b/frontend/src/lib/useChannelTagActions.ts new file mode 100644 index 00000000..b847f137 --- /dev/null +++ b/frontend/src/lib/useChannelTagActions.ts @@ -0,0 +1,43 @@ +import { useQueryClient } from "@tanstack/react-query"; +import { useTranslation } from "react-i18next"; +import { api } from "./api"; +import { qk } from "./queryKeys"; +import { notify } from "./notifications"; + +/** The shared channel-tag mutation flow used by both the channel manager and the channel detail + * page: optimistic attach/detach + a create-then-attach shortcut, with the cache-shape-specific + * optimistic update injected (the manager maps over a ManagedChannel[]; the detail patches one + * ChannelDetail). Keeps the two call sites from drifting. */ +export function useChannelTagActions(opts: { + /** Apply the optimistic tag change to whichever cache the caller owns. `attach` true = add. */ + optimisticToggle: (channelId: string, tagId: number, attach: boolean) => void; + /** Called after the attach/detach lands (e.g. sync a sibling cache). Optional. */ + onSuccess?: (channelId: string) => void; + /** Called if the attach/detach fails — revert by refetching the owning cache. */ + onError: (channelId: string) => void; +}) { + const qc = useQueryClient(); + const { t } = useTranslation(); + + const toggleTag = (channelId: string, tagId: number, hasTag: boolean) => { + opts.optimisticToggle(channelId, tagId, !hasTag); + const call = hasTag + ? api.detachChannelTag(channelId, tagId) + : api.attachChannelTag(channelId, tagId); + call.then(() => opts.onSuccess?.(channelId)).catch(() => opts.onError(channelId)); + }; + + // Create a brand-new personal tag and attach it in one step (so tagging doesn't require the + // top-of-page "Manage tags" dialog first). Refresh the tag catalog so it appears everywhere. + const createAndAttachTag = (channelId: string, name: string) => { + api + .createTag({ name }) + .then((tag) => { + qc.invalidateQueries({ queryKey: qk.tags() }); + toggleTag(channelId, tag.id, false); + }) + .catch(() => notify({ level: "error", message: t("channels.tags.createFailed") })); + }; + + return { toggleTag, createAndAttachTag }; +} From 5d1854b8081988d853ee9e60a2e3d8c88107b31f Mon Sep 17 00:00:00 2001 From: npeter83 Date: Wed, 29 Jul 2026 02:54:21 +0200 Subject: [PATCH 09/18] feat(popover): keyboard + focus + ARIA layer for the popover platform (R8 S2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the ViewSwitcher-grade menu model to the R8 popover platform, via Floating UI's idioms rather than a second hand-rolled roving engine: useListNavigation (↑/↓/Home/End roving, focusItemOnOpen so both click- and key-opens land on the first item), useTypeahead, and FloatingFocusManager (trap into the surface, return focus to the trigger on close). A new self-registers for navigation and carries the right role (menuitem / menuitemradio+aria-checked / option+aria-selected); menu/listbox surfaces are focus-managed by default. Wire the ChannelTags editor as the pilot: tag toggles become menuitemradio, and the inline create-input stopPropagations non-Escape keys so arrows and typed chars stay in the field (typeahead off for that menu). --- frontend/src/components/AnchoredPopover.tsx | 174 ++++++++++++++++++-- frontend/src/components/ChannelTags.tsx | 21 ++- 2 files changed, 179 insertions(+), 16 deletions(-) diff --git a/frontend/src/components/AnchoredPopover.tsx b/frontend/src/components/AnchoredPopover.tsx index 95f5d0c6..ed7c023e 100644 --- a/frontend/src/components/AnchoredPopover.tsx +++ b/frontend/src/components/AnchoredPopover.tsx @@ -1,4 +1,13 @@ -import { type CSSProperties, type ReactNode } from "react"; +import { + createContext, + useContext, + useEffect, + useRef, + useState, + type ButtonHTMLAttributes, + type CSSProperties, + type ReactNode, +} from "react"; import { useFloating, autoUpdate, @@ -12,10 +21,14 @@ import { useFocus, useDismiss, useRole, + useListNavigation, + useTypeahead, useInteractions, useFloatingNodeId, + useListItem, safePolygon, FloatingNode, + FloatingList, FloatingPortal, FloatingFocusManager, type Placement, @@ -33,7 +46,18 @@ import { // Positioning is portalled through FloatingPortal (escapes ancestor stacking contexts — same job the // house did for TagsCell). Every popover is a node in the app's so Escape / // outside-press dispatch is topmost-only across nested layers (S3 folds the Modal + players in too). -// Focus management (trap + return-to-trigger, roving nav) arrives in S2 via `focusManaged`. +// +// S2 (this layer) adds the keyboard/ARIA model the roles promise, mirroring `ViewSwitcher` (the house's +// textbook menu) but via Floating UI's idioms rather than a second hand-rolled roving engine: +// - useListNavigation → roving focus over the registered items (↑/↓/Home/End, loop), menu/listbox only +// - useTypeahead → type-to-focus a matching item by its label +// - FloatingFocusManager → focus moves into the surface on open and RETURNS to the trigger on close +// (a body-portaled popover sits at the end of the document; without this, Tab can't reach it and +// Escape strands focus on ). Non-modal so the rest of the page stays interactive. +// Call sites render their options as — it self-registers (useListItem) and carries the +// right role (menuitem / menuitemradio+aria-checked / option+aria-selected). A menu that also holds a +// free-text input (the tag creator) must stopPropagation non-Escape keys on that input so ↑/↓ and +// typeahead act on the text, not the list — see ChannelTags. // Internal to the platform; call sites pass the string literal via `mode`, they don't name the type. type PopoverMode = "menu" | "listbox" | "hover" | "dialog"; @@ -53,6 +77,9 @@ export interface AnchoredPopoverOptions { /** Cap the panel height to the available space (adds the `size` middleware). Off by default so a * caller's own max-height class wins; turn on for long lists that should shrink to fit. */ fitHeight?: boolean; + /** Type-to-focus a matching item by label (menu/listbox only). Default on; turn OFF for menus that + * contain a free-text input, where character keys belong to the text field. */ + typeahead?: boolean; } export type AnchoredPopover = ReturnType; @@ -60,9 +87,17 @@ export type AnchoredPopover = ReturnType; export function useAnchoredPopover(opts: AnchoredPopoverOptions) { const mode = opts.mode ?? "menu"; const isHover = mode === "hover"; + // Roving-keyboard list surfaces: a menu of actions or a listbox of options. A "dialog" popover holds + // arbitrary focusable content (its own Tab order), and "hover" is a passive tooltip — neither roves. + const isNav = mode === "menu" || mode === "listbox"; // null when not inside a — the hook still works, just without nested-layer awareness. const nodeId = useFloatingNodeId(); + // Item registry for list navigation + typeahead, populated by via useListItem. + const elementsRef = useRef>([]); + const labelsRef = useRef>([]); + const [activeIndex, setActiveIndex] = useState(null); + const { refs, floatingStyles, context, middlewareData, isPositioned } = useFloating({ nodeId, open: opts.open, @@ -87,6 +122,12 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) { ], }); + // A stale active item would highlight (and steal the next open's initial focus) the second time a + // menu opens; clear it whenever the surface closes. + useEffect(() => { + if (!opts.open) setActiveIndex(null); + }, [opts.open]); + const click = useClick(context, { enabled: !isHover }); const hover = useHover(context, { enabled: isHover, @@ -97,31 +138,119 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) { const focus = useFocus(context, { enabled: isHover }); const dismiss = useDismiss(context); const role = useRole(context, { role: opts.role ?? (isHover ? "tooltip" : "menu") }); + const listNav = useListNavigation(context, { + enabled: isNav, + listRef: elementsRef, + activeIndex, + onNavigate: setActiveIndex, + loop: true, + // Focus the first item on open for BOTH click and keyboard (the default 'auto' skips pointer-opens, + // which strands focus on the menu container — arrows then can't enter the roving list at all). + focusItemOnOpen: true, + }); + const typeahead = useTypeahead(context, { + enabled: isNav && (opts.typeahead ?? true), + listRef: labelsRef, + activeIndex, + onMatch: setActiveIndex, + }); - const interactions = useInteractions([click, hover, focus, dismiss, role]); + const interactions = useInteractions([click, hover, focus, dismiss, role, listNav, typeahead]); return { nodeId, mode, + isNav, open: opts.open, refs, floatingStyles, context, isPositioned, referenceHidden: middlewareData.hide?.referenceHidden ?? false, + elementsRef, + labelsRef, + activeIndex, ...interactions, }; } +// Carries the active index + item-props getter down to . FloatingList (in PopoverSurface) +// separately owns the elementsRef/labelsRef registration; this context is the active-state channel. +interface PopoverListContextValue { + activeIndex: number | null; + getItemProps: AnchoredPopover["getItemProps"]; +} +const PopoverListContext = createContext(null); + +type ItemRole = "menuitem" | "menuitemradio" | "option"; + +/** One keyboard-navigable option inside a menu/listbox `PopoverSurface`. Self-registers for roving + * focus + typeahead and applies the correct ARIA. Use `menuitemradio`+`checked` for a toggle, + * `option`+`selected` for a listbox choice, plain `menuitem` for an action. */ +export function PopoverItem({ + children, + onSelect, + role = "menuitem", + checked, + selected, + disabled, + className, + /** Typeahead match string; defaults to the text child. Set explicitly when children aren't plain text. */ + label, + ...rest +}: { + children: ReactNode; + onSelect?: () => void; + role?: ItemRole; + checked?: boolean; + selected?: boolean; + disabled?: boolean; + className?: string; + label?: string; +} & Omit, "onSelect" | "role" | "children" | "className">) { + const ctx = useContext(PopoverListContext); + const textLabel = label ?? (typeof children === "string" ? children : undefined); + const { ref, index } = useListItem({ label: textLabel }); + const active = ctx?.activeIndex === index; + + const ariaProps = + role === "menuitemradio" + ? { role, "aria-checked": !!checked } + : role === "option" + ? { role, "aria-selected": !!selected } + : { role }; + + const handlers = ctx + ? ctx.getItemProps({ onClick: () => !disabled && onSelect?.() }) + : { onClick: () => !disabled && onSelect?.() }; + + return ( + + ); +} + /** Renders the popover panel: the FloatingTree node + portal + the positioned surface. Merges the - * library's floating ref/props with the caller's className/style (and an optional extra ref, e.g. a - * scroll-fade callback ref). `focusManaged` wraps it in a FloatingFocusManager (S2). */ + * library's floating ref/props with the caller's className/style. Menu/listbox surfaces trap+return + * focus and host the list-navigation registry by default; pass `focusManaged={false}` to opt a + * surface out (e.g. a hover card). */ export function PopoverSurface({ popover, className, style, children, - focusManaged = false, + focusManaged, initialFocus, }: { popover: AnchoredPopover; @@ -136,11 +265,16 @@ export function PopoverSurface({ refs, floatingStyles, getFloatingProps, + getItemProps, nodeId, referenceHidden, isPositioned, open, mode, + isNav, + activeIndex, + elementsRef, + labelsRef, } = popover; const { style: floatingPropStyle, ...floatingProps } = getFloatingProps() as { style?: CSSProperties; @@ -149,6 +283,9 @@ export function PopoverSurface({ if (!open) return null; const zClass = mode === "hover" ? "z-tooltip" : "z-popover"; + // Everything but a passive hover card manages focus by default (trap into the surface, return to + // the trigger on close). + const managed = focusManaged ?? mode !== "hover"; const surface = (
); + // Menu/listbox surfaces host the item registry (FloatingList) + active-state context so nested + // s can self-register and rove. + const positioned = isNav ? ( + + + {surface} + + + ) : ( + surface + ); + return ( - {focusManaged ? ( - - {surface} + {managed ? ( + + {positioned} ) : ( - surface + positioned )} diff --git a/frontend/src/components/ChannelTags.tsx b/frontend/src/components/ChannelTags.tsx index b2a3cb97..b6c6b9d9 100644 --- a/frontend/src/components/ChannelTags.tsx +++ b/frontend/src/components/ChannelTags.tsx @@ -2,7 +2,7 @@ import { useEffect, useLayoutEffect, useRef, useState } from "react"; import { Check, Plus } from "lucide-react"; import { useTranslation } from "react-i18next"; import { type Tag } from "../lib/api"; -import { useAnchoredPopover, PopoverSurface } from "./AnchoredPopover"; +import { useAnchoredPopover, PopoverSurface, PopoverItem } from "./AnchoredPopover"; // The channel tag surface, shared by the channel manager (table + cards) and the channel detail // page. Built on the R8 popover platform. Two design goals it fixes over the old inline TagsCell: @@ -66,11 +66,13 @@ function TagEditorMenu({ {userTags.map((tg) => { const on = attached.has(tg.id); return ( - + ); })} {userTags.length === 0 && ( @@ -94,6 +96,11 @@ function TagEditorMenu({ value={draft} onChange={(e) => setDraft(e.target.value)} onKeyDown={(e) => { + // Escape bubbles to the popover's dismiss (close + refocus the trigger); every other key — + // arrows, Home/End, typed characters — is text editing here, so keep it out of the menu's + // list-navigation/typeahead handlers on the surrounding surface. + if (e.key === "Escape") return; + e.stopPropagation(); if (e.key === "Enter") { e.preventDefault(); submit(); @@ -194,6 +201,8 @@ export function ChannelTagsControl({ onOpenChange: setEditorOpen, mode: "menu", placement: "bottom-start", + // The menu contains a free-text "create tag" input — type-to-focus would fight the text field. + typeahead: false, }); const [overflowOpen, setOverflowOpen] = useState(false); From d950867d2653666b9701e2e7da26e8596924857b Mon Sep 17 00:00:00 2001 From: npeter83 Date: Wed, 29 Jul 2026 03:05:39 +0200 Subject: [PATCH 10/18] fix(popover): visible accent highlight on the active menu item (R8 S2 UAT) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tag editor's active row barely differed from the menu background. Drive the highlight off the roving `data-active` attribute (reliable across click- vs key-open, unlike :focus-visible, which a click-open's programmatic focus doesn't trip) and tint it with the accent. `bg-accent/NN` can't be used — the theme's --accent is a hex var and Tailwind v3 can't inject alpha into it — so use a color-mix arbitrary value, matching the app's glass idiom. --- frontend/src/components/AnchoredPopover.tsx | 12 +++++++++++- frontend/src/components/ChannelTags.tsx | 4 ++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/frontend/src/components/AnchoredPopover.tsx b/frontend/src/components/AnchoredPopover.tsx index ed7c023e..5ea62784 100644 --- a/frontend/src/components/AnchoredPopover.tsx +++ b/frontend/src/components/AnchoredPopover.tsx @@ -186,7 +186,15 @@ type ItemRole = "menuitem" | "menuitemradio" | "option"; /** One keyboard-navigable option inside a menu/listbox `PopoverSurface`. Self-registers for roving * focus + typeahead and applies the correct ARIA. Use `menuitemradio`+`checked` for a toggle, - * `option`+`selected` for a listbox choice, plain `menuitem` for an action. */ + * `option`+`selected` for a listbox choice, plain `menuitem` for an action. + * + * The active (roving) item carries `data-active` — style the highlight off THAT, not `:focus-visible`: + * a click-opened menu focuses the first item programmatically, which does NOT reliably trip the + * focus-visible heuristic, so a `:focus-visible` highlight can stay invisible until the first arrow + * key. The recommended recipe is an accent tint — and note `bg-accent/NN` does NOT work here (the + * theme's `--accent` is a hex var, and Tailwind v3 can't inject alpha into it), so use a color-mix + * arbitrary value like the app's own glass idiom: + * `data-[active]:bg-[color-mix(in_srgb,var(--accent)_24%,transparent)] data-[active]:text-fg`. */ export function PopoverItem({ children, onSelect, @@ -230,6 +238,8 @@ export function PopoverItem({ type="button" // Roving tabindex: only the active item is in the Tab order; arrows move focus between them. tabIndex={active ? 0 : -1} + // Highlight hook: reliable across click- vs key-open (see the note above), unlike :focus-visible. + data-active={active ? "true" : undefined} disabled={disabled} className={className} {...ariaProps} diff --git a/frontend/src/components/ChannelTags.tsx b/frontend/src/components/ChannelTags.tsx index b6c6b9d9..100dbaa4 100644 --- a/frontend/src/components/ChannelTags.tsx +++ b/frontend/src/components/ChannelTags.tsx @@ -72,8 +72,8 @@ function TagEditorMenu({ checked={on} onSelect={() => onToggle(tg.id)} label={tg.name} - className={`w-full flex items-center gap-2 text-left text-xs px-2 py-1.5 rounded-md outline-none transition focus-visible:bg-card ${ - on ? "text-fg" : "text-muted hover:bg-card" + className={`w-full flex items-center gap-2 text-left text-xs px-2 py-1.5 rounded-md outline-none transition hover:bg-card data-[active]:bg-[color-mix(in_srgb,var(--accent)_24%,transparent)] data-[active]:text-fg ${ + on ? "text-fg" : "text-muted" }`} > Date: Wed, 29 Jul 2026 03:29:37 +0200 Subject: [PATCH 11/18] feat(overlay): shared layer-stack for topmost-only Escape across modals, popovers, players (R8 S3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the three mutually-unaware Escape systems — Modal's private modalStack, the popovers' document listeners with a blanket stopPropagation, and the two players' modalCount() checks — with one shared lib/layerStack.ts. Every open dismissable layer registers in open order; a layer acts on Escape only when it is topmost, and reads anyLayerAbove() to decide whether to defer. Coordination is now a stack lookup, not event-propagation order, so it no longer depends on which listener happens to fire first. - Modal + useDismiss register via the stack; useDismiss drops the blanket stopPropagation (its documented side effect: it swallowed a player's own Escape while any popover was open). - Both players register as a layer and defer via anyLayerAbove() instead of modalCount(); PlayerModal checks it before its maximise/fullscreen steps. A player MENU is a layer too, so it closes first and the next Escape reaches the auto-skip-cancel — fixing the documented PlexPlayer bug. - AnchoredPopover routes non-hover Escape through the stack (floating-ui's own escapeKey off there) and registers as a layer, so a player/modal underneath defers once S4 migrates the in-player menus. FloatingTree still owns popover-vs-popover ordering; the stack adds the cross-type ordering. --- frontend/src/components/AnchoredPopover.tsx | 11 ++- frontend/src/components/Modal.tsx | 25 +++---- frontend/src/components/PlayerModal.tsx | 21 ++++-- frontend/src/components/PlexPlayer.tsx | 20 +++++- frontend/src/lib/layerStack.ts | 76 +++++++++++++++++++++ frontend/src/lib/useDismiss.ts | 34 ++++----- 6 files changed, 142 insertions(+), 45 deletions(-) create mode 100644 frontend/src/lib/layerStack.ts diff --git a/frontend/src/components/AnchoredPopover.tsx b/frontend/src/components/AnchoredPopover.tsx index 5ea62784..bd4255f1 100644 --- a/frontend/src/components/AnchoredPopover.tsx +++ b/frontend/src/components/AnchoredPopover.tsx @@ -34,6 +34,7 @@ import { type Placement, type UseRoleProps, } from "@floating-ui/react"; +import { useLayer } from "../lib/layerStack"; // R8 popover platform. ONE anchored-popover primitive over @floating-ui/react, replacing the ~9 // hand-rolled positioning engines (measured flip / width clamp / rAF scroll-follow / scroller-relative @@ -136,7 +137,11 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) { handleClose: safePolygon(), }); const focus = useFocus(context, { enabled: isHover }); - const dismiss = useDismiss(context); + // Escape for an interactive (non-hover) popover is dispatched through the shared `layerStack` + // (below) so it coordinates with Modals and players, not just other floating nodes — so turn + // floating-ui's own Escape off there. Outside-press stays with floating-ui (tree-aware). Hover + // tooltips keep floating-ui's Escape and are NOT layers (they mustn't make a player defer). + const dismiss = useDismiss(context, { escapeKey: isHover }); const role = useRole(context, { role: opts.role ?? (isHover ? "tooltip" : "menu") }); const listNav = useListNavigation(context, { enabled: isNav, @@ -157,6 +162,10 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) { const interactions = useInteractions([click, hover, focus, dismiss, role, listNav, typeahead]); + // Register the open interactive popover as a dismissable layer (topmost-only Escape, and so a + // player/modal underneath defers to it). Hover tooltips are excluded on purpose. + useLayer(opts.open && !isHover, () => opts.onOpenChange(false)); + return { nodeId, mode, diff --git a/frontend/src/components/Modal.tsx b/frontend/src/components/Modal.tsx index 805743fc..170e3130 100644 --- a/frontend/src/components/Modal.tsx +++ b/frontend/src/components/Modal.tsx @@ -3,20 +3,12 @@ import { createPortal } from "react-dom"; import { useTranslation } from "react-i18next"; import { X } from "lucide-react"; import { useBackToClose } from "../lib/history"; +import { pushLayer, removeLayer, isTopmost } from "../lib/layerStack"; -// Stack of open modals so ESC only closes the topmost one — e.g. an error dialog over the -// tag editor: pressing ESC dismisses just the error and returns to the editor underneath. -let modalStack: number[] = []; -let nextModalId = 1; - -/** How many s are currently open. The players (which use their own window-level Escape - * listeners, not this stack) check it so their Escape defers to a Modal opened above them — - * otherwise one Escape closes the player UNDER the dialog and then the dialog too (U-C). */ -export function modalCount(): number { - return modalStack.length; -} - -// Small centered modal shell (portaled to ): backdrop + ESC + scroll-lock close. +// Small centered modal shell (portaled to ): backdrop + ESC + scroll-lock close. Each open +// modal is a layer in the shared `layerStack` so ESC only closes the topmost one — e.g. an error +// dialog over the tag editor dismisses just the error and returns to the editor underneath, and a +// player below defers to it (R8 S3). export default function Modal({ title, onClose, @@ -41,10 +33,9 @@ export default function Modal({ const onCloseRef = useRef(onClose); onCloseRef.current = onClose; useEffect(() => { - const id = nextModalId++; - modalStack.push(id); + const id = pushLayer(); const onKey = (e: KeyboardEvent) => { - if (e.key === "Escape" && modalStack[modalStack.length - 1] === id) { + if (e.key === "Escape" && isTopmost(id)) { e.stopPropagation(); onCloseRef.current(); } @@ -54,7 +45,7 @@ export default function Modal({ document.body.style.overflow = "hidden"; return () => { window.removeEventListener("keydown", onKey); - modalStack = modalStack.filter((x) => x !== id); + removeLayer(id); document.body.style.overflow = prev; }; }, []); diff --git a/frontend/src/components/PlayerModal.tsx b/frontend/src/components/PlayerModal.tsx index 23c5fd83..4aa5bd09 100644 --- a/frontend/src/components/PlayerModal.tsx +++ b/frontend/src/components/PlayerModal.tsx @@ -27,7 +27,7 @@ import { } from "lucide-react"; import Avatar from "./Avatar"; import AddToPlaylist from "./AddToPlaylist"; -import { modalCount } from "./Modal"; +import { pushLayer, removeLayer, anyLayerAbove } from "../lib/layerStack"; import DownloadButton from "./DownloadButton"; import { api, type Video, type VideoStatus } from "../lib/api"; import type { YTNamespace, YTPlayer, YTPlayerEvent } from "../lib/youtube"; @@ -490,12 +490,28 @@ export default function PlayerModal({ staleTime: 5 * 60_000, }); + // This player is a dismissable layer: register it so a Modal or popover opened ABOVE it (e.g. the + // download dialog, or — after S4 — an anchored menu) takes Escape first and this player defers. + const layerIdRef = useRef(null); + useEffect(() => { + const id = pushLayer(); + layerIdRef.current = id; + return () => { + removeLayer(id); + layerIdRef.current = null; + }; + }, []); + // Keyboard shortcuts (Esc close / F fullscreen / Space play-pause) + background scroll lock. // These fire only while focus is on our page, not inside the cross-origin player iframe — so // we focus the modal card on open and again on player-ready (autoplay can grab focus). useEffect(() => { const onKey = (e: KeyboardEvent) => { if (e.key === "Escape") { + // Defer to any layer opened above the player FIRST (a Modal like the download dialog, or an + // anchored popover): it takes the Escape and we do nothing — so we don't also step out of + // maximise or close the player underneath it. + if (layerIdRef.current != null && anyLayerAbove(layerIdRef.current)) return; // In fullscreen, let the browser's Esc exit fullscreen only — don't also close the modal. if (fullscreenElement()) return; // Same for our own maximise: Esc steps back out of it before it closes the player. @@ -504,9 +520,6 @@ export default function PlayerModal({ setMaximized(false); return; } - // Defer to any Modal opened above the player (e.g. the download dialog): its Escape closes - // it, ours would otherwise also fire and close the player underneath it (U-C). - if (modalCount() > 0) return; onClose(); return; } diff --git a/frontend/src/components/PlexPlayer.tsx b/frontend/src/components/PlexPlayer.tsx index 4dcb5509..225e089d 100644 --- a/frontend/src/components/PlexPlayer.tsx +++ b/frontend/src/components/PlexPlayer.tsx @@ -46,7 +46,7 @@ import { LS, useAccountPersistedObject } from "../lib/storage"; import { useDismiss } from "../lib/useDismiss"; import { useScrollFade } from "../lib/useScrollFade"; import { useBackToClose } from "../lib/history"; -import { modalCount } from "./Modal"; +import { pushLayer, removeLayer, anyLayerAbove } from "../lib/layerStack"; // The rich info overlay (poster/cast/ratings) reuses the same component as the card's info page. const PlexInfo = lazy(() => import("./PlexInfo")); @@ -808,6 +808,18 @@ export default function PlexPlayer({ itemId, onClose, queue }: Props) { handleBack(); }, [handleBack]); + // This player is a dismissable layer: register it so a Modal or popover opened ABOVE it (its own + // settings/tracks menus included) takes Escape first and this player defers (R8 S3). + const layerIdRef = useRef(null); + useEffect(() => { + const id = pushLayer(); + layerIdRef.current = id; + return () => { + removeLayer(id); + layerIdRef.current = null; + }; + }, []); + // --- keyboard -------------------------------------------------------------------------------- useEffect(() => { const onKey = (e: KeyboardEvent) => { @@ -874,8 +886,10 @@ export default function PlexPlayer({ itemId, onClose, queue }: Props) { else handleBack(); break; case "Escape": - // Defer to any Modal opened above the player — let its Escape close it, not ours (U-C). - if (modalCount() > 0) break; + // Defer to any layer opened above the player (Modal or anchored popover) — let it take the + // Escape, not us. A player MENU registers as a layer too, so it closes first and the next + // Escape reaches the auto-skip-cancel below (the old blanket-stopPropagation bug). + if (layerIdRef.current != null && anyLayerAbove(layerIdRef.current)) break; if (skipProgressRef.current != null) cancelAutoSkipRef.current(); else if (menuOpenRef.current) { setMenuOpen(false); diff --git a/frontend/src/lib/layerStack.ts b/frontend/src/lib/layerStack.ts new file mode 100644 index 00000000..d0380824 --- /dev/null +++ b/frontend/src/lib/layerStack.ts @@ -0,0 +1,76 @@ +import { useEffect, useRef } from "react"; + +// R8 S3 — one shared registry of open "dismissable layers" (modals, popovers, players), ordered by +// open time. It replaces the three mutually-unaware Escape systems that each guessed at ordering with +// its own `stopPropagation`: Modal's private `modalStack`, the popovers' document listeners, and the +// two players' `modalCount()` checks. Now EVERY layer keeps its own key handler but coordinates +// through this one stack — a layer acts on Escape only when it is topmost, and reads `anyAbove()` to +// know whether to defer. Because the decision is a stack lookup, not event-propagation order, the fix +// is robust to which listener happens to run first (the old stopPropagation was not). +// +// Notably this fixes the documented PlexPlayer bug: its menus register here, so the player sees a menu +// as a layer ABOVE it and defers, letting the menu's own Escape close it first — the player's +// auto-skip-cancel Escape then lands on the next press, instead of being swallowed by a blanket +// stopPropagation. (Popover-vs-popover ordering is still floating-ui's ; this stack adds +// the cross-TYPE ordering the tree can't see.) + +let stack: number[] = []; +let nextId = 1; + +export function pushLayer(): number { + const id = nextId++; + stack.push(id); + return id; +} + +export function removeLayer(id: number): void { + stack = stack.filter((x) => x !== id); +} + +export function isTopmost(id: number): boolean { + return stack.length > 0 && stack[stack.length - 1] === id; +} + +/** True when at least one layer sits above `id` — the "should I defer?" predicate for a layer with a + * richer key handler than a plain close (the players). */ +export function anyLayerAbove(id: number): boolean { + const i = stack.indexOf(id); + return i !== -1 && i < stack.length - 1; +} + +/** Register an open layer for as long as `active` is true, and get live predicates to read inside your + * own key handler. Pass `onEscape` to also install a topmost-only Escape listener (the common case — + * modals, popovers). Omit it for a layer that owns a bigger key handler and only needs `anyAbove()` + * to decide whether to defer (the players). */ +export function useLayer( + active: boolean, + onEscape?: () => void, +): { isTopmost: () => boolean; anyAbove: () => boolean } { + const idRef = useRef(null); + const escRef = useRef(onEscape); + escRef.current = onEscape; + + useEffect(() => { + if (!active) return; + const id = pushLayer(); + idRef.current = id; + let onKey: ((e: KeyboardEvent) => void) | undefined; + if (escRef.current) { + onKey = (e: KeyboardEvent) => { + if (e.key === "Escape" && isTopmost(id)) escRef.current?.(); + }; + document.addEventListener("keydown", onKey); + } + return () => { + if (onKey) document.removeEventListener("keydown", onKey); + removeLayer(id); + idRef.current = null; + }; + // Re-register only when open/closed toggles; onEscape is read fresh via escRef. + }, [active]); + + return { + isTopmost: () => idRef.current != null && isTopmost(idRef.current), + anyAbove: () => idRef.current != null && anyLayerAbove(idRef.current), + }; +} diff --git a/frontend/src/lib/useDismiss.ts b/frontend/src/lib/useDismiss.ts index 94b365bb..d5e7be2c 100644 --- a/frontend/src/lib/useDismiss.ts +++ b/frontend/src/lib/useDismiss.ts @@ -1,14 +1,25 @@ import { useEffect, type RefObject } from "react"; +import { useLayer } from "./layerStack"; /** While `active`, close (call `onClose`) on Escape or a mousedown outside ALL of the given * element(s). The reusable half of the popover/dropdown pattern that several components each * hand-rolled; positioning stays with the caller (it varies per popover). Pass the panel ref - * (and optionally a trigger ref so clicking the trigger doesn't immediately re-close). */ + * (and optionally a trigger ref so clicking the trigger doesn't immediately re-close). + * + * Escape is dispatched through the shared `layerStack` (R8 S3): the popover registers as a layer and + * closes only when it is topmost, so a Modal or another popover opened above it takes the Escape + * first, and a player below defers to it. This replaces the old blanket `stopPropagation` — which + * swallowed a player's own Escape (e.g. PlexPlayer's auto-skip-cancel) whenever any popover was open. */ export function useDismiss( active: boolean, onClose: () => void, refs: RefObject | RefObject[], ): void { + // Registration + topmost-only Escape. + useLayer(active, onClose); + + // Outside-press stays per-instance (the refs/positioning are the caller's): close on a mousedown + // that lands outside every registered element. useEffect(() => { if (!active) return; const list = Array.isArray(refs) ? refs : [refs]; @@ -16,29 +27,12 @@ export function useDismiss( const target = e.target as Node; if (!list.some((r) => r.current?.contains(target))) onClose(); }; - const onKey = (e: KeyboardEvent) => { - if (e.key === "Escape") { - // Stop the same Escape from also reaching a player's window-level listener underneath — - // this document listener bubbles first, so one Escape closes only this popover, not the - // popover AND the player behind it. (U-C quick-fix.) - // - // KNOWN LIMITATION (accepted, → R8 popover-platform): because this is a blanket - // stopPropagation, a PlexPlayer settings/tracks menu open DURING an auto-skip countdown - // swallows the Escape that would otherwise ALSO cancel the skip — Escape closes the menu, - // and the skip proceeds. Properly fixing this needs the shared layer-registry R8 builds - // (topmost-only dispatch), not a broad stopPropagation. - e.stopPropagation(); - onClose(); - } - }; document.addEventListener("mousedown", onDown); - document.addEventListener("keydown", onKey); return () => { document.removeEventListener("mousedown", onDown); - document.removeEventListener("keydown", onKey); }; - // Matches the original effects: (re)subscribe only when open/closed toggles. Refs are - // stable and onClose is read fresh from this run's closure. + // Matches the original effect: (re)subscribe only when open/closed toggles. Refs are stable and + // onClose is read fresh from this run's closure. // eslint-disable-next-line react-hooks/exhaustive-deps }, [active]); } From 2756c21dbd698096c752e351e937107df79aaf1b Mon Sep 17 00:00:00 2001 From: npeter83 Date: Thu, 30 Jul 2026 23:49:29 +0200 Subject: [PATCH 12/18] feat(popover): fullscreen-aware portal + migrate the hover surfaces (R8 S4a) The platform's FloatingPortal targeted , which is painted UNDERNEATH whatever holds the fullscreen lock. That is why the player's in-bar menus were either non-portaled (and clipped) or, for add-to-playlist / download, disabled outright in fullscreen. PopoverSurface now portals into `document.fullscreenElement` while one exists, via a new reactive `useFullscreenElement()` on the existing WebKit-aware fullscreen lib. Two floating-ui details this had to work around: a null `root` means "the container does not exist yet" (no portal node is created at all), so the no-fullscreen case must pass undefined; and the portal node is built once and ignores a later root change, so the portal is keyed on the fullscreen state to remount on a transition. Migrated the three hand-rolled hover engines onto the platform: - Tooltip: kept as a passive caption via a new `passthrough` prop on PopoverSurface, so it cannot swallow a click aimed at a control it overlaps during the close delay. Gains aria-describedby wiring it never had. - AboutCell: the 160 ms hide-timer hover bridge is now safePolygon; the close-on-page- scroll is now hide-and-follow, so the card reappears when the row scrolls back. - TagManager: same bridge removal; the scroll-fade moves to an inner scroller so the mask rides the scrolling box rather than the glass panel. Verified in-browser: tooltip flip + centering + pointer-transparency, About card flip- above with the measured height, tag popover right-start anchoring, and the portal round-trip body -> fullscreen element -> body on a live fullscreenchange. --- frontend/src/components/AboutCell.tsx | 148 ++++++-------------- frontend/src/components/AnchoredPopover.tsx | 20 ++- frontend/src/components/TagManager.tsx | 81 +++++------ frontend/src/components/Tooltip.tsx | 100 ++++--------- frontend/src/lib/fullscreen.ts | 15 ++ 5 files changed, 137 insertions(+), 227 deletions(-) diff --git a/frontend/src/components/AboutCell.tsx b/frontend/src/components/AboutCell.tsx index e8891785..68ca37ea 100644 --- a/frontend/src/components/AboutCell.tsx +++ b/frontend/src/components/AboutCell.tsx @@ -1,28 +1,34 @@ -import { useEffect, useLayoutEffect, useRef, useState } from "react"; -import { createPortal } from "react-dom"; +import { useState } from "react"; import { useQuery } from "@tanstack/react-query"; import { qk } from "../lib/queryKeys"; import { api } from "../lib/api"; import ChannelAboutContent from "./ChannelAbout"; - -const MARGIN = 8; -const POP_W = 480; // the overlay's fixed width (px) +import { PopoverSurface, useAnchoredPopover } from "./AnchoredPopover"; /** * A channel-description cell: one truncated line in the table, revealing the FULL channel "About" * (the same content as the detail page's About tab — description, links, country, language, topics, - * keywords) in a portalled glass overlay. The detail is fetched ON DEMAND on hover/focus, sharing - * the ["channel", id] react-query cache with the channel page, so the list stays light. The overlay - * is interactive (scrollable, links clickable) via a small hover-bridge; it closes on page scroll - * (but not when scrolling inside itself) or resize. Shared by the subscribed + discovery tables. + * keywords) in a glass hover card. The detail is fetched ON DEMAND on hover/focus, sharing the + * ["channel", id] react-query cache with the channel page, so the list stays light. The card is + * interactive (scrollable, links clickable) — the platform's safePolygon lets the cursor travel the + * gap, replacing the hand-rolled 160 ms hide-timer bridge. Shared by the subscribed + discovery + * tables. + * + * The card keeps the platform's default hover role (`tooltip`); making this rich, interactive panel + * properly keyboard-reachable belongs to R9 (focus/a11y), not to the mechanical migration. */ export default function AboutCell({ text, channelId }: { text: string | null; channelId: string }) { const preview = (text ?? "").trim(); - const ref = useRef(null); - const popRef = useRef(null); - const hideTimer = useRef | null>(null); const [open, setOpen] = useState(false); - const [coords, setCoords] = useState<{ left: number; top: number } | null>(null); + + const pop = useAnchoredPopover({ + open: open && !!preview, + onOpenChange: setOpen, + mode: "hover", + offset: 6, + // Pop instantly like the old cell did; the close grace lets the cursor reach the card. + hoverDelay: { open: 0, close: 160 }, + }); const { data: ch } = useQuery({ queryKey: qk.channel(channelId), @@ -31,105 +37,29 @@ export default function AboutCell({ text, channelId }: { text: string | null; ch staleTime: 5 * 60_000, }); - function place() { - const el = ref.current; - if (!el) return; - const r = el.getBoundingClientRect(); - setCoords({ - left: Math.min(Math.max(r.left, MARGIN), window.innerWidth - POP_W - MARGIN), - top: r.bottom + 6, - }); - } - function show() { - if (hideTimer.current) { - clearTimeout(hideTimer.current); - hideTimer.current = null; - } - place(); - setOpen(true); - } - // Small grace period so the cursor can travel the gap from the cell to the overlay without it - // vanishing — the overlay's own mouseenter cancels it. - function scheduleHide() { - if (hideTimer.current) clearTimeout(hideTimer.current); - hideTimer.current = setTimeout(() => setOpen(false), 160); - } - function cancelHide() { - if (hideTimer.current) { - clearTimeout(hideTimer.current); - hideTimer.current = null; - } - } - - useEffect( - () => () => { - if (hideTimer.current) clearTimeout(hideTimer.current); - }, - [], - ); - - // Close on page scroll (the fixed overlay would strand at a stale anchor) or resize — but NOT - // when the scroll happens inside the overlay itself (it is scrollable). - useEffect(() => { - if (!open) return; - const onScroll = (e: Event) => { - if (popRef.current && e.target instanceof Node && popRef.current.contains(e.target)) return; - setOpen(false); - }; - const onResize = () => setOpen(false); - window.addEventListener("scroll", onScroll, true); - window.addEventListener("resize", onResize); - return () => { - window.removeEventListener("scroll", onScroll, true); - window.removeEventListener("resize", onResize); - }; - }, [open]); - - // Flip above the anchor if the overlay would run off the bottom of the viewport. - useLayoutEffect(() => { - const pop = popRef.current; - const el = ref.current; - if (!open || !coords || !pop || !el) return; - const r = el.getBoundingClientRect(); - const h = pop.offsetHeight; - if (r.bottom + 6 + h > window.innerHeight - MARGIN && r.top - 6 - h > MARGIN) { - const top = r.top - 6 - h; - if (Math.abs(top - coords.top) > 0.5) setCoords((c) => (c ? { ...c, top } : c)); - } - }, [open, coords, ch]); - if (!preview) return —; return ( - - {preview} - {open && - coords && - createPortal( -
- {/* Full About once the detail loads; until then the list's truncated description. */} - {ch ? ( - - ) : ( -

{preview}

- )} -
, - document.body, + <> + + {preview} + + + {/* Full About once the detail loads; until then the list's truncated description. */} + {ch ? ( + + ) : ( +

{preview}

)} - + + ); } diff --git a/frontend/src/components/AnchoredPopover.tsx b/frontend/src/components/AnchoredPopover.tsx index bd4255f1..d0ca9d77 100644 --- a/frontend/src/components/AnchoredPopover.tsx +++ b/frontend/src/components/AnchoredPopover.tsx @@ -35,6 +35,7 @@ import { type UseRoleProps, } from "@floating-ui/react"; import { useLayer } from "../lib/layerStack"; +import { useFullscreenElement } from "../lib/fullscreen"; // R8 popover platform. ONE anchored-popover primitive over @floating-ui/react, replacing the ~9 // hand-rolled positioning engines (measured flip / width clamp / rAF scroll-follow / scroller-relative @@ -271,6 +272,7 @@ export function PopoverSurface({ children, focusManaged, initialFocus, + passthrough, }: { popover: AnchoredPopover; className?: string; @@ -278,6 +280,10 @@ export function PopoverSurface({ children: ReactNode; focusManaged?: boolean; initialFocus?: number | React.MutableRefObject; + /** The surface never receives pointer events — for a passive caption that must not swallow a click + * aimed at whatever it happens to cover during its close delay. Interactive hover cards (scrollable + * content, links) must NOT set this. */ + passthrough?: boolean; }) { const { context, @@ -295,6 +301,11 @@ export function PopoverSurface({ elementsRef, labelsRef, } = popover; + // A node portaled to is painted UNDERNEATH whatever holds the fullscreen lock, so a popover + // opened from a player's control bar in fullscreen would be invisible. Portal into the fullscreen + // element instead while one exists — this is what let the player's in-bar menus stop being either + // non-portaled (and clipped) or outright disabled in fullscreen. + const fsRoot = useFullscreenElement(); const { style: floatingPropStyle, ...floatingProps } = getFloatingProps() as { style?: CSSProperties; } & Record; @@ -317,8 +328,9 @@ export function PopoverSurface({ // Hide until the first position is measured (else it flashes at 0,0 before the transform // lands) and while the trigger is scrolled out of its clipping scroller (hide-and-follow). ...(!isPositioned || referenceHidden - ? { visibility: "hidden", pointerEvents: "none" } + ? { visibility: "hidden" as const, pointerEvents: "none" as const } : null), + ...(passthrough ? { pointerEvents: "none" as const } : null), }} > {/* Inner wrapper carries the visual styling + pop-in animation. Keeping the ANIMATION off the @@ -345,7 +357,11 @@ export function PopoverSurface({ return ( - + {/* `root` must be undefined (NOT null) to mean "default to " — floating-ui reads a null + root as "the container doesn't exist yet" and creates no portal node at all. And it only + builds that node once, so a root that changes mid-open is ignored; the key remounts the + portal on a fullscreen transition, which is exactly when the popover must move. */} + {managed ? ( so the list's own scroll can't clip it. +// Channel manager). The popover rides the R8 platform, so the list's own scroll can't clip it and +// the cursor can travel the gap into it (safePolygon) without the old hand-rolled 150 ms timer. function TagRow({ tag, channels, @@ -29,25 +30,20 @@ function TagRow({ const { t } = useTranslation(); const [name, setName] = useState(tag.name); const [open, setOpen] = useState(false); - const [pos, setPos] = useState({ left: 0, top: 0 }); const popFade = useScrollFade(); - const countRef = useRef(null); - const closeTimer = useRef(undefined); const commit = () => { const v = name.trim(); if (v && v !== tag.name) onRename(v); else setName(tag.name); }; - const openPop = () => { - window.clearTimeout(closeTimer.current); - if (channels.length === 0) return; - const r = countRef.current?.getBoundingClientRect(); - if (r) setPos({ left: r.right + 8, top: r.top }); - setOpen(true); - }; - const closeSoon = () => { - closeTimer.current = window.setTimeout(() => setOpen(false), 150); - }; + const pop = useAnchoredPopover({ + open: open && channels.length > 0, + onOpenChange: setOpen, + mode: "hover", + placement: "right-start", + offset: 8, + hoverDelay: { open: 0, close: 150 }, + }); return (
- {open && - createPortal( -
-
- {t("tagManager.onChannels")} -
- {channels.map((ch) => ( - - ))} -
, - document.body, - )} + +
+ {t("tagManager.onChannels")} +
+ {/* Inner scroller so the scroll-fade mask rides the scrolling box, not the glass panel. */} +
+ {channels.map((ch) => ( + + ))} +
+
); } diff --git a/frontend/src/components/Tooltip.tsx b/frontend/src/components/Tooltip.tsx index 7c2d94eb..2695d474 100644 --- a/frontend/src/components/Tooltip.tsx +++ b/frontend/src/components/Tooltip.tsx @@ -1,16 +1,13 @@ -import { useLayoutEffect, useRef, useState, useSyncExternalStore } from "react"; -import { createPortal } from "react-dom"; +import { useState, useSyncExternalStore } from "react"; +import { PopoverSurface, useAnchoredPopover } from "./AnchoredPopover"; import { hintsEnabled, subscribeHints } from "../lib/hints"; type Side = "top" | "bottom"; -type Coords = { left: number; top: number; placement: Side }; - -const MARGIN = 8; -const MAX_HALF = 120; // half of the tooltip's max-w-[240px] — the widest it can get /** Wrap any element to show a short glass hint caption on hover — but only while the - * app-wide hints toggle (Settings → Appearance) is on. Rendered in a portal with - * fixed positioning so it is never clipped by an overflow/stacking ancestor. */ + * app-wide hints toggle (Settings → Appearance) is on. Anchored by the R8 popover platform, so it + * is never clipped by an overflow/stacking ancestor, flips when the preferred side runs out of room + * and slides back inside the viewport near an edge. */ export default function Tooltip({ hint, side = "top", @@ -21,74 +18,31 @@ export default function Tooltip({ children: React.ReactNode; }) { const enabled = useSyncExternalStore(subscribeHints, hintsEnabled, hintsEnabled); - const ref = useRef(null); - const tipRef = useRef(null); - const [coords, setCoords] = useState(null); - - function show() { - const el = ref.current; - if (!el) return; - const r = el.getBoundingClientRect(); - // Prefer the requested side; flip to bottom if there's no room above. - const placement: Side = side === "bottom" || r.top < 90 ? "bottom" : "top"; - const center = r.left + r.width / 2; - // Clamp using the MAX half-width so the caption can never overflow the viewport, even on the - // first paint (before we know its real width). A left-edge anchor near x=0 would otherwise push - // the centered box off-screen. The layout effect below refines this to the actual width. - setCoords({ - left: Math.min(Math.max(center, MAX_HALF + MARGIN), window.innerWidth - MAX_HALF - MARGIN), - top: placement === "top" ? r.top - 8 : r.bottom + 8, - placement, - }); - } - function hide() { - setCoords(null); - } - - // Once rendered, re-center on the anchor using the caption's ACTUAL width (a short hint doesn't - // need the full 240px reservation), still clamped inside the viewport. - useLayoutEffect(() => { - const tip = tipRef.current; - const el = ref.current; - if (!coords || !tip || !el) return; - const r = el.getBoundingClientRect(); - const half = tip.offsetWidth / 2; - const left = Math.min( - Math.max(r.left + r.width / 2, half + MARGIN), - window.innerWidth - half - MARGIN, - ); - if (Math.abs(left - coords.left) > 0.5) setCoords((c) => (c ? { ...c, left } : c)); - }, [coords]); + const [open, setOpen] = useState(false); + const pop = useAnchoredPopover({ + open: open && enabled && !!hint, + onOpenChange: setOpen, + mode: "hover", + placement: side, + offset: 8, + }); if (!enabled || !hint) return <>{children}; return ( - - {children} - {coords && - createPortal( -
- {hint} -
, - document.body, - )} - + <> + + {children} + + {/* Passive caption: it must not swallow a click aimed at a neighbouring control that it + overlaps while the hover-close delay runs out. */} + + {hint} + + ); } diff --git a/frontend/src/lib/fullscreen.ts b/frontend/src/lib/fullscreen.ts index 9dae2cfc..7b907665 100644 --- a/frontend/src/lib/fullscreen.ts +++ b/frontend/src/lib/fullscreen.ts @@ -1,3 +1,5 @@ +import { useSyncExternalStore } from "react"; + // The browser's Fullscreen API, WebKit included. // // Safari and iPadOS still ship only the prefixed spelling: no `fullscreenchange`, no @@ -47,3 +49,16 @@ export function onFullscreenChange(handler: () => void): () => void { document.removeEventListener("webkitfullscreenchange", handler); }; } + +// `fullscreenElement()` returns the live DOM node, so the same element is the same reference across +// calls — a valid useSyncExternalStore snapshot without any caching of our own. +const fsSnapshot = () => fullscreenElement() as HTMLElement | null; +const fsServerSnapshot = () => null; + +/** The fullscreen element as reactive state, for code that must RENDER differently while something + * holds the lock — chiefly the popover platform, which has to portal INTO that element (a node + * portaled to is painted underneath the fullscreen element, i.e. invisible). Components that + * merely react to the transition should keep using `onFullscreenChange` directly. */ +export function useFullscreenElement(): HTMLElement | null { + return useSyncExternalStore(onFullscreenChange, fsSnapshot, fsServerSnapshot); +} From 280de2e59229f0d5d1397d8911d6b467ba3aea70 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 31 Jul 2026 00:02:14 +0200 Subject: [PATCH 13/18] feat(popover): migrate the click menus onto the platform (R8 S4b) AddToPlaylist, LanguageSwitcher, the Plex DetailCustomizeMenu and ViewSwitcher each had their own anchoring, dismissal and (for ViewSwitcher) keyboard engine. All four now use the platform, so their items are PopoverItems with real menuitemradio/aria-checked, roving focus, typeahead and flip/shift anchoring. Two platform additions this needed: - `selectedIndex`: opening a single-choice menu must land on the choice already in effect, which is what ViewSwitcher's hand-rolled engine did by seeding its roving index to the current option. - `ariaLabel` on PopoverSurface: an icon-only trigger leaves the panel with no name. Fixed while wiring `selectedIndex` up: it stranded focus on the menu container. That path focuses its item synchronously in a layout effect, and FloatingFocusManager's own initial focus then ran afterwards and took focus back; the no-selectedIndex path only escaped this because it defers focus to a microtask while waiting for the item list to populate. On a roving list the list navigation owns initial focus, so the manager is now told to take none (initialFocus -1). Behaviour changes worth naming: - The header LanguageSwitcher no longer opens on hover. The platform focuses the first item on open, so a hover-open would yank focus off whatever the user was doing. Both variants are now click-only, like every other menu in the app. - PrefToggle became a PopoverItem, so the rows callers inject through the menu's `extra` prop join the same roving list as the built-in ones. - AddToPlaylist fetches its list on open, so on the first paint there are no items to focus and focus stays on the trigger (as it did before the migration); arrows move into the list once it lands. Verified in-browser: ViewSwitcher opens on the checked mode, arrows rove, Escape closes and refocuses the trigger; the ChannelTags pilot is unregressed; AddToPlaylist anchors bottom-start without the click reaching the feed card underneath; the rail language menu keeps its right-edge, bottom-aligned anchoring. --- frontend/src/components/AddToPlaylist.tsx | 181 ++++++++---------- frontend/src/components/AnchoredPopover.tsx | 18 +- frontend/src/components/LanguageSwitcher.tsx | 185 +++++++------------ frontend/src/components/ViewSwitcher.tsx | 151 +++++---------- frontend/src/lib/plexDetailUi.tsx | 65 +++---- 5 files changed, 240 insertions(+), 360 deletions(-) diff --git a/frontend/src/components/AddToPlaylist.tsx b/frontend/src/components/AddToPlaylist.tsx index 8d8fc581..9236233a 100644 --- a/frontend/src/components/AddToPlaylist.tsx +++ b/frontend/src/components/AddToPlaylist.tsx @@ -1,16 +1,17 @@ -import { useEffect, useLayoutEffect, useRef, useState } from "react"; -import { createPortal } from "react-dom"; +import { useState } from "react"; import { useTranslation } from "react-i18next"; import { useQuery, useQueryClient } from "@tanstack/react-query"; import { qk } from "../lib/queryKeys"; 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"; +import { PopoverItem, PopoverSurface, useAnchoredPopover } from "./AnchoredPopover"; -// 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. +// A small anchored menu for toggling a video's membership in the user's playlists, with inline +// "new playlist" creation. Same shape as the channel tag editor (R8 S1's pilot): radio-style +// toggles plus a free-text creator, so it follows that file's two conventions — typeahead OFF and +// the input swallowing non-Escape keys, since character and arrow keys belong to the text field. export default function AddToPlaylist({ videoId, className, @@ -21,60 +22,23 @@ export default function AddToPlaylist({ const { t } = useTranslation(); const qc = useQueryClient(); const fade = useScrollFade(); - const triggerRef = useRef(null); - const panelRef = useRef(null); const [open, setOpen] = useState(false); - const [coords, setCoords] = useState<{ top: number; left: number }>({ top: 0, left: 0 }); const [newName, setNewName] = useState(""); const [busy, setBusy] = useState(false); + const pop = useAnchoredPopover({ + open, + onOpenChange: setOpen, + offset: 6, + typeahead: false, + }); + const membership = useQuery({ queryKey: qk.playlistMembership(videoId), queryFn: () => api.playlists(videoId), enabled: open, }); - // Re-place once the panel is actually rendered (so we know its real height) and whenever - // its content — hence height — changes, so the flip-above decision uses the true size. - useLayoutEffect(() => { - if (open) place(); - }, [open, membership.data]); - - function place() { - const r = triggerRef.current?.getBoundingClientRect(); - if (!r) return; - const width = 240; - const margin = 8; - // Use the panel's real height once it's mounted; estimate on the first open. - const h = panelRef.current?.offsetHeight ?? 300; - let top = r.bottom + 6; - if (top + h > window.innerHeight - margin) { - // Not enough room below — open upward, clamped to the viewport. - top = Math.max(margin, r.top - h - 6); - } - const left = Math.min(Math.max(margin, r.left), window.innerWidth - width - margin); - setCoords({ top: Math.round(top), left: Math.round(left) }); - } - - function toggleOpen(e: React.MouseEvent) { - e.preventDefault(); - e.stopPropagation(); - if (!open) place(); - setOpen((o) => !o); - } - - useDismiss(open, () => setOpen(false), [panelRef, triggerRef]); - // Keep the panel anchored to the trigger as the page resizes/scrolls while open. - useEffect(() => { - if (!open) return; - window.addEventListener("resize", place); - window.addEventListener("scroll", place, true); - return () => { - window.removeEventListener("resize", place); - window.removeEventListener("scroll", place, true); - }; - }, [open]); - function refresh() { qc.invalidateQueries({ queryKey: qk.playlistMembership(videoId) }); qc.invalidateQueries({ queryKey: qk.playlists() }); @@ -95,8 +59,7 @@ export default function AddToPlaylist({ } } - async function createAndAdd(e: React.FormEvent) { - e.preventDefault(); + async function createAndAdd() { const name = newName.trim(); if (!name || busy) return; setBusy(true); @@ -117,8 +80,15 @@ export default function AddToPlaylist({ return ( <> - {open && - createPortal( -
e.stopPropagation()} - > -
{t("playlists.addToPlaylist")}
-
- {lists.length === 0 && !membership.isLoading && ( -
{t("playlists.noneYet")}
- )} - {lists.map((pl) => ( - - ))} -
-
- - setNewName(e.target.value)} - placeholder={t("playlists.newPlaylist")} - className="flex-1 min-w-0 bg-card border border-border rounded-md px-2 py-1 text-xs outline-none focus:border-accent" - /> - -
, - document.body, - )} + {pl.has_video && } + + {playlistName(pl, t)} + {(pl.source === "youtube" || pl.yt_playlist_id) && ( + + )} + + ))} +
+
+ + setNewName(e.target.value)} + onKeyDown={(e) => { + // Escape bubbles to the popover's dismiss; every other key is text editing here and + // must not reach the surrounding menu's list navigation. + if (e.key === "Escape") return; + e.stopPropagation(); + if (e.key === "Enter") { + e.preventDefault(); + void createAndAdd(); + } + }} + placeholder={t("playlists.newPlaylist")} + className="flex-1 min-w-0 bg-card border border-border rounded-md px-2 py-1 text-xs outline-none focus:border-accent" + /> +
+
+ ); } diff --git a/frontend/src/components/AnchoredPopover.tsx b/frontend/src/components/AnchoredPopover.tsx index d0ca9d77..2eeefef7 100644 --- a/frontend/src/components/AnchoredPopover.tsx +++ b/frontend/src/components/AnchoredPopover.tsx @@ -82,6 +82,10 @@ export interface AnchoredPopoverOptions { /** Type-to-focus a matching item by label (menu/listbox only). Default on; turn OFF for menus that * contain a free-text input, where character keys belong to the text field. */ typeahead?: boolean; + /** Index of the item that is currently CHOSEN, for a single-choice menu/listbox. Opening then + * lands the roving focus on it instead of the first item — the behaviour a "pick one of these + * modes" menu needs. Leave unset for action menus and multi-select toggle lists. */ + selectedIndex?: number | null; } export type AnchoredPopover = ReturnType; @@ -149,6 +153,7 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) { listRef: elementsRef, activeIndex, onNavigate: setActiveIndex, + selectedIndex: opts.selectedIndex ?? null, loop: true, // Focus the first item on open for BOTH click and keyboard (the default 'auto' skips pointer-opens, // which strands focus on the menu container — arrows then can't enter the roving list at all). @@ -273,11 +278,15 @@ export function PopoverSurface({ focusManaged, initialFocus, passthrough, + ariaLabel, }: { popover: AnchoredPopover; className?: string; style?: CSSProperties; children: ReactNode; + /** Accessible name for the panel itself — a menu whose trigger is icon-only has nothing else to + * take its name from. */ + ariaLabel?: string; focusManaged?: boolean; initialFocus?: number | React.MutableRefObject; /** The surface never receives pointer events — for a passive caption that must not swallow a click @@ -321,6 +330,7 @@ export function PopoverSurface({
{positioned} diff --git a/frontend/src/components/LanguageSwitcher.tsx b/frontend/src/components/LanguageSwitcher.tsx index 6ae3bc70..8f427c68 100644 --- a/frontend/src/components/LanguageSwitcher.tsx +++ b/frontend/src/components/LanguageSwitcher.tsx @@ -1,15 +1,16 @@ -import { useEffect, useRef, useState } from "react"; -import { createPortal } from "react-dom"; +import { useState } from "react"; import { Check, Globe } from "lucide-react"; import { LANGUAGES, type LangCode } from "../i18n"; +import { PopoverItem, PopoverSurface, useAnchoredPopover } from "./AnchoredPopover"; // Compact language picker (globe + current code). Presentational: the parent decides what // changing the language does (set it; persist server-side when signed in). // -// Two variants: "header" (legacy inline dropdown, opens below) and "rail" (used in the left -// nav's bottom icon cluster — an icon-only button whose menu is portaled to and -// anchored to the right + above the button, escaping the nav's backdrop-filter which would -// otherwise trap an absolutely-positioned popover). +// Two variants, now one implementation on the R8 popover platform: "header" opens below the pill +// (used on the landing page and the setup wizard) and "rail" is an icon-only button in the left +// nav's bottom cluster whose menu opens to the RIGHT, bottom-aligned. Both used to hand-roll their +// own anchoring, and the header one also opened on hover — dropped on purpose: the platform focuses +// the first item when a menu opens, so a hover-open would yank focus off whatever the user was doing. export default function LanguageSwitcher({ value, onChange, @@ -22,128 +23,68 @@ export default function LanguageSwitcher({ variant?: "header" | "rail"; }) { const [open, setOpen] = useState(false); - const closeTimer = useRef | null>(null); - const btnRef = useRef(null); - const panelRef = useRef(null); - const [pos, setPos] = useState<{ left: number; bottom: number }>({ left: 0, bottom: 0 }); - const current = LANGUAGES.find((l) => l.code === value) ?? LANGUAGES[0]; - - function openNow() { - if (closeTimer.current) clearTimeout(closeTimer.current); - setOpen(true); - } - function closeSoon() { - closeTimer.current = setTimeout(() => setOpen(false), 200); - } - function toggleRail() { - if (!open) { - const r = btnRef.current?.getBoundingClientRect(); - if (r) setPos({ left: r.right + 8, bottom: window.innerHeight - r.bottom }); - } - setOpen((o) => !o); - } - - // Rail popover: dismiss on outside click / Escape (it's portaled, so a contains() check - // spans both the button and the floating panel). - useEffect(() => { - if (variant !== "rail" || !open) return; - function onDoc(e: MouseEvent) { - const target = e.target as Node; - if (panelRef.current?.contains(target) || btnRef.current?.contains(target)) return; - setOpen(false); - } - function onKey(e: KeyboardEvent) { - if (e.key === "Escape") setOpen(false); - } - document.addEventListener("mousedown", onDoc); - document.addEventListener("keydown", onKey); - return () => { - document.removeEventListener("mousedown", onDoc); - document.removeEventListener("keydown", onKey); - }; - }, [variant, open]); - - const menu = ( -
- {LANGUAGES.map((l) => ( - - ))} -
+ const currentIdx = Math.max( + 0, + LANGUAGES.findIndex((l) => l.code === value), ); + const current = LANGUAGES.find((l) => l.code === value) ?? LANGUAGES[0]; + const rail = variant === "rail"; - if (variant === "rail") { - return ( - <> - - {open && - createPortal( -
- {menu} -
, - document.body, - )} - - ); - } - - return ( -
- - {open && ( -
- {LANGUAGES.map((l) => ( - - ))} -
- )} -
+ + {LANGUAGES.map((l) => ( + { + onChange(l.code); + setOpen(false); + }} + label={l.label} + className="w-full flex items-center justify-between gap-2 text-sm px-2 py-2 rounded-lg text-muted outline-none transition hover:text-fg hover:bg-card data-[active]:bg-[color-mix(in_srgb,var(--accent)_24%,transparent)] data-[active]:text-fg" + > + {l.label} + {l.code === value && } + + ))} + + ); } diff --git a/frontend/src/components/ViewSwitcher.tsx b/frontend/src/components/ViewSwitcher.tsx index 80a808e5..61d38d98 100644 --- a/frontend/src/components/ViewSwitcher.tsx +++ b/frontend/src/components/ViewSwitcher.tsx @@ -1,7 +1,7 @@ -import { useEffect, useRef, useState } from "react"; +import { useState } from "react"; import { Check, ChevronDown, type LucideIcon } from "lucide-react"; import clsx from "clsx"; -import { useDismiss } from "../lib/useDismiss"; +import { PopoverItem, PopoverSurface, useAnchoredPopover } from "./AnchoredPopover"; export interface ViewOption { id: T; @@ -13,9 +13,11 @@ export interface ViewOption { // menu behind it. Generic over the mode id so any module's list can adopt it (the feed's // density modes; the managers' layouts) without re-deriving the menu behaviour. // -// This is a real menu, so it carries the whole keyboard model the role promises: arrows/Home/End -// move a roving focus, Escape closes, and closing always hands focus back to the trigger (the -// menu unmounts under the focused item, which would otherwise strand focus on ). +// This was the house's textbook menu, with its own hand-rolled roving-tabindex/arrow/Escape engine. +// It now rides the R8 popover platform like every other menu, which supplies the same model +// (arrows/Home/End rove, Escape closes and hands focus back to the trigger, Tab out closes) plus the +// two things the hand-rolled version lacked: typeahead, and anchoring that flips and shifts instead +// of being absolutely positioned inside whatever ancestor happens to clip it. export default function ViewSwitcher({ value, options, @@ -34,19 +36,6 @@ export default function ViewSwitcher({ labelClassName?: string; }) { const [open, setOpen] = useState(false); - // Which item holds the roving tabindex; seeded to the active one when the menu opens. - const [focusIdx, setFocusIdx] = useState(0); - const btnRef = useRef(null); - const menuRef = useRef(null); - const itemRefs = useRef<(HTMLButtonElement | null)[]>([]); - - function close({ refocus = true }: { refocus?: boolean } = {}) { - setOpen(false); - if (refocus) btnRef.current?.focus(); - } - // Outside-click must NOT refocus the trigger — that would steal focus from whatever the user - // just clicked. Escape is handled on the menu itself (below), where returning focus is right. - useDismiss(open, () => close({ refocus: false }), [btnRef, menuRef]); // A `value` outside `options` is a caller bug; degrade to the first option rather than render // nothing. The menu then shows no checked item, which is honest — nothing IS selected. @@ -54,55 +43,24 @@ export default function ViewSwitcher({ 0, options.findIndex((o) => o.id === value), ); - // Seeding the roving index HERE — rather than in an effect keyed on `open` — matters: an effect - // would only schedule the update, so the focus effect below would still see the previous index - // and focus the wrong item for one render before correcting. - function openMenu() { - setFocusIdx(currentIdx); - setOpen(true); - } - // Clamped at render, so a caller whose option list shrinks while the menu is open can't leave - // the roving tabindex pointing past the end (which would make every item un-tabbable). - const rovingIdx = Math.min(focusIdx, options.length - 1); - useEffect(() => { - if (open) itemRefs.current[rovingIdx]?.focus(); - }, [open, rovingIdx]); + + const pop = useAnchoredPopover({ + open, + onOpenChange: setOpen, + placement: "bottom-end", + // Opening a "pick one of these modes" menu should land on the mode that is already picked. + selectedIndex: currentIdx, + }); const current = options[currentIdx]; if (!current) return null; const CurrentIcon = current.icon; - function onMenuKeyDown(e: React.KeyboardEvent) { - const last = options.length - 1; - if (e.key === "Escape") { - // Stop it reaching useDismiss's document listener, which would close without refocusing. - e.stopPropagation(); - close(); - } else if (e.key === "ArrowDown") { - e.preventDefault(); - setFocusIdx((i) => (i >= last ? 0 : i + 1)); - } else if (e.key === "ArrowUp") { - e.preventDefault(); - setFocusIdx((i) => (i <= 0 ? last : i - 1)); - } else if (e.key === "Home") { - e.preventDefault(); - setFocusIdx(0); - } else if (e.key === "End") { - e.preventDefault(); - setFocusIdx(last); - } else if (e.key === "Tab") { - // Tabbing out of a menu closes it, but the focus is moving on by itself. - close({ refocus: false }); - } - } - return ( -
+ <> - {open && ( -
- {/* The modes are one exclusive choice, so the radio items need a group to scope their - checked-ness to — otherwise they read as independent checkable items. */} -
- {options.map((o, i) => { - const Icon = o.icon; - const on = o.id === value; - return ( - - ); - })} -
+ + {/* The modes are one exclusive choice, so the radio items need a group to scope their + checked-ness to — otherwise they read as independent checkable items. */} +
+ {options.map((o) => { + const Icon = o.icon; + const on = o.id === value; + return ( + { + onChange(o.id); + setOpen(false); + }} + label={o.label} + className={`w-full flex items-center gap-2 px-2 py-1.5 rounded-lg text-sm text-left outline-none transition data-[active]:bg-[color-mix(in_srgb,var(--accent)_24%,transparent)] ${ + on ? "text-accent" : "text-fg hover:bg-card" + }`} + > + + {o.label} + {on && } + + ); + })}
- )} -
+ + ); } diff --git a/frontend/src/lib/plexDetailUi.tsx b/frontend/src/lib/plexDetailUi.tsx index 08e7f4a9..6c78ad2d 100644 --- a/frontend/src/lib/plexDetailUi.tsx +++ b/frontend/src/lib/plexDetailUi.tsx @@ -1,10 +1,10 @@ -import { useLayoutEffect, useRef, useState, type ReactNode } from "react"; +import { useLayoutEffect, useState, type ReactNode } from "react"; import { qk } from "./queryKeys"; import { useTranslation } from "react-i18next"; import { useQueryClient } from "@tanstack/react-query"; import { SlidersHorizontal } from "lucide-react"; -import { useDismiss } from "./useDismiss"; import { api } from "./api"; +import { PopoverItem, PopoverSurface, useAnchoredPopover } from "../components/AnchoredPopover"; // Shared "detail page" UI reused by the movie info page (PlexInfo) and the series show/season pages, // so they look consistent (the standing glassy/HTPC brief): the faint fixed art backdrop, the two @@ -131,43 +131,41 @@ export function DetailCustomizeMenu({ }) { const { t } = useTranslation(); const [open, setOpen] = useState(false); - const btnRef = useRef(null); - const menuRef = useRef(null); - useDismiss(open, () => setOpen(false), [menuRef, btnRef]); + const pop = useAnchoredPopover({ open, onOpenChange: setOpen, placement: "bottom-end" }); return ( -
+
- {open && ( -
- - {hasCast && ( - - )} - {hasRelated && onToggleRelated && ( - - )} - {extra} -
- )} + + + {hasCast && ( + + )} + {hasRelated && onToggleRelated && ( + + )} + {extra} +
); } +/** One switch row inside `DetailCustomizeMenu` — a `PopoverItem`, so the rows callers pass through + * `extra` join the same roving-focus list as the built-in ones. */ export function PrefToggle({ label, on, @@ -178,9 +176,12 @@ export function PrefToggle({ onClick: () => void; }) { return ( - + ); } From e35c4be56174ec9ac4df8828aba39e1cf15a2fa4 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 31 Jul 2026 00:11:26 +0200 Subject: [PATCH 14/18] feat(popover): migrate the dialog surfaces onto the platform (R8 S4c) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The column filter panel, the nav's account panel and the header's sync read-out are mixed content with their own tab order rather than lists of choices, so they migrate as `dialog` popovers — trapping and returning focus, dismissing through the shared layer stack instead of their own document listeners. The column filter needed a structural change: its open state lived in the header row while the panel rendered per column, which cannot work when the platform anchors a panel to its own trigger. Funnel and panel are now one module-level component (module-level still matters — declaring it inline once remounted the popover on every keystroke, which broke the caret and IME composition). That retires three hand-rolled focus chores at once: capturing the trigger element at open time because a per-column ref nulled out before focus could return, the effect that stopped Escape stranding focus on , and the useDismiss registration that had to include the trigger so its own mousedown did not close-then-reopen the panel. The filter also stops being one undifferentiated panel: a text filter is a dialog whose input takes initial focus, while the select and multi filters are real menus with roving focus — select as radios, multi as checkboxes. `menuitemcheckbox` is new to the platform, and the two existing multi-select toggle lists (channel tags, add-to-playlist) move onto it as well: several tags or playlists can be on at once, so they were never one exclusive choice. Role-only change, no interaction difference. Verified in-browser: all three anchor exactly where their hand-rolled placement did, Escape closes and returns focus to the trigger, the text filter keeps focus and caret position while typing narrows the table, and the multi filter opens as a checkbox menu with roving focus. --- frontend/src/components/AddToPlaylist.tsx | 3 +- frontend/src/components/AnchoredPopover.tsx | 7 +- frontend/src/components/ChannelTags.tsx | 3 +- frontend/src/components/NavSidebar.tsx | 199 +++++++--------- frontend/src/components/SyncStatus.tsx | 113 +++++---- frontend/src/components/tableHeader.tsx | 252 ++++++++++---------- 6 files changed, 279 insertions(+), 298 deletions(-) diff --git a/frontend/src/components/AddToPlaylist.tsx b/frontend/src/components/AddToPlaylist.tsx index 9236233a..e289f300 100644 --- a/frontend/src/components/AddToPlaylist.tsx +++ b/frontend/src/components/AddToPlaylist.tsx @@ -108,7 +108,8 @@ export default function AddToPlaylist({ {lists.map((pl) => ( toggle(pl)} disabled={busy} diff --git a/frontend/src/components/AnchoredPopover.tsx b/frontend/src/components/AnchoredPopover.tsx index 2eeefef7..bcbbc4ed 100644 --- a/frontend/src/components/AnchoredPopover.tsx +++ b/frontend/src/components/AnchoredPopover.tsx @@ -197,10 +197,11 @@ interface PopoverListContextValue { } const PopoverListContext = createContext(null); -type ItemRole = "menuitem" | "menuitemradio" | "option"; +type ItemRole = "menuitem" | "menuitemradio" | "menuitemcheckbox" | "option"; /** One keyboard-navigable option inside a menu/listbox `PopoverSurface`. Self-registers for roving - * focus + typeahead and applies the correct ARIA. Use `menuitemradio`+`checked` for a toggle, + * focus + typeahead and applies the correct ARIA. Use `menuitemradio`+`checked` when the items are + * one exclusive choice, `menuitemcheckbox`+`checked` when several can be on at once, * `option`+`selected` for a listbox choice, plain `menuitem` for an action. * * The active (roving) item carries `data-active` — style the highlight off THAT, not `:focus-visible`: @@ -237,7 +238,7 @@ export function PopoverItem({ const active = ctx?.activeIndex === index; const ariaProps = - role === "menuitemradio" + role === "menuitemradio" || role === "menuitemcheckbox" ? { role, "aria-checked": !!checked } : role === "option" ? { role, "aria-selected": !!selected } diff --git a/frontend/src/components/ChannelTags.tsx b/frontend/src/components/ChannelTags.tsx index 100dbaa4..e138bb3f 100644 --- a/frontend/src/components/ChannelTags.tsx +++ b/frontend/src/components/ChannelTags.tsx @@ -68,7 +68,8 @@ function TagEditorMenu({ return ( onToggle(tg.id)} label={tg.name} diff --git a/frontend/src/components/NavSidebar.tsx b/frontend/src/components/NavSidebar.tsx index d0ce865a..d9cac4c0 100644 --- a/frontend/src/components/NavSidebar.tsx +++ b/frontend/src/components/NavSidebar.tsx @@ -1,6 +1,5 @@ -import { useEffect, useRef, useState, useSyncExternalStore } from "react"; +import { useState, useSyncExternalStore } from "react"; import { qk } from "../lib/queryKeys"; -import { createPortal } from "react-dom"; import { useTranslation } from "react-i18next"; import { useQuery } from "@tanstack/react-query"; import { useNavigation, useNavigationActions } from "./NavigationProvider"; @@ -25,6 +24,7 @@ import { useHoverFocusWithin } from "../lib/useHoverFocusWithin"; import { type LangCode } from "../i18n"; import AvatarImg from "./Avatar"; import LanguageSwitcher from "./LanguageSwitcher"; +import { PopoverSurface, useAnchoredPopover } from "./AnchoredPopover"; // Primary app navigation: a collapsible left rail with icon+label entries (Design C). The // modules used to hide under the avatar dropdown; they now live here. Collapsed, it becomes @@ -57,41 +57,20 @@ export default function NavSidebar({ const expanded = pinned || peek.active; const slim = !expanded; // visual: icon-only rail const [acctOpen, setAcctOpen] = useState(false); - const acctBtnRef = useRef(null); - const acctPanelRef = useRef(null); - // Popover position (fixed, viewport coords). It's portaled to so it escapes the - // nav's backdrop-filter, which would otherwise trap fixed positioning + stacking and let - // clicks fall through to the controls behind it. - const [acctPos, setAcctPos] = useState<{ left: number; bottom: number }>({ left: 0, bottom: 0 }); - - function toggleAccount() { - if (!acctOpen) { - const r = acctBtnRef.current?.getBoundingClientRect(); - if (r) setAcctPos({ left: r.right + 8, bottom: window.innerHeight - r.bottom }); - } - setAcctOpen((o) => !o); - } - - // Dismiss on outside click / Escape via document listeners — NOT a full-screen backdrop - // div, which (sitting between the popover and the page) would break the popover's - // backdrop-filter and make it look solid. - useEffect(() => { - if (!acctOpen) return; - function onDoc(e: MouseEvent) { - const t = e.target as Node; - if (acctPanelRef.current?.contains(t) || acctBtnRef.current?.contains(t)) return; - setAcctOpen(false); - } - function onKey(e: KeyboardEvent) { - if (e.key === "Escape") setAcctOpen(false); - } - document.addEventListener("mousedown", onDoc); - document.addEventListener("keydown", onKey); - return () => { - document.removeEventListener("mousedown", onDoc); - document.removeEventListener("keydown", onKey); - }; - }, [acctOpen]); + // The account panel holds mixed content with its own tab order (profile, account switches, two + // actions), so it is a `dialog` popover, not a roving menu. It portals out of the nav — whose + // backdrop-filter would otherwise trap fixed positioning + stacking and let clicks fall through + // to the controls behind it — and anchors to the RIGHT of the button, bottom-aligned, as its + // hand-rolled placement did. Dismissal (outside press, topmost-only Escape, focus return) now + // comes from the platform instead of a pair of document listeners. + const acctPop = useAnchoredPopover({ + open: acctOpen, + onOpenChange: setAcctOpen, + mode: "dialog", + role: "dialog", + placement: "right-end", + offset: 8, + }); async function logout() { // Signs THIS tab's active account out of the browser wallet (server is per-tab aware); then @@ -338,10 +317,10 @@ export default function NavSidebar({ {!slim && {t("header.account.settings")}} -
+
- {acctOpen && - createPortal( -
-
- -
-
{name}
-
{me.email}
-
-
- {me.role === "admin" && ( -
- - {t("header.account.admin")} -
- )} - {otherAccounts.length > 0 && ( -
- {otherAccounts.map((a) => ( - - ))} -
- )} -
- - -
-
, - document.body, + +
+ +
+
{name}
+
{me.email}
+
+
+ {me.role === "admin" && ( +
+ + {t("header.account.admin")} +
)} + {otherAccounts.length > 0 && ( +
+ {otherAccounts.map((a) => ( + + ))} +
+ )} +
+ + +
+
diff --git a/frontend/src/components/SyncStatus.tsx b/frontend/src/components/SyncStatus.tsx index 19c0d28f..5e5095d5 100644 --- a/frontend/src/components/SyncStatus.tsx +++ b/frontend/src/components/SyncStatus.tsx @@ -1,12 +1,11 @@ -import { useRef, useState } from "react"; -import { createPortal } from "react-dom"; +import { useState } from "react"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { qk } from "../lib/queryKeys"; import { useTranslation } from "react-i18next"; import { CheckCircle2, Clock, Database, History, Loader2, Pause, Play } from "lucide-react"; import { api, type MyStatus } from "../lib/api"; import { formatViews } from "../lib/format"; -import { useDismiss } from "../lib/useDismiss"; +import { PopoverSurface, useAnchoredPopover } from "./AnchoredPopover"; // Per-user status (not the global catalog): shows the number of videos available to *this* // user, how many of *their* channels are still being fetched, and how many lack full @@ -21,12 +20,17 @@ export default function SyncStatus({ const { t } = useTranslation(); const qc = useQueryClient(); const [open, setOpen] = useState(false); - const chipRef = useRef(null); - const popRef = useRef(null); - // Fixed viewport coords: the popover is portaled out of the header, whose glass backdrop-filter - // would otherwise trap its stacking and positioning. - const [pos, setPos] = useState<{ top: number; right: number }>({ top: 0, right: 0 }); - useDismiss(open, () => setOpen(false), [chipRef, popRef]); + // A read-out with a couple of controls rather than a list of choices, so it is a `dialog` + // popover. It portals out of the header, whose glass backdrop-filter would otherwise trap its + // stacking and positioning. + const pop = useAnchoredPopover({ + open, + onOpenChange: setOpen, + mode: "dialog", + role: "dialog", + placement: "bottom-end", + offset: 8, + }); const { data } = useQuery({ queryKey: qk.myStatus(), queryFn: api.myStatus, @@ -106,16 +110,11 @@ export default function SyncStatus({ return (
- {open && - createPortal( -
-
- - - {formatViews(data.my_videos)}{" "} - {t("header.sync.yours")} - / - {formatViews(data.total_videos)} {t("header.sync.total")} - -
- {showMain && ( -
- {stateNode} - {pauseBtn} -
- )} - {notFull > 0 && ( -
- - {!showMain && pauseBtn} -
- )} -
, - document.body, + +
+ + + {formatViews(data.my_videos)}{" "} + {t("header.sync.yours")} + / + {formatViews(data.total_videos)} {t("header.sync.total")} + +
+ {showMain && ( +
+ {stateNode} + {pauseBtn} +
)} + {notFull > 0 && ( +
+ + {!showMain && pauseBtn} +
+ )} +
); } diff --git a/frontend/src/components/tableHeader.tsx b/frontend/src/components/tableHeader.tsx index 3b92bcd5..55507138 100644 --- a/frontend/src/components/tableHeader.tsx +++ b/frontend/src/components/tableHeader.tsx @@ -1,7 +1,7 @@ -import { useEffect, useRef, useState, type ReactNode, type RefObject } from "react"; +import { 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 { PopoverItem, PopoverSurface, useAnchoredPopover } from "./AnchoredPopover"; import { useScrollFade } from "../lib/useScrollFade"; import { type SortState } from "../lib/columnSort"; @@ -85,100 +85,145 @@ function FilterOptionList({ children }: { children: ReactNode }) { ); } -// 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 +// MODULE LEVEL, deliberately. The panel 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({ +// +// The funnel and its panel live in ONE component because the R8 popover platform anchors the panel +// to its trigger. That also retires this file's three hand-rolled focus chores: capturing the +// trigger element at open time (a `ref` scoped to the open column nulled out before focus could go +// back), the restore-focus effect that stopped Escape stranding focus on , and the useDismiss +// registration that had to include the trigger so its own mousedown didn't close-then-reopen. +function FilterFunnel({ col, value, onApply, - panelRef, + open, + onOpenChange, }: { col: Column; value: string | string[] | undefined; onApply: (key: string, value: string | string[]) => void; - panelRef: RefObject; + open: boolean; + onOpenChange: (open: boolean) => void; }) { const { t } = useTranslation(); const f = col.filter!; + const inputRef = useRef(null); + // A text filter is a free-text field, so it is a dialog whose input takes focus; the option lists + // are real menus with roving focus. `select` is one exclusive choice (radio), `multi` is several + // independent ones (checkbox). + const isText = f.kind === "text"; + const pop = useAnchoredPopover({ + open, + onOpenChange, + mode: isText ? "dialog" : "menu", + role: isText ? "dialog" : "menu", + }); + const filterOn = isActive(value); + 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 === "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" && ( +
+ onApply(col.key, "")} + label={t("datatable.all")} + className={`text-left text-xs px-2 py-1.5 rounded-md outline-none transition data-[active]:ring-1 data-[active]:ring-accent ${ + !filterOn ? "bg-accent text-accent-fg" : "text-muted hover:bg-card" }`} > - {o.label} - - ))} -
- )} - {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 ( -
+ )} + {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 ( + + onApply(col.key, on ? arr.filter((x) => x !== o.value) : [...arr, o.value]) + } + label={o.label} + className={`flex w-full items-center gap-2 text-left text-xs px-2 py-1.5 rounded-md outline-none transition data-[active]:bg-[color-mix(in_srgb,var(--accent)_24%,transparent)] ${ + on ? "text-fg" : "text-muted hover:bg-card" }`} > - {on && } - - {o.label} - - ); - })} - - )} - {isActive(value) && ( - - )} -
+ + {on && } + + {o.label} + + ); + })} + + )} + {filterOn && ( + + )} + + ); } @@ -203,33 +248,14 @@ export function ColumnHead({ filters: FilterMap; onFilter: (key: string, value: string | string[]) => void; }) { - const { t } = useTranslation(); + // Only one column's filter panel is open at a time; each funnel owns its own anchoring. const [openFilter, setOpenFilter] = useState(null); - const panelRef = useRef(null); - // The funnel that opened the panel. `useDismiss` must know about it too, or the trigger's own - // mousedown closes the panel and the click that follows immediately reopens it — clicking the - // funnel a second time then looks like it does nothing. Also the thing to hand focus back to. - const triggerRef = useRef(null); - const restoreFocus = useRef(false); - useDismiss(!!openFilter, () => setOpenFilter(null), [panelRef, triggerRef]); - - // Escape (or an outside click) used to unmount the focused input and strand focus on , - // so the next Tab restarted from the top of the page. - useEffect(() => { - if (openFilter) { - restoreFocus.current = true; - } else if (restoreFocus.current) { - restoreFocus.current = false; - triggerRef.current?.focus(); - } - }, [openFilter]); return ( {columns.map((col) => { const isSorted = sort?.key === col.key; - const filterOn = isActive(filters[col.key]); return ( ({ {col.header} )} {col.filter && ( - + setOpenFilter(o ? col.key : null)} + /> )} - {openFilter === col.key && col.filter && ( - - )} ); })} From 2727b59fa5f3eb924fc6652cc8cf9bc32e50dab6 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 31 Jul 2026 00:16:41 +0200 Subject: [PATCH 15/18] feat(popover): player description card on the platform, add-to-playlist works in fullscreen (R8 S4d) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The description hover card loses its two hand-managed timers and its fixed `bottom` anchor. It now opens above the title through the platform, which means `flip` can rescue it when there is no room up there either — the old fixed anchor could only grow upward. Its width moves from a px clamp to a rem one, so it follows the font-scale slider like the rest of the app. Add-to-playlist is no longer disabled in the player's true fullscreen: its menu rides the platform, which portals into the fullscreen element rather than . The download button stays hidden there on purpose — it opens a Modal, and Modal still portals to , so its dialog would open invisibly. Lifting that needs the same fullscreen-aware root inside Modal. NOT migrated, deliberately: PlexPlayer's gear and tracks panels. They are the one place where portalling would be a visual regression — the control bar sits inside the player's `transform: scale(1.25)` windowed subtree, so a panel portaled out of it renders at 80% of the surrounding UI's size. Positioning would still be correct (floating-ui reads post-transform viewport rects); the panel's own type and padding would not. Doing this properly means either portalling into that scaled wrapper or dropping the scale, and either way it needs a UAT with a playing video that this environment cannot run. Verified in-browser: the description card opens after its hover-intent delay, anchored 8px above the title and left-aligned to it, with the description loaded. --- frontend/src/components/PlayerModal.tsx | 121 +++++++++--------------- 1 file changed, 47 insertions(+), 74 deletions(-) diff --git a/frontend/src/components/PlayerModal.tsx b/frontend/src/components/PlayerModal.tsx index 4aa5bd09..b8c7d1dc 100644 --- a/frontend/src/components/PlayerModal.tsx +++ b/frontend/src/components/PlayerModal.tsx @@ -27,6 +27,7 @@ import { } from "lucide-react"; import Avatar from "./Avatar"; import AddToPlaylist from "./AddToPlaylist"; +import { PopoverSurface, useAnchoredPopover } from "./AnchoredPopover"; import { pushLayer, removeLayer, anyLayerAbove } from "../lib/layerStack"; import DownloadButton from "./DownloadButton"; import { api, type Video, type VideoStatus } from "../lib/api"; @@ -446,41 +447,21 @@ export default function PlayerModal({ } }; - // Lazy description (fetched only when the title is hovered). The popover is - // portaled to with fixed positioning so the modal card's overflow-y-auto - // can't clip it. A small close grace lets the mouse travel title → popover. + // Lazy description (fetched only when the title is hovered). Anchored ABOVE the title so it grows + // upward over the player rather than downward off-screen / behind the OS taskbar — `flip` still + // rescues it when there is no room up there either, which the old fixed `bottom` could not. The + // hover-intent delay (only pop it if the pointer lingers, so a quick pass doesn't flash the card) + // and the travel grace are now the platform's, along with the safePolygon bridge that replaces the + // pair of hand-managed timers. const [showDesc, setShowDesc] = useState(false); - // `bottom` anchors the popover just above the title so it grows upward (over the - // player) instead of downward off-screen / behind the OS taskbar. - const [descRect, setDescRect] = useState<{ left: number; bottom: number; width: number } | null>( - null, - ); - const titleRef = useRef(null); - const closeTimer = useRef(undefined); - const openTimer = useRef(undefined); - const openDescNow = () => { - window.clearTimeout(closeTimer.current); - window.clearTimeout(openTimer.current); - const el = titleRef.current; - if (el) { - const r = el.getBoundingClientRect(); - const width = Math.min(560, window.innerWidth - 32); - const left = Math.max(16, Math.min(r.left, window.innerWidth - width - 16)); - setDescRect({ left, bottom: window.innerHeight - r.top + 8, width }); - } - setShowDesc(true); - }; - // Hover-intent: only pop the description if the pointer lingers on the title (~400ms), so a - // quick pass over it doesn't flash the popover. - const scheduleOpenDesc = () => { - window.clearTimeout(closeTimer.current); - window.clearTimeout(openTimer.current); - openTimer.current = window.setTimeout(openDescNow, 400); - }; - const scheduleCloseDesc = () => { - window.clearTimeout(openTimer.current); - closeTimer.current = window.setTimeout(() => setShowDesc(false), 150); - }; + const descPop = useAnchoredPopover({ + open: showDesc, + onOpenChange: setShowDesc, + mode: "hover", + placement: "top-start", + offset: 8, + hoverDelay: { open: 400, close: 150 }, + }); const detail = useQuery({ queryKey: qk.videoDetail(currentVideoId), queryFn: () => api.videoDetail(currentVideoId), @@ -573,8 +554,6 @@ export default function PlayerModal({ return () => { window.removeEventListener("keydown", onKey); document.body.style.overflow = prevOverflow; - window.clearTimeout(closeTimer.current); - window.clearTimeout(openTimer.current); window.clearTimeout(volTimerRef.current); // Flush a debounced volume write — closing the player right after a wheel spin (the normal // way to leave) must not drop the level the user just set. @@ -960,12 +939,18 @@ export default function PlayerModal({ )} - {!navigated && !isFullscreen && ( + {/* Works in fullscreen now: its menu rides the popover platform, which portals into + the fullscreen element instead of (a body-portaled node is painted + underneath the element holding the lock, i.e. invisible). */} + {!navigated && ( )} + {/* Still hidden in fullscreen: this one opens a Modal, not a popover, and Modal + still portals to — so the dialog would open invisibly. Lifting that needs + the same fullscreen-aware root in Modal. */} {!navigated && !isFullscreen && ( {/* Hover target is the text itself (inline), not the whole row. */} {navigated ? (liveData?.title ?? t("player.loading")) : active.title} @@ -1146,43 +1130,32 @@ export default function PlayerModal({ {t("player.back")} )} - {showDesc && - descRect && - createPortal( + +
+ {t("player.description")} +
+ {detail.isLoading ? ( +
{t("player.loading")}
+ ) : detail.data?.description ? (
-
- {t("player.description")} -
- {detail.isLoading ? ( -
{t("player.loading")}
- ) : detail.data?.description ? ( -
- {renderDescription(detail.data.description, { - currentId: currentVideoId, - onSeek: seekTo, - onLoadVideo: loadVideo, - t, - })} -
- ) : ( -
{t("player.noDescription")}
- )} -
, - document.body, + {renderDescription(detail.data.description, { + currentId: currentVideoId, + onSeek: seekTo, + onLoadVideo: loadVideo, + t, + })} +
+ ) : ( +
{t("player.noDescription")}
)} + ); } -/** Renders the popover panel: the FloatingTree node + portal + the positioned surface. Merges the - * library's floating ref/props with the caller's className/style. Menu/listbox surfaces trap+return - * focus and host the list-navigation registry by default; pass `focusManaged={false}` to opt a - * surface out (e.g. a hover card). */ -export function PopoverSurface({ - popover, - className, - style, - children, - focusManaged, - initialFocus, - passthrough, - ariaLabel, -}: { +interface PopoverSurfaceProps { popover: AnchoredPopover; className?: string; style?: CSSProperties; @@ -294,7 +306,30 @@ export function PopoverSurface({ * aimed at whatever it happens to cover during its close delay. Interactive hover cards (scrollable * content, links) must NOT set this. */ passthrough?: boolean; -}) { +} + +/** Renders the popover panel: the FloatingTree node + portal + the positioned surface. Merges the + * library's floating ref/props with the caller's className/style. Menu/listbox surfaces trap+return + * focus and host the list-navigation registry by default; pass `focusManaged={false}` to opt a + * surface out (e.g. a hover card). + * + * A thin wrapper so the closed case runs NO hooks: a mounted-but-closed popover (there can be ~10 + * per table row) must not subscribe fullscreen listeners or build a portal container. */ +export function PopoverSurface(props: PopoverSurfaceProps) { + if (!props.popover.open) return null; + return ; +} + +function PopoverSurfaceInner({ + popover, + className, + style, + children, + focusManaged, + initialFocus, + passthrough, + ariaLabel, +}: PopoverSurfaceProps) { const { context, refs, @@ -304,23 +339,36 @@ export function PopoverSurface({ nodeId, referenceHidden, isPositioned, - open, mode, isNav, + selectedIndex, activeIndex, elementsRef, labelsRef, } = popover; + // A node portaled to is painted UNDERNEATH whatever holds the fullscreen lock, so a popover - // opened from a player's control bar in fullscreen would be invisible. Portal into the fullscreen - // element instead while one exists — this is what let the player's in-bar menus stop being either - // non-portaled (and clipped) or outright disabled in fullscreen. + // opened from a player's control bar in fullscreen would be invisible; it must portal INTO the + // fullscreen element. We own one persistent container div and re-parent it (body ↔ fullscreen + // element) with appendChild, which MOVES the node atomically — no detach, so the open panel's + // focus, caret, scroll offset and animation state survive a fullscreen transition, and an element + // SWAP (A→B without exiting) is handled by the element-identity dependency. (Keying the portal to + // remount instead would rebuild the subtree and lose all of that.) const fsRoot = useFullscreenElement(); + const [container] = useState(() => + typeof document === "undefined" ? null : document.createElement("div"), + ); + useLayoutEffect(() => { + if (container) (fsRoot ?? document.body).appendChild(container); + }, [container, fsRoot]); + useEffect(() => () => container?.remove(), [container]); + const { style: floatingPropStyle, ...floatingProps } = getFloatingProps() as { style?: CSSProperties; } & Record; - - if (!open) return null; + // useRole stamps `aria-labelledby` (the trigger) on a menu-role panel, and it WINS over aria-label + // in the accessible-name computation — so an explicit ariaLabel is dead unless we drop the ref. + if (ariaLabel) delete floatingProps["aria-labelledby"]; const zClass = mode === "hover" ? "z-tooltip" : "z-popover"; // Everything but a passive hover card manages focus by default (trap into the surface, return to @@ -368,22 +416,18 @@ export function PopoverSurface({ return ( - {/* `root` must be undefined (NOT null) to mean "default to " — floating-ui reads a null - root as "the container doesn't exist yet" and creates no portal node at all. And it only - builds that node once, so a root that changes mid-open is ignored; the key remounts the - portal on a fullscreen transition, which is exactly when the popover must move. */} - + {managed ? ( {positioned} diff --git a/frontend/src/components/ChannelPage.tsx b/frontend/src/components/ChannelPage.tsx index d898d2de..2b28dfe6 100644 --- a/frontend/src/components/ChannelPage.tsx +++ b/frontend/src/components/ChannelPage.tsx @@ -171,7 +171,13 @@ export default function ChannelPage({ // Personal tags, editable right here (the same control + flow the channel manager uses). Optimistic // on this channel's detail cache; a failed attach/detach reverts by refetching, and a success // invalidates the manager list so a tag added here shows there too. - const tagsQuery = useQuery({ queryKey: qk.tags(), queryFn: api.tags }); + // Only fetch the tag catalog when the editor will actually render (subscribed, non-demo) — an + // unsubscribed/demo channel page never shows the control, so the request would be thrown away. + const tagsQuery = useQuery({ + queryKey: qk.tags(), + queryFn: api.tags, + enabled: !!ch?.subscribed && !me.is_demo, + }); const userTags = (tagsQuery.data ?? []).filter((tg) => !tg.system); const { toggleTag, createAndAttachTag } = useChannelTagActions({ optimisticToggle: (id, tagId, attach) => @@ -441,6 +447,8 @@ export default function ChannelPage({ attachedIds={ch.tag_ids} onToggle={(tagId) => toggleTag(channelId, tagId, ch.tag_ids.includes(tagId))} onCreate={(name) => createAndAttachTag(channelId, name)} + // The header row has free width — show tags inline instead of collapsing to "+N". + bounded={false} /> )}
diff --git a/frontend/src/components/ChannelTags.tsx b/frontend/src/components/ChannelTags.tsx index e138bb3f..8abed9f4 100644 --- a/frontend/src/components/ChannelTags.tsx +++ b/frontend/src/components/ChannelTags.tsx @@ -3,6 +3,7 @@ import { Check, Plus } from "lucide-react"; import { useTranslation } from "react-i18next"; import { type Tag } from "../lib/api"; import { useAnchoredPopover, PopoverSurface, PopoverItem } from "./AnchoredPopover"; +import { useScrollFade } from "../lib/useScrollFade"; // The channel tag surface, shared by the channel manager (table + cards) and the channel detail // page. Built on the R8 popover platform. Two design goals it fixes over the old inline TagsCell: @@ -53,6 +54,7 @@ function TagEditorMenu({ }) { const { t } = useTranslation(); const [draft, setDraft] = useState(""); + const fade = useScrollFade(); const attached = new Set(attachedIds); const submit = () => { const name = draft.trim(); @@ -62,7 +64,7 @@ function TagEditorMenu({ }; return (
-
+
{userTags.map((tg) => { const on = attached.has(tg.id); return ( @@ -135,6 +137,7 @@ export function ChannelTagsControl({ onToggle, onCreate, onChipClick, + bounded = true, }: { userTags: Tag[]; attachedIds: number[]; @@ -142,6 +145,10 @@ export function ChannelTagsControl({ onCreate: (name: string) => void; /** Manager only: clicking a chip filters the feed by that tag. Omit on the detail page. */ onChipClick?: (tagId: number, name: string) => void; + /** Cap the chip row at a fixed 12rem so it can't widen a `table-layout:auto` column and reflow the + * table (the manager). Set false where there is free width (the channel detail header), so tags + * fit inline instead of needlessly collapsing into "+N". */ + bounded?: boolean; }) { const { t } = useTranslation(); const attached = userTags.filter((tg) => attachedIds.includes(tg.id)); @@ -174,6 +181,16 @@ export function ChannelTagsControl({ ? Math.max(0, pill.offsetLeft - (lastChip.offsetLeft + lastChip.offsetWidth)) : 0; const avail = row.clientWidth; + // Whole-set fast path: if the LAST chip's right edge already fits, every chip fits and no "+N" + // pill is needed. The reservation loop below reserves the pill's width for every chip except + // the last and breaks at the first overflow, so on its own it never reaches the only "does the + // whole set fit?" test — it would collapse chips that would have fitted (e.g. a short trailing + // chip narrower than the pill), leaving "+N" beside empty space. + const lastRight = lastChip ? lastChip.offsetLeft + lastChip.offsetWidth : 0; + if (lastRight <= avail) { + setVisible(chips.length); + return; + } let count = chips.length; for (let i = 0; i < chips.length; i++) { const chip = chips[i]; @@ -202,8 +219,9 @@ export function ChannelTagsControl({ onOpenChange: setEditorOpen, mode: "menu", placement: "bottom-start", - // The menu contains a free-text "create tag" input — type-to-focus would fight the text field. - typeahead: false, + // The menu contains a free-text "create tag" input — keep both typed chars and pointer-hover out + // of the roving list so neither steals the field's caret. + hostsInput: true, }); const [overflowOpen, setOverflowOpen] = useState(false); @@ -223,11 +241,14 @@ export function ChannelTagsControl({ "shrink-0 text-[10px] px-1.5 py-0.5 rounded-full bg-card text-muted border border-border"; return ( - // FIXED width so the cell's footprint is constant under `table-layout:auto`: a content-driven cell - // re-solves EVERY column's width as chips are added, visibly shifting the whole table. A fixed - // width keeps the table stable and centered; the chip row fits what it can and collapses the rest - // into "+N". (max-w-full lets a narrower card shrink it.) -
+ // When `bounded`, a FIXED width keeps the cell's footprint constant under `table-layout:auto`: a + // content-driven cell re-solves EVERY column's width as chips are added, visibly shifting the whole + // table. The fixed width keeps the table stable and centered; the chip row fits what it can and + // collapses the rest into "+N". (max-w-full lets a narrower card shrink it.) Unbounded, the row + // takes its container's width, so a wide header shows the tags inline instead of collapsing them. +
{/* Hidden measurer: all chips + a sample pill, laid out unbounded to read their true widths. */}
qc.invalidateQueries({ queryKey: qk.channel(id) }), onError: () => qc.invalidateQueries({ queryKey: qk.channels() }), }); const syncSubs = useMutation({ diff --git a/frontend/src/components/PlayerModal.tsx b/frontend/src/components/PlayerModal.tsx index b8c7d1dc..ed33fc3f 100644 --- a/frontend/src/components/PlayerModal.tsx +++ b/frontend/src/components/PlayerModal.tsx @@ -867,8 +867,9 @@ export default function PlayerModal({ bar — that stays reachable by hovering the video's bottom edge, just above ours. Always visible (no auto-hide) so there's no guessing whether it will appear and no fight with the seek bar over the same pixels. stopPropagation so a click here doesn't toggle playback; - the exit button also re-arms our shortcuts. Add-to-playlist / download are hidden in true - fullscreen — their popovers portal to , outside the fullscreen element. */} + the exit button also re-arms our shortcuts. Add-to-playlist works in true fullscreen (its + popover portals INTO the fullscreen element); download stays hidden there because it opens + a Modal, which still portals to — outside the fullscreen element. */} {(maximized || isFullscreen) && (
e.stopPropagation()} diff --git a/frontend/src/components/Tooltip.tsx b/frontend/src/components/Tooltip.tsx index 2695d474..2e6831d8 100644 --- a/frontend/src/components/Tooltip.tsx +++ b/frontend/src/components/Tooltip.tsx @@ -25,6 +25,9 @@ export default function Tooltip({ mode: "hover", placement: side, offset: 8, + // Show/hide on contact, as the pre-platform tooltip did — the platform's 120/140 ms hover-intent + // default suits interactive cards but makes a fast sweep across an icon row surface no hints. + hoverDelay: { open: 0, close: 0 }, }); if (!enabled || !hint) return <>{children}; diff --git a/frontend/src/lib/plexDetailUi.tsx b/frontend/src/lib/plexDetailUi.tsx index 6c78ad2d..838e5c06 100644 --- a/frontend/src/lib/plexDetailUi.tsx +++ b/frontend/src/lib/plexDetailUi.tsx @@ -1,10 +1,10 @@ -import { useLayoutEffect, useState, type ReactNode } from "react"; +import { useLayoutEffect, useRef, useState, type ReactNode } from "react"; import { qk } from "./queryKeys"; import { useTranslation } from "react-i18next"; import { useQueryClient } from "@tanstack/react-query"; import { SlidersHorizontal } from "lucide-react"; +import { useDismiss } from "./useDismiss"; import { api } from "./api"; -import { PopoverItem, PopoverSurface, useAnchoredPopover } from "../components/AnchoredPopover"; // Shared "detail page" UI reused by the movie info page (PlexInfo) and the series show/season pages, // so they look consistent (the standing glassy/HTPC brief): the faint fixed art backdrop, the two @@ -131,41 +131,51 @@ export function DetailCustomizeMenu({ }) { const { t } = useTranslation(); const [open, setOpen] = useState(false); - const pop = useAnchoredPopover({ open, onOpenChange: setOpen, placement: "bottom-end" }); + const btnRef = useRef(null); + const menuRef = useRef(null); + useDismiss(open, () => setOpen(false), [menuRef, btnRef]); + // Deliberately NOT on the R8 popover platform. In the player's OVERLAY variant this menu renders + // inside PlexPlayer's `transform: scale(1.25)` windowed subtree, and the platform portals its panel + // out to — which would paint it at 1× while the surrounding player UI is at 1.25×. A plain + // `absolute` child stays in the scaled subtree and matches, in both this variant and the unscaled + // detail-page variant. Same reason the player's own gear/tracks menus stay hand-rolled; migrating + // any of them needs the platform to portal INTO a caller-named (scaled) container. return ( -
+
- - - {hasCast && ( - - )} - {hasRelated && onToggleRelated && ( - - )} - {extra} - + {open && ( +
+ + {hasCast && ( + + )} + {hasRelated && onToggleRelated && ( + + )} + {extra} +
+ )}
); } -/** One switch row inside `DetailCustomizeMenu` — a `PopoverItem`, so the rows callers pass through - * `extra` join the same roving-focus list as the built-in ones. */ +/** One switch row inside `DetailCustomizeMenu`. Independent on/off toggles (all can be on at once), + * so a plain button, not a menuitemradio. */ export function PrefToggle({ label, on, @@ -176,12 +186,9 @@ export function PrefToggle({ onClick: () => void; }) { return ( - {label} - + ); } From a6e3ef81226a6197d1afab8070bafb405d72f207 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 31 Jul 2026 03:24:49 +0200 Subject: [PATCH 17/18] feat(player): download works in fullscreen + de-dup the transport controls (R8 S5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two R8 S5 (player big-view control-bar) follow-ups: - Modal now portals into `document.fullscreenElement ?? document.body` (reactive), the same fullscreen-aware root the popover platform uses. A node under is painted beneath whatever holds the fullscreen lock, so the download dialog — a Modal, opened from the player's control bar — would open invisibly in true fullscreen. With that fixed, the `!isFullscreen` guard on the player's DownloadButton is dropped, so download joins add-to-playlist as working in true fullscreen. (Every Modal is now fullscreen- aware; the fix is at the primitive, not the call site.) - The queue transport cluster (prev / counter / next + auto-advance + loop toggles) was duplicated inline in two places — the modal queue bar and the big-view docked control bar — with identical logic and only theme + label-visibility differing. Extracted a ``; a mode/icon change now lands once. Behaviour-preserving (the docked bar's trailing divider and the wrapper rows stay at the call sites). Note the modal variant keeps its `bg-accent/15` active tint, which is a latent no-op under Tailwind v3's hex --accent — left as-is for R10 S0's styling audit. Gate green (tsc/eslint/76 vitest). Device-UAT'd in real Chrome: transport controls render correctly in both the modal queue bar and the docked bar; the download button now shows in true fullscreen and its dialog portals into the fullscreen stage, fully visible and centered. No console errors. --- frontend/src/components/Modal.tsx | 14 +- frontend/src/components/PlayerModal.tsx | 250 ++++++++++++------------ 2 files changed, 137 insertions(+), 127 deletions(-) diff --git a/frontend/src/components/Modal.tsx b/frontend/src/components/Modal.tsx index 170e3130..e6738433 100644 --- a/frontend/src/components/Modal.tsx +++ b/frontend/src/components/Modal.tsx @@ -4,11 +4,14 @@ import { useTranslation } from "react-i18next"; import { X } from "lucide-react"; import { useBackToClose } from "../lib/history"; import { pushLayer, removeLayer, isTopmost } from "../lib/layerStack"; +import { useFullscreenElement } from "../lib/fullscreen"; -// Small centered modal shell (portaled to ): backdrop + ESC + scroll-lock close. Each open -// modal is a layer in the shared `layerStack` so ESC only closes the topmost one — e.g. an error -// dialog over the tag editor dismisses just the error and returns to the editor underneath, and a -// player below defers to it (R8 S3). +// Small centered modal shell: backdrop + ESC + scroll-lock close. Each open modal is a layer in the +// shared `layerStack` so ESC only closes the topmost one — e.g. an error dialog over the tag editor +// dismisses just the error and returns to the editor underneath, and a player below defers to it +// (R8 S3). Portals into `document.fullscreenElement ?? document.body`: a node under is painted +// beneath whatever holds the fullscreen lock, so a Modal opened from a player in true fullscreen (the +// download dialog) would otherwise open invisibly. export default function Modal({ title, onClose, @@ -23,6 +26,7 @@ export default function Modal({ const { t } = useTranslation(); // Browser/mouse Back closes the topmost modal instead of navigating away. useBackToClose(onClose); + const fsRoot = useFullscreenElement(); // Backdrop-click close must only fire when the press STARTED on the backdrop too. Otherwise a // drag that begins inside the dialog (e.g. selecting text in an input) and is released out on @@ -79,6 +83,6 @@ export default function Modal({
{children}
, - document.body, + fsRoot ?? document.body, ); } diff --git a/frontend/src/components/PlayerModal.tsx b/frontend/src/components/PlayerModal.tsx index ed33fc3f..b5cda7fb 100644 --- a/frontend/src/components/PlayerModal.tsx +++ b/frontend/src/components/PlayerModal.tsx @@ -68,6 +68,108 @@ type LoopMode = "off" | "one" | "all"; const AUTO_MODES: AutoMode[] = ["off", "next", "prev", "random"]; const LOOP_MODES: LoopMode[] = ["off", "one", "all"]; +/** The queue transport cluster — prev / counter / next, then the auto-advance and loop mode toggles. + * Rendered in two places with identical logic: the modal's queue bar (`variant:"modal"`, normal glass + * theme, labels always shown) and the big-view docked control bar (`variant:"docked"`, white-on-dark, + * labels hidden until a breakpoint). Only the theme + label-visibility differ; the behaviour is one, + * so a mode/icon change lands once. The wrapper (fragment vs. bordered row) stays at the call site. */ +function PlayerTransportControls({ + variant, + index, + queueLength, + autoMode, + loopMode, + onPrev, + onNext, + onCycleAuto, + onCycleLoop, +}: { + variant: "docked" | "modal"; + index: number; + queueLength: number; + autoMode: AutoMode; + loopMode: LoopMode; + onPrev: () => void; + onNext: () => void; + onCycleAuto: () => void; + onCycleLoop: () => void; +}) { + const { t } = useTranslation(); + const docked = variant === "docked"; + const navBtn = docked + ? "inline-flex items-center gap-1 text-white/80 enabled:hover:text-white disabled:opacity-30 transition" + : "inline-flex items-center gap-1.5 text-muted enabled:hover:text-fg disabled:opacity-40 transition"; + const navLabel = docked ? "hidden sm:inline" : ""; + const counter = docked ? "tabular-nums text-white/70" : "text-muted tabular-nums"; + const divider = docked ? "h-4 w-px bg-white/20" : "w-px h-4 bg-border"; + const toggle = (on: boolean) => + docked + ? `inline-flex items-center gap-1 rounded-lg px-2 py-1 transition ${ + on ? "bg-white/20 text-white" : "text-white/70 hover:bg-white/10 hover:text-white" + }` + : `inline-flex items-center gap-1.5 rounded-lg px-2.5 py-1 transition ${ + on ? "bg-accent/15 text-accent" : "text-muted hover:bg-surface hover:text-fg" + }`; + const toggleLabel = docked ? "hidden md:inline" : ""; + return ( + <> + + + {index + 1} / {queueLength} + + + + {/* Persistent playback settings — cycle on click, saved to your account. */} + + + + ); +} + /** Can `el` (or an ancestor inside the modal) still scroll vertically in `dir` (-1 up, 1 down)? * Used to leave ArrowUp/ArrowDown alone while there is content to scroll — the volume shortcut * must not eat the only keyboard way to read a long description. Stops at ``: the page @@ -877,82 +979,30 @@ export default function PlayerModal({ > {hasQueue && ( <> - - - {index + 1} / {queue!.length} - - - - - + )} - {/* Works in fullscreen now: its menu rides the popover platform, which portals into - the fullscreen element instead of (a body-portaled node is painted - underneath the element holding the lock, i.e. invisible). */} + {/* Both work in fullscreen now: add-to-playlist's popover and download's Modal both + portal into the fullscreen element rather than (a body-portaled node is + painted underneath the element holding the lock, i.e. invisible). */} {!navigated && ( )} - {/* Still hidden in fullscreen: this one opens a Modal, not a popover, and Modal - still portals to — so the dialog would open invisibly. Lifting that needs - the same fullscreen-aware root in Modal. */} - {!navigated && !isFullscreen && ( + {!navigated && ( - - - {index + 1} / {queue!.length} - - - - {/* Persistent playback settings — cycle on click, saved to your account. */} - - +
)} From 7b6481055cfd4121c4752f7d5eb6bdbc94e354b4 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 31 Jul 2026 03:43:41 +0200 Subject: [PATCH 18/18] chore(popover): drop the unused AnchoredPopover type export (knip) --- frontend/src/components/AnchoredPopover.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frontend/src/components/AnchoredPopover.tsx b/frontend/src/components/AnchoredPopover.tsx index 54648942..d77bd9f2 100644 --- a/frontend/src/components/AnchoredPopover.tsx +++ b/frontend/src/components/AnchoredPopover.tsx @@ -93,7 +93,8 @@ export interface AnchoredPopoverOptions { selectedIndex?: number | null; } -export type AnchoredPopover = ReturnType; +// Internal: call sites use the inferred hook return, they don't name the type. +type AnchoredPopover = ReturnType; export function useAnchoredPopover(opts: AnchoredPopoverOptions) { const mode = opts.mode ?? "menu";