refactor(header): move --hdr-h measurement into PageShell (S7 review)
Addresses the S7 mobile-fix code-review findings: - (altitude) The header-height→content-pad contract lived across two files with an implicit DOM coupling: Header measured itself and wrote --hdr-h onto `el.parentElement`, assuming PageShell rendered it as a direct child of the padded column. Move the whole contract into PageShell, which owns BOTH the header slot and the padded content wrapper: Header is now forwardRef and PageShell attaches a ref (via cloneElement), measures the header with a ResizeObserver, and writes --hdr-h straight onto the content wrapper that consumes it — no parent assumption, producer and consumer together. - (simplification) The `+ 8` gap is now documented as the header's own gap-y-2 row gap reused below it, not a bare magic number. - (test-coverage) PageShell.dom.test.tsx: stub ResizeObserver + the header's offset metrics and assert --hdr-h is published onto the content wrapper (and absent with no header). Re-verified live at 440 (mobile, no overlap) and desktop (one row). FE gate: tsc / knip / lint clean, 220 vitest pass.
This commit is contained in:
@@ -1,4 +1,4 @@
|
|||||||
import { useLayoutEffect, useRef, type ReactNode } from "react";
|
import { forwardRef, type ReactNode } from "react";
|
||||||
import { useTranslation } from "react-i18next";
|
import { useTranslation } from "react-i18next";
|
||||||
import { Youtube } from "lucide-react";
|
import { Youtube } from "lucide-react";
|
||||||
import type { Me } from "../lib/api";
|
import type { Me } from "../lib/api";
|
||||||
@@ -16,15 +16,18 @@ import { useChannels, useChannelsActions } from "./ChannelsProvider";
|
|||||||
// The floating top header: a row of glass pills over the content — a ModuleName pill (label +
|
// The floating top header: a row of glass pills over the content — a ModuleName pill (label +
|
||||||
// accent dot + back/forward arrows), the module's SearchBar where it has one, and the per-user
|
// accent dot + back/forward arrows), the module's SearchBar where it has one, and the per-user
|
||||||
// sync chip at the right end.
|
// sync chip at the right end.
|
||||||
export default function Header({
|
// `forwardRef` so PageShell can attach a ref to the header's root box and measure its real (possibly
|
||||||
me,
|
// wrapped) height — that measurement + the `--hdr-h` content pad it drives live in PageShell, which
|
||||||
onYtSearch,
|
// owns both the header slot and the padded content wrapper (E5 S7).
|
||||||
}: {
|
const Header = forwardRef<
|
||||||
|
HTMLDivElement,
|
||||||
|
{
|
||||||
me: Me;
|
me: Me;
|
||||||
// Trigger a live YouTube search for the current term (feed only; hidden for the demo account,
|
// Trigger a live YouTube search for the current term (feed only; hidden for the demo account,
|
||||||
// which can't spend the shared quota).
|
// which can't spend the shared quota).
|
||||||
onYtSearch: (q: string) => void;
|
onYtSearch: (q: string) => void;
|
||||||
}) {
|
}
|
||||||
|
>(function Header({ me, onYtSearch }, ref) {
|
||||||
const { t } = useTranslation();
|
const { t } = useTranslation();
|
||||||
const { page } = useNavigation();
|
const { page } = useNavigation();
|
||||||
const { setPage } = useNavigationActions();
|
const { setPage } = useNavigationActions();
|
||||||
@@ -46,27 +49,6 @@ export default function Header({
|
|||||||
// Drop input focus on Go/Enter for the live-filter modules (feed's Go escalates to YouTube).
|
// Drop input focus on Go/Enter for the live-filter modules (feed's Go escalates to YouTube).
|
||||||
const blur = () => (document.activeElement as HTMLElement | null)?.blur?.();
|
const blur = () => (document.activeElement as HTMLElement | null)?.blur?.();
|
||||||
|
|
||||||
// The header floats (absolute) over the content, which pads its top by `--hdr-h`. That default is
|
|
||||||
// sized for a single row; on a narrow window (esp. mobile) the flex-wrap header grows to 2–3 rows,
|
|
||||||
// so a FIXED pad would let the content toolbar (sort / count / filter chips) slide up under it.
|
|
||||||
// Measure the real header box and publish its height as `--hdr-h` on the content column, so the pad
|
|
||||||
// always tracks the actual (wrapped) header height at any width.
|
|
||||||
const headerRef = useRef<HTMLDivElement>(null);
|
|
||||||
useLayoutEffect(() => {
|
|
||||||
const el = headerRef.current;
|
|
||||||
const host = el?.parentElement; // the `relative` content column (PageShell renders a fragment)
|
|
||||||
if (!el || !host) return;
|
|
||||||
const apply = () =>
|
|
||||||
host.style.setProperty("--hdr-h", `${el.offsetTop + el.offsetHeight + 8}px`);
|
|
||||||
apply();
|
|
||||||
const ro = new ResizeObserver(apply);
|
|
||||||
ro.observe(el);
|
|
||||||
return () => {
|
|
||||||
ro.disconnect();
|
|
||||||
host.style.removeProperty("--hdr-h");
|
|
||||||
};
|
|
||||||
}, []);
|
|
||||||
|
|
||||||
// Per-page SearchBar wiring — null for modules without a search.
|
// Per-page SearchBar wiring — null for modules without a search.
|
||||||
let search: ReactNode = null;
|
let search: ReactNode = null;
|
||||||
if (page === "feed") {
|
if (page === "feed") {
|
||||||
@@ -116,7 +98,7 @@ export default function Header({
|
|||||||
// actual left zone: it sits just inside the content, reclaiming the space when the nav rail is
|
// actual left zone: it sits just inside the content, reclaiming the space when the nav rail is
|
||||||
// slim and/or no filter sidebar is present — no hard-coded offset.
|
// slim and/or no filter sidebar is present — no hard-coded offset.
|
||||||
<div
|
<div
|
||||||
ref={headerRef}
|
ref={ref}
|
||||||
className="pointer-events-none absolute top-3 left-3 right-[10%] z-chrome flex flex-wrap items-center gap-x-3 gap-y-2"
|
className="pointer-events-none absolute top-3 left-3 right-[10%] z-chrome flex flex-wrap items-center gap-x-3 gap-y-2"
|
||||||
>
|
>
|
||||||
<ModuleName
|
<ModuleName
|
||||||
@@ -140,4 +122,6 @@ export default function Header({
|
|||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
}
|
});
|
||||||
|
|
||||||
|
export default Header;
|
||||||
|
|||||||
@@ -0,0 +1,53 @@
|
|||||||
|
// @vitest-environment jsdom
|
||||||
|
//
|
||||||
|
// E5 S7: PageShell measures the floating header and publishes its real height as `--hdr-h` onto the
|
||||||
|
// content wrapper, so a header that flex-wraps to 2–3 rows on a narrow window never overlaps the
|
||||||
|
// content below it. jsdom does no layout (offsetHeight is 0) and has no ResizeObserver, so we stub
|
||||||
|
// both: a ResizeObserver that hands us its callback, and offset metrics on the measured header.
|
||||||
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||||
|
import { render, cleanup } from "@testing-library/react";
|
||||||
|
import PageShell from "./PageShell";
|
||||||
|
|
||||||
|
vi.mock("./PageScroller", () => ({
|
||||||
|
default: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||||
|
}));
|
||||||
|
|
||||||
|
let roCb: (() => void) | null = null;
|
||||||
|
beforeEach(() => {
|
||||||
|
roCb = null;
|
||||||
|
vi.stubGlobal(
|
||||||
|
"ResizeObserver",
|
||||||
|
class {
|
||||||
|
constructor(cb: () => void) {
|
||||||
|
roCb = cb;
|
||||||
|
}
|
||||||
|
observe() {}
|
||||||
|
disconnect() {}
|
||||||
|
},
|
||||||
|
);
|
||||||
|
});
|
||||||
|
afterEach(() => {
|
||||||
|
cleanup();
|
||||||
|
vi.unstubAllGlobals();
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("PageShell --hdr-h (E5 S7)", () => {
|
||||||
|
it("publishes the measured header height onto the content wrapper", () => {
|
||||||
|
const { container } = render(
|
||||||
|
<PageShell header={<div data-testid="hdr">H</div>}>content</PageShell>,
|
||||||
|
);
|
||||||
|
const content = container.querySelector('[class*="pt-(--hdr-h)"]') as HTMLElement;
|
||||||
|
expect(content).toBeTruthy();
|
||||||
|
// Give the header a real (multi-row) box, then fire the ResizeObserver callback the effect made.
|
||||||
|
const hdr = container.querySelector('[data-testid="hdr"]') as HTMLElement;
|
||||||
|
Object.defineProperty(hdr, "offsetTop", { value: 12, configurable: true });
|
||||||
|
Object.defineProperty(hdr, "offsetHeight", { value: 96, configurable: true });
|
||||||
|
roCb?.();
|
||||||
|
expect(content.style.getPropertyValue("--hdr-h")).toBe("116px"); // 12 + 96 + 8
|
||||||
|
});
|
||||||
|
|
||||||
|
it("adds no `--hdr-h` pad when there is no header", () => {
|
||||||
|
const { container } = render(<PageShell>content</PageShell>);
|
||||||
|
expect(container.querySelector('[class*="pt-(--hdr-h)"]')).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -1,4 +1,15 @@
|
|||||||
import { createContext, useContext, useState, type ReactNode } from "react";
|
import {
|
||||||
|
cloneElement,
|
||||||
|
createContext,
|
||||||
|
isValidElement,
|
||||||
|
useContext,
|
||||||
|
useLayoutEffect,
|
||||||
|
useRef,
|
||||||
|
useState,
|
||||||
|
type ReactElement,
|
||||||
|
type ReactNode,
|
||||||
|
type Ref,
|
||||||
|
} from "react";
|
||||||
import { createPortal } from "react-dom";
|
import { createPortal } from "react-dom";
|
||||||
import PageScroller from "./PageScroller";
|
import PageScroller from "./PageScroller";
|
||||||
|
|
||||||
@@ -52,10 +63,39 @@ export default function PageShell({
|
|||||||
fadeTop?: boolean;
|
fadeTop?: boolean;
|
||||||
}) {
|
}) {
|
||||||
const [toolbarSlot, setToolbarSlot] = useState<HTMLDivElement | null>(null);
|
const [toolbarSlot, setToolbarSlot] = useState<HTMLDivElement | null>(null);
|
||||||
|
// The floating header pads the content below it by `--hdr-h`. That must equal the header's REAL
|
||||||
|
// height, which grows as the flex-wrap header wraps to 2–3 rows on a narrow window — a fixed pad
|
||||||
|
// would let the content toolbar slide up under it (E5 S7). Measure the header (a ref forwarded onto
|
||||||
|
// its root box) and write its height straight onto the content wrapper that consumes `--hdr-h`, so
|
||||||
|
// the two live together with no assumption about the header's DOM parent.
|
||||||
|
const headerRef = useRef<HTMLDivElement | null>(null);
|
||||||
|
const contentRef = useRef<HTMLDivElement | null>(null);
|
||||||
|
useLayoutEffect(() => {
|
||||||
|
const h = headerRef.current;
|
||||||
|
const c = contentRef.current;
|
||||||
|
if (!h || !c) return;
|
||||||
|
// `+ 8` = the header's own gap-y-2 row gap (0.5rem), reused as the gap below it.
|
||||||
|
const apply = () => c.style.setProperty("--hdr-h", `${h.offsetTop + h.offsetHeight + 8}px`);
|
||||||
|
apply();
|
||||||
|
const ro = new ResizeObserver(apply);
|
||||||
|
ro.observe(h);
|
||||||
|
return () => {
|
||||||
|
ro.disconnect();
|
||||||
|
c.style.removeProperty("--hdr-h");
|
||||||
|
};
|
||||||
|
}, []);
|
||||||
|
const measuredHeader = isValidElement(header)
|
||||||
|
? cloneElement(header as ReactElement<{ ref?: Ref<HTMLDivElement> }>, {
|
||||||
|
ref: headerRef,
|
||||||
|
})
|
||||||
|
: header;
|
||||||
return (
|
return (
|
||||||
<PageToolbarSlot.Provider value={toolbarSlot}>
|
<PageToolbarSlot.Provider value={toolbarSlot}>
|
||||||
{header}
|
{measuredHeader}
|
||||||
<div className={`flex-1 min-w-0 min-h-0 flex flex-col ${header ? "pt-(--hdr-h)" : ""}`}>
|
<div
|
||||||
|
ref={contentRef}
|
||||||
|
className={`flex-1 min-w-0 min-h-0 flex flex-col ${header ? "pt-(--hdr-h)" : ""}`}
|
||||||
|
>
|
||||||
{banners}
|
{banners}
|
||||||
<div ref={setToolbarSlot} className="shrink-0" />
|
<div ref={setToolbarSlot} className="shrink-0" />
|
||||||
<PageScroller scrollKey={scrollKey} fadeTop={fadeTop}>
|
<PageScroller scrollKey={scrollKey} fadeTop={fadeTop}>
|
||||||
|
|||||||
Reference in New Issue
Block a user