fix(ui): a sticky table header turns the scroller's top fade off itself; DataTable renders one layout

- "A page with a sticky ColumnHead must declare fadeTop: false" lived in
  prose: a registry flag on three pages plus an App special case for the
  Channels card layout, and the next table page would have re-acquired the
  fade-on-the-header defect with every lane green. ColumnHead now registers
  itself with the enclosing PageScroller (ui/stickyTop) while mounted; the
  registry flag, the App branch and the prop plumbing are gone. Measured on
  localdev: the top mask is 0px under Channels' and Audit's headers, 20px on
  Home and the Feed.
- ui/DataTable mounted its table AND its card list at every width, one
  hidden by CSS: double DOM and render cost, duplicate ids, two keyboard
  copies of every row. It now mounts the one the `md` media query calls for
  (lib/useMediaQuery); the keyboard test asserts one layout at each width.
- The Plex module's content width went px → rem with the rest of the page.
This commit is contained in:
2026-09-19 07:51:33 +02:00
parent 8018cd4685
commit 6fda49369f
10 changed files with 225 additions and 110 deletions
+1 -10
View File
@@ -20,7 +20,6 @@ import { usePref } from "./lib/usePref";
import { useVerifyEntry } from "./lib/useVerifyEntry";
import { useNavigation, useNavigationActions } from "./components/navigationContext";
import { useFeedFilters, useFeedFiltersActions } from "./components/feedFiltersContext";
import { useChannelLayout } from "./components/channelsContext";
import { useFeedView, useFeedViewActions } from "./components/feedViewContext";
import { moduleDef, moduleOrder } from "./lib/modules";
import { PAGE_CONTENT, PAGE_RAIL, preloadPages } from "./components/moduleRegistry";
@@ -71,11 +70,6 @@ export default function App() {
// handlers that mutate the feed view (onShowInFeed, tag-click, the feed scrollKey).
const filters = useFeedFilters();
const { setFilters } = useFeedFiltersActions();
// The channel manager turns its top fade OFF for the table (its sticky header sits at the scroller
// top) but wants it ON for the card layout (no sticky header) — so the fade depends on the layout.
// Deliberately the NARROW layout context: subscribing to the manager's whole state here would
// re-render the app root on every keystroke in its search box.
const channelLayout = useChannelLayout();
// The feed's view mode (card/row/tile density) lives in FeedViewProvider — server-synced, shared
// by the feed page and the channel page. App reads it only to hand to <Feed>/<ChannelPage>.
const view = useFeedView();
@@ -325,7 +319,7 @@ export default function App() {
const me = meQuery.data!;
const pageGate = moduleDef(page).gate;
const activePage: Page = !pageGate || pageGate(me) ? page : "feed";
const { Component: PageContent, fadeTop, maxWidth } = PAGE_CONTENT[activePage];
const { Component: PageContent, maxWidth } = PAGE_CONTENT[activePage];
const railDef = channelView ? undefined : PAGE_RAIL[activePage];
return (
@@ -392,9 +386,6 @@ export default function App() {
<PageShell
scrollKey={normalScrollKey}
maxWidth={maxWidth}
// Channels: the registry disables the top fade for its sticky-header table, but the card
// layout has no sticky header, so re-enable it there.
fadeTop={activePage === "channels" ? channelLayout === "cards" : (fadeTop ?? true)}
header={<Header me={meQuery.data!} onYtSearch={enterYtSearch} />}
banners={
<>
+4 -7
View File
@@ -39,9 +39,6 @@ export type PageDef = {
// Prop-free by construction: reads `me` + module state from hooks/providers. Preloadable by type, so
// a new page cannot silently opt out of preloadPages.
Component: PreloadableComponent<object>;
// Channels + Audit render a table with a sticky header at the scroller top; the shell's top fade
// would sit on it and flip on with the first scroll, so they turn it off. Defaults to on.
fadeTop?: boolean;
// The content column's max width. The header + toolbar centre on this same width (via --content-mw)
// so the floating chrome lines up with the centred content instead of spanning the whole column.
// MUST match the max-w the page's own content wrapper uses. Omit for a full-width page (feed).
@@ -53,9 +50,9 @@ export const PAGE_CONTENT: Record<Page, PageDef> = {
home: { Component: Home, maxWidth: "80rem" },
// The two wrappers preload the lazy chunk they render.
feed: { Component: Object.assign(FeedPage, { preload: Feed.preload }) },
channels: { Component: Channels, fadeTop: false, maxWidth: "96rem" },
playlists: { Component: Playlists, fadeTop: false, maxWidth: "64rem" },
plex: { Component: PlexBrowse, maxWidth: "1600px" },
channels: { Component: Channels, maxWidth: "96rem" },
playlists: { Component: Playlists, maxWidth: "64rem" },
plex: { Component: PlexBrowse, maxWidth: "100rem" },
notifications: { Component: NotificationsPanel, maxWidth: "48rem" },
messages: { Component: Object.assign(MessagesPage, { preload: Messages.preload }) },
downloads: { Component: DownloadCenter, maxWidth: "56rem" },
@@ -63,7 +60,7 @@ export const PAGE_CONTENT: Record<Page, PageDef> = {
scheduler: { Component: Scheduler },
config: { Component: ConfigPanel },
users: { Component: AdminUsers, maxWidth: "48rem" },
audit: { Component: AuditLog, fadeTop: false },
audit: { Component: AuditLog },
logs: { Component: AdminLogs },
settings: { Component: SettingsPanel },
};
@@ -1,9 +1,9 @@
// @vitest-environment jsdom
//
// DataTable's `keyboardNav` (E11 S4c): the rows are a keyboard list — `j`/`k` walk them and Enter
// clicks the element a column marks `data-nav-open`. The table renders its rows twice (a table and,
// below `md`, a card list); only the copy on screen may be walked, or `j` past the last row would carry
// on into the hidden one.
// clicks the element a column marks `data-nav-open`. It used to render its rows twice (a table and,
// below `md`, a card list, one hidden by CSS), so `j` past the last row could carry on into the hidden
// copy. Now it mounts exactly one layout, chosen by the `md` media query.
import { afterEach, describe, expect, it, vi } from "vitest";
import { act, cleanup, render } from "@testing-library/react";
import DataTable, { type Column } from "./DataTable";
@@ -13,7 +13,10 @@ vi.mock("react-i18next", async (importOriginal) => {
return { ...actual, useTranslation: () => ({ t: (key: string) => key }) };
});
afterEach(cleanup);
afterEach(() => {
cleanup();
vi.unstubAllGlobals();
});
type Row = { id: string };
const rows: Row[] = [{ id: "a" }, { id: "b" }];
@@ -45,9 +48,15 @@ function press(key: string) {
});
}
/** What a display:none copy answers in a browser; jsdom lays nothing out, so every copy is "visible". */
const offScreen = (el: Element | null) =>
Object.defineProperty(el!, "checkVisibility", { value: () => false, configurable: true });
/** The viewport `md` question, answered as asked (jsdom has no matchMedia of its own). */
function viewport(wide: boolean) {
vi.stubGlobal("matchMedia", (query: string) => ({
matches: query === "(min-width: 48rem)" ? wide : false,
media: query,
addEventListener: () => {},
removeEventListener: () => {},
}));
}
/** The focused row: which copy (TR / DIV) and which row. */
const focused = () => {
@@ -58,26 +67,38 @@ const focused = () => {
describe("DataTable · keyboardNav", () => {
it("walks the table's rows, stops at the last one, and opens a row on Enter", () => {
const opened: string[] = [];
const { container } = table((id) => opened.push(id));
offScreen(container.querySelector(".md\\:hidden"));
viewport(true);
table((id) => opened.push(id));
press("j");
expect(focused()).toBe("TR a");
press("j");
expect(focused()).toBe("TR b");
// The hidden card copy follows the table in the DOM — it must not be the next stop.
// The last row is the last stop — there is no second copy after it to walk into.
press("j");
expect(focused()).toBe("TR b");
press("Enter");
expect(opened).toEqual(["b"]);
});
it("walks the card copy when that is the one on screen", () => {
const { container } = table(() => {});
offScreen(container.querySelector("tbody"));
it("walks the card list when the viewport is narrow", () => {
viewport(false);
table(() => {});
press("j");
expect(focused()).toBe("DIV a");
});
it("mounts one layout, never both", () => {
viewport(true);
const wide = table(() => {});
expect(wide.container.querySelectorAll("[data-nav-index]")).toHaveLength(2);
expect(wide.container.querySelector("table")).not.toBeNull();
cleanup();
viewport(false);
const narrow = table(() => {});
expect(narrow.container.querySelectorAll("[data-nav-index]")).toHaveLength(2);
expect(narrow.container.querySelector("table")).toBeNull();
});
it("leaves the rows out of the keyboard without keyboardNav", () => {
const { container } = table(() => {}, false);
expect(container.querySelector("[data-nav-index]")).toBeNull();
+74 -67
View File
@@ -8,6 +8,7 @@ import { ColumnHead } from "./tableHeader";
import { alignClass } from "./tableAlign";
import { filterRows, type Column, type FilterMap } from "../../lib/tableColumns";
import { navItemAttrs, useListNav } from "../../lib/listNav";
import { useMediaQuery } from "../../lib/useMediaQuery";
// A reusable, client-side data table: per-column sort + filter (in the header) + pagination,
// with its sort/filter/page state optionally persisted to localStorage so a reload (F5) keeps
@@ -134,6 +135,7 @@ export default function DataTable<T>({
const filters = controlledFilters ? (filtersProp ?? EMPTY_FILTERS) : internalFilters;
const [page, setPage] = useState(initial.page);
const [size, setSize] = useState(initial.size ?? pageSize);
const wide = useMediaQuery("(min-width: 48rem)");
const bodyRef = useRef<HTMLTableSectionElement>(null);
const cardsRef = useRef<HTMLDivElement>(null);
useListNav(bodyRef, { enabled: keyboardNav });
@@ -249,83 +251,88 @@ export default function DataTable<T>({
return (
<div>
{(controlsPosition === "top" || controlsPosition === "both") && topControls}
{/* Wide screens: table. The wrapper isn't clipped so header filter popovers can overflow. */}
<div className="hidden md:block">
<table className={`w-full border-collapse text-sm ${fixedLayout ? "table-fixed" : ""}`}>
{/* In fixed-layout mode a colgroup drives the column widths: those with an explicit
{/* ONE layout, chosen by width — not both mounted with CSS hiding one: that doubled every
row's DOM and render cost, duplicated ids and test hooks, and handed keyboard nav and any
DOM query two copies of each row. `md` (48rem) is the breakpoint the CSS used.
Wide screens: table. The wrapper isn't clipped so header filter popovers can overflow. */}
{wide ? (
<div>
<table className={`w-full border-collapse text-sm ${fixedLayout ? "table-fixed" : ""}`}>
{/* In fixed-layout mode a colgroup drives the column widths: those with an explicit
`width` keep it, and the unspecified (fluid) columns split the remainder — so the
table never exceeds its container and the fluid cells truncate instead. */}
{fixedLayout && (
<colgroup>
{columns.map((col) => (
<col key={col.key} style={col.width ? { width: col.width } : undefined} />
))}
</colgroup>
)}
<ColumnHead
columns={columns}
sort={sort}
onToggleSort={toggleSort}
filters={filters}
onFilter={applyFilter}
/>
<tbody ref={bodyRef}>
{paged.map((row, i) => (
<tr
key={rowKey(row)}
{...rowProps?.(row)}
{...(keyboardNav && navItemAttrs(i))}
className={`border-b border-border/50 hover:bg-(--hover) transition ${
rowClassName?.(row) ?? ""
}`}
>
{fixedLayout && (
<colgroup>
{columns.map((col) => (
<td
key={col.key}
style={col.width ? { width: col.width } : undefined}
className={`px-2 py-2 align-middle ${col.nowrap ? "whitespace-nowrap" : ""} ${alignClass(col.align)}`}
>
{col.render(row)}
</td>
<col key={col.key} style={col.width ? { width: col.width } : undefined} />
))}
</tr>
))}
</tbody>
</table>
</div>
{/* Narrow screens: a compact card per row — the primary column as the heading, the
rest flowing in a single wrapping meta line (no full-width label/value gaps). */}
<div ref={cardsRef} className="md:hidden flex flex-col gap-2">
{paged.map((row, i) => (
<div
key={rowKey(row)}
{...rowProps?.(row)}
{...(keyboardNav && navItemAttrs(i))}
className={`glass-card rounded-xl p-3 ${rowClassName?.(row) ?? ""}`}
>
{columns
.filter((c) => c.cardPrimary)
.map((col) => (
<div key={col.key} className="font-medium">
{col.render(row)}
</div>
</colgroup>
)}
<ColumnHead
columns={columns}
sort={sort}
onToggleSort={toggleSort}
filters={filters}
onFilter={applyFilter}
/>
<tbody ref={bodyRef}>
{paged.map((row, i) => (
<tr
key={rowKey(row)}
{...rowProps?.(row)}
{...(keyboardNav && navItemAttrs(i))}
className={`border-b border-border/50 hover:bg-(--hover) transition ${
rowClassName?.(row) ?? ""
}`}
>
{columns.map((col) => (
<td
key={col.key}
style={col.width ? { width: col.width } : undefined}
className={`px-2 py-2 align-middle ${col.nowrap ? "whitespace-nowrap" : ""} ${alignClass(col.align)}`}
>
{col.render(row)}
</td>
))}
</tr>
))}
<div className="flex flex-wrap items-center gap-x-3 gap-y-1.5 mt-2 text-sm">
</tbody>
</table>
</div>
) : (
/* Narrow screens: a compact card per row — the primary column as the heading, the
rest flowing in a single wrapping meta line (no full-width label/value gaps). */
<div ref={cardsRef} className="flex flex-col gap-2">
{paged.map((row, i) => (
<div
key={rowKey(row)}
{...rowProps?.(row)}
{...(keyboardNav && navItemAttrs(i))}
className={`glass-card rounded-xl p-3 ${rowClassName?.(row) ?? ""}`}
>
{columns
.filter((c) => !c.hideInCard && !c.cardPrimary)
.filter((c) => c.cardPrimary)
.map((col) => (
<span key={col.key} className="inline-flex items-center gap-1">
{col.cardLabel !== false && (
<span className="text-muted text-xs">{col.header}</span>
)}
<div key={col.key} className="font-medium">
{col.render(row)}
</span>
</div>
))}
<div className="flex flex-wrap items-center gap-x-3 gap-y-1.5 mt-2 text-sm">
{columns
.filter((c) => !c.hideInCard && !c.cardPrimary)
.map((col) => (
<span key={col.key} className="inline-flex items-center gap-1">
{col.cardLabel !== false && (
<span className="text-muted text-xs">{col.header}</span>
)}
{col.render(row)}
</span>
))}
</div>
</div>
</div>
))}
</div>
))}
</div>
)}
{paged.length === 0 && (
<div className="text-muted py-8 text-center text-sm">
+11 -6
View File
@@ -1,5 +1,6 @@
import { useCallback, useLayoutEffect, useRef, type ReactNode } from "react";
import { useCallback, useLayoutEffect, useRef, useState, type ReactNode } from "react";
import { useScrollFade } from "../../lib/useScrollFade";
import { StickyTopProvider } from "./stickyTop";
import { ensureNavId, peekNavId, subscribeNav } from "../../lib/history";
// Saved scroll offsets, keyed by `${historyEntryId}:${scrollKey}`. The entry id ties an offset to the
@@ -78,14 +79,18 @@ function restoreScroll(el: HTMLElement, top: number): () => void {
export default function PageScroller({
children,
scrollKey,
fadeTop = true,
}: {
children: ReactNode;
scrollKey?: string;
// Off when the scroller's top is a sticky element (a table's sticky header) — see useScrollFade.
fadeTop?: boolean;
}) {
const fade = useScrollFade(20, fadeTop);
// How many sticky elements sit at the scroller's top right now (see ./stickyTop): the top fade is
// off while any is mounted — a table layout — and back on the moment none is (a card layout).
const [stickyTops, setStickyTops] = useState(0);
const registerStickyTop = useCallback(() => {
setStickyTops((n) => n + 1);
return () => setStickyTops((n) => n - 1);
}, []);
const fade = useScrollFade(20, stickyTops === 0);
const fadeRef = fade.ref;
const elRef = useRef<HTMLElement | null>(null);
// Stable identity: an inline ref callback changes every render, which makes React detach and
@@ -151,7 +156,7 @@ export default function PageScroller({
style={fade.style}
className="flex-1 min-w-0 overflow-y-auto scrollbar-gutter-stable"
>
{children}
<StickyTopProvider value={registerStickyTop}>{children}</StickyTopProvider>
</main>
);
}
+1 -6
View File
@@ -45,7 +45,6 @@ export default function PageShell({
banners,
children,
scrollKey,
fadeTop = true,
maxWidth,
}: {
header?: ReactNode;
@@ -53,8 +52,6 @@ export default function PageShell({
children: ReactNode;
// Identifies the logical view for the scroll restore/reset policy — see PageScroller.
scrollKey?: string;
// Off when this page's scroll region starts with a sticky header — see PageScroller.
fadeTop?: boolean;
// The content column's max width (e.g. "1600px"); the header + toolbar centre on the same width via
// --content-mw. Omit for a full-width page (feed). See index.css --content-mw.
maxWidth?: string;
@@ -145,9 +142,7 @@ export default function PageShell({
{/* pr-[--sbw]: the toolbar band isn't the scroller, so without this it would run the
scrollbar-gutter's width past the scrolled cards on the right. Pad it back to card width. */}
<div ref={setToolbarSlot} className="shrink-0 pr-[var(--sbw)]" />
<PageScroller scrollKey={scrollKey} fadeTop={fadeTop}>
{children}
</PageScroller>
<PageScroller scrollKey={scrollKey}>{children}</PageScroller>
</div>
</PageToolbarSlot.Provider>
);
@@ -0,0 +1,62 @@
// @vitest-environment jsdom
//
// The page scroller's top fade sits ON a sticky table header, so a page with one used to opt out by
// hand (`fadeTop: false` in the module registry, plus an App special case for the Channels card
// layout) — and the next table page would have re-acquired the defect with every lane green. The
// header now says so itself: while a `ColumnHead` is mounted the scroller asks for no top fade.
import { afterEach, describe, expect, it, vi } from "vitest";
import { cleanup, render } from "@testing-library/react";
const fadeTopArgs: boolean[] = [];
vi.mock("../../lib/useScrollFade", () => ({
useScrollFade: (_px: number, fadeTop: boolean) => {
fadeTopArgs.push(fadeTop);
return { ref: () => {}, style: {} };
},
}));
vi.mock("react-i18next", async (importOriginal) => {
const actual = await importOriginal<typeof import("react-i18next")>();
return { ...actual, useTranslation: () => ({ t: (key: string) => key }) };
});
import PageScroller from "./PageScroller";
import { ColumnHead } from "./tableHeader";
afterEach(() => {
cleanup();
fadeTopArgs.length = 0;
});
function page(withTable: boolean) {
return (
<PageScroller scrollKey="k">
{withTable ? (
<table>
<ColumnHead
columns={[]}
sort={null}
onToggleSort={() => {}}
filters={{}}
onFilter={() => {}}
/>
</table>
) : (
<div>cards</div>
)}
</PageScroller>
);
}
describe("the page scroller's top fade and a sticky table header", () => {
it("is off while the header is mounted, and back on the moment it is gone", () => {
const view = render(page(true));
expect(fadeTopArgs.at(-1)).toBe(false);
view.rerender(page(false)); // e.g. the Channels card layout, or a narrow DataTable
expect(fadeTopArgs.at(-1)).toBe(true);
});
it("is on for a page with no sticky header at all", () => {
render(page(false));
expect(fadeTopArgs.at(-1)).toBe(true);
});
});
+17
View File
@@ -0,0 +1,17 @@
import { createContext, useContext, useLayoutEffect } from "react";
// A sticky element at the very top of the page scroller (a table's sticky header) and the
// scroller's top fade cannot coexist: the fade sits ON the header and flips on with the first
// scroll. Pages used to opt out by hand — a `fadeTop: false` in the module registry, plus an App
// special case for the Channels card layout, which has no sticky header — and the next table page
// would have re-acquired the defect with every lane green. Now the sticky element says so itself:
// while it is mounted the enclosing `PageScroller` keeps its top edge crisp.
const StickyTopContext = createContext<(() => () => void) | null>(null);
export const StickyTopProvider = StickyTopContext.Provider;
/** Call from a component that renders a sticky element at the top of the page scroller. */
export function useStickyTop(): void {
const register = useContext(StickyTopContext);
useLayoutEffect(() => register?.(), [register]);
}
+3 -1
View File
@@ -7,6 +7,7 @@ import { useScrollFade } from "../../lib/useScrollFade";
import { type SortState } from "../../lib/columnSort";
import { isActive, type Column, type FilterMap } from "../../lib/tableColumns";
import { alignClass } from "./tableAlign";
import { useStickyTop } from "./stickyTop";
// The column-header machinery — the sortable header row with its filter funnel + popover — lifted
// OUT of DataTable so a non-DataTable body can render under the same headers. The column contract
@@ -185,7 +186,7 @@ function FilterFunnel<T>({
* through; the bottom border rides a `box-shadow` because a sticky `<tr>`'s own border scrolls away
* with the collapsed table box in some browsers. The `::after` paints a short fade just BELOW the
* header so rows dissolve as they slide under it while the header itself stays crisp — which is why
* pages using this pass `PageShell fadeTop={false}`. */
* it switches the page scroller's own top fade off while it is mounted (`useStickyTop`). */
export function ColumnHead<T>({
columns,
sort,
@@ -199,6 +200,7 @@ export function ColumnHead<T>({
filters: FilterMap;
onFilter: (key: string, value: string | string[]) => void;
}) {
useStickyTop();
// Only one column's filter panel is open at a time; each funnel owns its own anchoring.
const [openFilter, setOpenFilter] = useState<string | null>(null);
+18
View File
@@ -0,0 +1,18 @@
import { useSyncExternalStore } from "react";
/** Whether a CSS media query matches, live. For a component that must RENDER differently at a
* breakpoint rather than merely hide one of two renders with CSS (`ui/DataTable` mounted its table
* AND its card list at every width: double the DOM, duplicate ids and keyboard targets). Where
* `matchMedia` is missing (jsdom) it answers `fallback`. */
export function useMediaQuery(query: string, fallback = true): boolean {
return useSyncExternalStore(
(onChange) => {
if (typeof window.matchMedia !== "function") return () => {};
const mql = window.matchMedia(query);
mql.addEventListener("change", onChange);
return () => mql.removeEventListener("change", onChange);
},
() => (typeof window.matchMedia === "function" ? window.matchMedia(query).matches : fallback),
() => fallback,
);
}