From e83c5e2d97f1d51ac402e786abbbd2132cd9ef33 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 23 Oct 2026 03:24:23 +0200 Subject: [PATCH] fix(frontend): the content is padded by the filled header before paint, not when the observer catches up The header's pills now arrive by portal after the shell has mounted, so the shell's one synchronous measurement saw an empty header, and the ResizeObserver brought `--hdr-h` up a frame or more later: meanwhile the content sat under the pills (the feed toolbar jumped 55px on a tablet; e2e caught it). The measurement re-runs before paint whenever what fills the header changes. --- .../ui/PageShell.header.dom.test.tsx | 32 +++++++++++++++++++ frontend/src/components/ui/PageShell.tsx | 32 +++++++++++++------ frontend/src/components/ui/pageHeaderSlot.ts | 7 ++-- 3 files changed, 60 insertions(+), 11 deletions(-) diff --git a/frontend/src/components/ui/PageShell.header.dom.test.tsx b/frontend/src/components/ui/PageShell.header.dom.test.tsx index fa2b8a66..d0852357 100644 --- a/frontend/src/components/ui/PageShell.header.dom.test.tsx +++ b/frontend/src/components/ui/PageShell.header.dom.test.tsx @@ -89,6 +89,38 @@ describe("the header slot", () => { expect(screen.getByTestId("hdr").textContent).toBe("search"); }); + it("pads the content by the filled header before paint, not when the observer catches up", () => { + // jsdom does no layout: the header's height here is one 40px row per pill in it. The observer + // never fires in this test, so only a measurement made as the pills arrive can see them. + const height = (el: HTMLElement) => 40 * el.childElementCount; + const page = (own: boolean) => ( + + {own && ( + + search + scope + + )} + {fallback} + + ); + const { rerender, container } = render({null}); + Object.defineProperty(screen.getByTestId("hdr"), "offsetHeight", { + get() { + return height(this as HTMLElement); + }, + configurable: true, + }); + const pad = () => + (container.querySelector('[class*="pt-(--hdr-h)"]') as HTMLElement).style.getPropertyValue( + "--hdr-h", + ); + rerender(page(false)); + expect(pad()).toBe("44px"); // the default alone: one row + rerender(page(true)); + expect(pad()).toBe("84px"); // the page's two pills, the default gone + }); + it("renders a page's pills nowhere in a shell without a header", () => { render( diff --git a/frontend/src/components/ui/PageShell.tsx b/frontend/src/components/ui/PageShell.tsx index 3986f971..9c8e391b 100644 --- a/frontend/src/components/ui/PageShell.tsx +++ b/frontend/src/components/ui/PageShell.tsx @@ -46,7 +46,13 @@ export function PageHeader({ children }: { children: ReactNode }) { // that its own controls would replace a moment later. export function PageHeaderDefault({ children }: { children: ReactNode }) { const slot = useContext(PageHeaderSlot); - return slot?.target && !slot.claimed ? createPortal(children, slot.target) : null; + const shown = !!slot?.target && !slot.claimed; + const showDefault = slot?.showDefault; + useLayoutEffect(() => { + showDefault?.(shown); + return () => showDefault?.(false); + }, [shown, showDefault]); + return shown && slot?.target ? createPortal(children, slot.target) : null; } // The page shell: the fixed chrome around a single scrollable content region. It formalizes "what @@ -144,6 +150,20 @@ export default function PageShell({ // the measurement to the live node re-runs it every time the header mounts/unmounts, so it stays correct. const [headerEl, setHeaderEl] = useState(null); const [contentEl, setContentEl] = useState(null); + // How many PageHeaders fill the header right now: a count, not a flag, so of two on one page the one + // that leaves releases only its own claim. + const [headerClaims, setHeaderClaims] = useState(0); + const claimHeader = useCallback(() => { + setHeaderClaims((n) => n + 1); + return () => setHeaderClaims((n) => n - 1); + }, []); + // Whether the shell's default is in the header (PageHeaderDefault reports it). + const [defaultShown, setDefaultShown] = useState(false); + // The pills arrive by portal AFTER this shell mounted (with the page's chunk), so the first measurement + // saw an empty header; the ResizeObserver catches up only a frame or more later, and until then the + // content sat under the pills (the feed toolbar jumped 55px on a tablet). So the measurement also re-runs + // here, before paint, whenever what fills the header changes; the observer still covers a change within + // a page's own pills (Plex's view switch appearing). useLayoutEffect(() => { if (!headerEl || !contentEl) { // No header (channel page) → drop any stale pad so a leftover value can't linger. @@ -157,18 +177,12 @@ export default function PageShell({ const ro = new ResizeObserver(apply); ro.observe(headerEl); return () => ro.disconnect(); - }, [headerEl, contentEl]); - // How many PageHeaders fill the header right now: a count, not a flag, so of two on one page the one - // that leaves releases only its own claim. - const [headerClaims, setHeaderClaims] = useState(0); - const claimHeader = useCallback(() => { - setHeaderClaims((n) => n + 1); - return () => setHeaderClaims((n) => n - 1); - }, []); + }, [headerEl, contentEl, headerClaims, defaultShown]); const headerSlot = { target: headerEl, claimed: headerClaims > 0, claim: claimHeader, + showDefault: setDefaultShown, }; // How many views below render their own page `h1` right now (see ./pageHeading). const [ownHeadings, setOwnHeadings] = useState(0); diff --git a/frontend/src/components/ui/pageHeaderSlot.ts b/frontend/src/components/ui/pageHeaderSlot.ts index e4308c2c..6b15cb84 100644 --- a/frontend/src/components/ui/pageHeaderSlot.ts +++ b/frontend/src/components/ui/pageHeaderSlot.ts @@ -1,14 +1,17 @@ import { createContext } from "react"; // The floating header's slot (see PageHeader in PageShell): the node a page portals its header -// controls into, whether a page has claimed it, and how a page claims it. Null with no shell, or in a -// shell without a header (the channel page), so a page renders its header nowhere there. Its own +// controls into, whether a page has claimed it, and how a page claims it. The value is null with no +// shell and the target null in a shell without a header (the channel page): a page renders its header +// nowhere there. Its own // module so PageShell exports only components (Fast Refresh). export type PageHeaderSlotValue = { target: HTMLElement | null; claimed: boolean; /** Claim the header for this page's controls; the returned function releases it. */ claim: () => () => void; + /** The default (PageHeaderDefault) reports whether it is in the header, so PageShell re-measures. */ + showDefault: (shown: boolean) => void; }; export const PageHeaderSlot = createContext(null);