diff --git a/apps/app/src/components/sidebar/SidebarWindowedItems.test.tsx b/apps/app/src/components/sidebar/SidebarWindowedItems.test.tsx index c2815fa5ff..fc4c13a746 100644 --- a/apps/app/src/components/sidebar/SidebarWindowedItems.test.tsx +++ b/apps/app/src/components/sidebar/SidebarWindowedItems.test.tsx @@ -1,6 +1,6 @@ // @vitest-environment jsdom -import { cleanup, render, screen } from "@testing-library/react"; +import { act, cleanup, render, screen } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const sidebarMocks = vi.hoisted(() => ({ @@ -13,20 +13,83 @@ vi.mock("@/components/ui/sidebar.js", () => ({ import { SidebarWindowedItems } from "./SidebarWindowedItems"; -beforeEach(() => { - const scrollElement = document.createElement("div"); - Object.defineProperty(scrollElement, "clientHeight", { +let observerCallback: IntersectionObserverCallback | null = null; +let observerInstance: IntersectionObserver | null = null; + +function setClientHeight(element: HTMLElement, value: number): void { + Object.defineProperty(element, "clientHeight", { configurable: true, - value: 500, + value, }); +} + +function items( + itemKeys: readonly string[] = ["first", "second", "third"], + alwaysMountedKeys?: ReadonlySet, +) { + return ( + 1} + alwaysMountedKeys={alwaysMountedKeys} + getNavigationEntries={(index) => [ + { projectId: "proj_test", threadId: `thr_${index}` }, + ]} + renderItem={(index) => ( + Real item {index} + )} + /> + ); +} + +function renderItems(alwaysMountedKeys?: ReadonlySet) { + return render(items(undefined, alwaysMountedKeys)); +} + +function emitIntersection(target: Element, isIntersecting: boolean): void { + if (observerCallback === null || observerInstance === null) { + throw new Error("Expected SidebarWindowedItems to create an observer."); + } + const rect = target.getBoundingClientRect(); + const entry: IntersectionObserverEntry = { + boundingClientRect: rect, + intersectionRatio: isIntersecting ? 1 : 0, + intersectionRect: isIntersecting ? rect : new DOMRect(), + isIntersecting, + rootBounds: new DOMRect(0, 0, 300, 500), + target, + time: performance.now(), + }; + observerCallback([entry], observerInstance); +} + +beforeEach(() => { + observerCallback = null; + observerInstance = null; + + const scrollElement = document.createElement("div"); + setClientHeight(scrollElement, 500); sidebarMocks.scrollElementRef.current = scrollElement; vi.stubGlobal( "IntersectionObserver", - class { + class implements IntersectionObserver { + readonly root = scrollElement; + readonly rootMargin = "240px 0px"; + readonly scrollMargin = "0px"; + readonly thresholds = [0]; + + constructor(callback: IntersectionObserverCallback) { + observerCallback = callback; + observerInstance = this; + } + observe() {} unobserve() {} disconnect() {} + takeRecords(): IntersectionObserverEntry[] { + return []; + } }, ); @@ -45,6 +108,7 @@ beforeEach(() => { afterEach(() => { cleanup(); + document.body.replaceChildren(); sidebarMocks.scrollElementRef.current = null; vi.restoreAllMocks(); vi.unstubAllGlobals(); @@ -52,18 +116,7 @@ afterEach(() => { describe("SidebarWindowedItems", () => { it("windows a short list when every item is outside the viewport margin", () => { - render( - 1} - getNavigationEntries={(index) => [ - { projectId: "proj_test", threadId: `thr_${index}` }, - ]} - renderItem={(index) => ( - Real item {index} - )} - />, - ); + renderItems(); expect(screen.queryByTestId("real-item-0")).toBeNull(); expect( @@ -73,4 +126,134 @@ describe("SidebarWindowedItems", () => { document.querySelectorAll("[data-sidebar-windowed-nav]"), ).toHaveLength(3); }); + + it("keeps placeholders while a newly mounted scrollport ref attaches", () => { + sidebarMocks.scrollElementRef.current = null; + + renderItems(); + + expect(screen.queryByTestId("real-item-0")).toBeNull(); + expect( + document.querySelectorAll("[data-sidebar-windowed-item]:empty"), + ).toHaveLength(3); + }); + + it("keeps placeholders for a connected scrollport with transient zero-height geometry", () => { + const scrollElement = sidebarMocks.scrollElementRef.current; + if (scrollElement === null) { + throw new Error("Expected a scroll element."); + } + document.body.append(scrollElement); + setClientHeight(scrollElement, 0); + + renderItems(); + + expect(screen.queryByTestId("real-item-0")).toBeNull(); + expect(observerInstance?.root).toBe(scrollElement); + }); + + it("renders every item for a detached zero-height preview", () => { + const scrollElement = sidebarMocks.scrollElementRef.current; + if (scrollElement === null) { + throw new Error("Expected a scroll element."); + } + setClientHeight(scrollElement, 0); + + renderItems(); + + expect(screen.getByTestId("real-item-0")).toBeTruthy(); + expect(screen.getByTestId("real-item-1")).toBeTruthy(); + expect(screen.getByTestId("real-item-2")).toBeTruthy(); + }); + + it("does not promote vertically overlapping rows inside a closed compact drawer", () => { + const scrollElement = sidebarMocks.scrollElementRef.current; + if (scrollElement === null) { + throw new Error("Expected a scroll element."); + } + const panel = document.createElement("aside"); + panel.dataset.sidebar = "panel"; + panel.dataset.state = "closed"; + panel.append(scrollElement); + document.body.append(panel); + vi.mocked(HTMLElement.prototype.getBoundingClientRect).mockImplementation( + function (this: HTMLElement) { + if (this === scrollElement) { + return new DOMRect(0, 0, 300, 500); + } + if (this.hasAttribute("data-sidebar-windowed-item")) { + return new DOMRect(0, 100, 300, 30); + } + return new DOMRect(); + }, + ); + + renderItems(new Set(["second"])); + + expect(screen.queryByTestId("real-item-0")).toBeNull(); + expect(screen.getByTestId("real-item-1")).toBeTruthy(); + expect(screen.queryByTestId("real-item-2")).toBeNull(); + }); + + it("retains realized rows without promoting new rows after the drawer closes", () => { + const scrollElement = sidebarMocks.scrollElementRef.current; + if (scrollElement === null) { + throw new Error("Expected a scroll element."); + } + const panel = document.createElement("aside"); + panel.dataset.sidebar = "panel"; + panel.dataset.state = "open"; + panel.append(scrollElement); + document.body.append(panel); + vi.mocked(HTMLElement.prototype.getBoundingClientRect).mockImplementation( + function (this: HTMLElement) { + if (this === scrollElement) { + return new DOMRect(0, 0, 300, 500); + } + if (this.hasAttribute("data-sidebar-windowed-item")) { + return new DOMRect(0, 100, 300, 30); + } + return new DOMRect(); + }, + ); + + const view = renderItems(); + expect(screen.getByTestId("real-item-0")).toBeTruthy(); + expect(screen.getByTestId("real-item-2")).toBeTruthy(); + + panel.dataset.state = "closed"; + view.rerender(items(["first", "second", "third", "fourth"])); + + expect(screen.getByTestId("real-item-0")).toBeTruthy(); + expect(screen.getByTestId("real-item-2")).toBeTruthy(); + expect(screen.queryByTestId("real-item-3")).toBeNull(); + }); + + it("realizes visible content after a closed drawer reopens", async () => { + const scrollElement = sidebarMocks.scrollElementRef.current; + if (scrollElement === null) { + throw new Error("Expected a scroll element."); + } + const panel = document.createElement("aside"); + panel.dataset.sidebar = "panel"; + panel.dataset.state = "closed"; + panel.append(scrollElement); + document.body.append(panel); + + renderItems(); + + const firstWrapper = document.querySelector( + "[data-sidebar-windowed-item]", + ); + if (firstWrapper === null) { + throw new Error("Expected the first windowed wrapper."); + } + expect(screen.queryByTestId("real-item-0")).toBeNull(); + + panel.dataset.state = "open"; + await act(async () => emitIntersection(firstWrapper, true)); + + expect(screen.getByTestId("real-item-0")).toBeTruthy(); + expect(screen.queryByTestId("real-item-1")).toBeNull(); + }); }); diff --git a/apps/app/src/components/sidebar/SidebarWindowedItems.tsx b/apps/app/src/components/sidebar/SidebarWindowedItems.tsx index 0c9da85697..b17a55f3f1 100644 --- a/apps/app/src/components/sidebar/SidebarWindowedItems.tsx +++ b/apps/app/src/components/sidebar/SidebarWindowedItems.tsx @@ -69,8 +69,8 @@ const EMPTY_KEY_SET: ReadonlySet = new Set(); * * Promotion runs in two tiers: a pre-paint layout pass whenever the key list * changes (no placeholder flash on mount), then an IntersectionObserver with - * a generous margin for scrolling. When no usable scrollport exists (jsdom, - * detached previews), every item renders for real. + * a generous margin for scrolling. Environments without IntersectionObserver + * and known detached previews render every item for real. */ export function SidebarWindowedItems({ itemKeys, @@ -147,8 +147,9 @@ export function SidebarWindowedItems({ // Pre-paint promotion whenever the item set changes: mount everything that // sits inside the scrollport plus margin before the browser paints, so the // initial view never flashes placeholders. Also prunes state for keys that - // left the list. With no usable viewport (jsdom, previews) it promotes - // everything. + // left the list. With no IntersectionObserver (jsdom) or a known detached + // viewport it promotes everything. A null parent ref stays windowed until + // the passive observer effect runs after host refs have attached. useLayoutEffect(() => { if (!windowingEnabled) { if (realizedKeys.size > 0) { @@ -171,9 +172,16 @@ export function SidebarWindowedItems({ } const scrollElement = scrollElementRef?.current ?? null; + // A connected scrollport can report zero height for one commit while an + // orientation/breakpoint change reparents the sidebar. Rendering every + // item in that state makes WebKit lay out the entire offscreen thread + // tree before IntersectionObserver immediately prunes it again. Keep the + // placeholders and let the observer promote visible items once the new + // geometry settles. Detached previews still render everything. const promoteAll = - !scrollElement || - scrollElement.clientHeight === 0 || + (scrollElement !== null && + scrollElement.clientHeight === 0 && + !scrollElement.isConnected) || typeof IntersectionObserver === "undefined"; const next = new Set(); @@ -187,17 +195,30 @@ export function SidebarWindowedItems({ for (const key of itemKeys) { next.add(key); } - } else { + } else if (scrollElement) { const viewport = scrollElement.getBoundingClientRect(); + const viewportIsHorizontallyVisible = + viewport.right > 0 && viewport.left < window.innerWidth; + const isInsideClosedSidebarPanel = + scrollElement.closest('[data-sidebar="panel"][data-state="closed"]') !== + null; const viewportTop = viewport.top - WINDOW_VIEWPORT_MARGIN_PX; const viewportBottom = viewport.bottom + WINDOW_VIEWPORT_MARGIN_PX; - for (const [key, element] of wrapperByKeyRef.current) { - if (next.has(key) || !keySet.has(key)) { - continue; - } - const rect = element.getBoundingClientRect(); - if (rect.bottom >= viewportTop && rect.top <= viewportBottom) { - next.add(key); + // A closed compact sidebar is translated completely offscreen. Its + // vertical coordinates still overlap the viewport, so checking only Y + // eagerly realized every nested thread-list window during a breakpoint + // change. WebKit then laid out the whole hidden tree before the + // observer pruned it. Existing realized rows stay warm, but a newly + // mounted offscreen drawer waits until it is actually visible. + if (viewportIsHorizontallyVisible && !isInsideClosedSidebarPanel) { + for (const [key, element] of wrapperByKeyRef.current) { + if (next.has(key) || !keySet.has(key)) { + continue; + } + const rect = element.getBoundingClientRect(); + if (rect.bottom >= viewportTop && rect.top <= viewportBottom) { + next.add(key); + } } } } @@ -218,7 +239,7 @@ export function SidebarWindowedItems({ return; } const scrollElement = scrollElementRef?.current ?? null; - if (!scrollElement || scrollElement.clientHeight === 0) { + if (!scrollElement) { return; }