From a6a1db5254df60bfa30ce0e95a525b343fa3b058 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Sun, 6 Sep 2026 19:36:36 +0200 Subject: [PATCH] fix(gallery): release grid tiles once they are far enough out of view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pre-load band made tiles arrive in time. It did nothing about them never leaving. PhotoCard latched its observer with triggerOnce, so a tile that had been scrolled past stayed mounted for the life of the page — holding its object URL, and where image protection is on a canvas sized to the image that the browser is not permitted to evict. Measured in Chrome on a seeded 546-photo grid, scrolling top to bottom: mounted tiles climb 24 → 100 → 212 → 364 → 546 and never fall. That is a monotonically growing retained set, which is the profile a memory-constrained browser discards a tab over — the reported symptom on iOS Safari 18.1 being tiles that stop appearing and a blank page after refresh. With this change the same scroll peaks at 68. PhotoCard now takes an optional outer band. The inner band, unchanged, decides when a tile starts loading; the outer one decides when it is far enough away to unmount, and unmounting is what actually frees anything, because AuthenticatedImage revokes its object URL and drops the canvas in its cleanup. The gap between the bands is the hysteresis: at three viewport heights against a one-viewport load band, a tile travels two further viewport heights after it stops loading before it is released, so ordinary scrolling never crosses both edges. Thumbnails are served private, max-age=1800, so returning costs a cache hit rather than a round trip. Opt-in per layout, and only Grid opts in. Its skeleton is aspect-square and holds the tile's box exactly, so releasing shifts nothing; the measured layouts have no such guarantee. Without the prop the observer keeps its original latch, so every other layout behaves exactly as before — pinned by a test, since that is the half most easily broken by accident. This is not presented as the fix for the iOS report. It removes the mechanism that best explains it, and it is worth having on any device; whether it is the mechanism still needs a measurement from the phone that failed. Relates to issue 1287 --- frontend/src/components/gallery/PhotoCard.tsx | 49 ++++- .../__tests__/PhotoCard.releaseBand.test.tsx | 183 ++++++++++++++++++ .../gallery/layouts/GridGalleryLayout.tsx | 20 ++ .../layouts/__tests__/gridLazyMargin.test.ts | 20 ++ 4 files changed, 266 insertions(+), 6 deletions(-) create mode 100644 frontend/src/components/gallery/__tests__/PhotoCard.releaseBand.test.tsx diff --git a/frontend/src/components/gallery/PhotoCard.tsx b/frontend/src/components/gallery/PhotoCard.tsx index 5e27a33b..62fc61f7 100644 --- a/frontend/src/components/gallery/PhotoCard.tsx +++ b/frontend/src/components/gallery/PhotoCard.tsx @@ -35,6 +35,13 @@ export interface PhotoCardProps { /** Lazy-render via IntersectionObserver with a skeleton placeholder. */ lazy?: boolean; inViewRootMargin?: string; + /** + * Outer band, in `rootMargin` form. When set, a tile that leaves it is + * unmounted again rather than kept for the life of the page (#1287). Opt-in + * per layout: only a layout whose skeleton holds the tile's box can release + * without reflowing, which today is Grid (`aspect-square`). + */ + releaseRootMargin?: string; skeletonClassName?: string; /** Keep container at opacity 0 until in view (only meaningful with `lazy`). */ fadeInWhenVisible?: boolean; @@ -85,6 +92,7 @@ export const PhotoCard: React.FC = ({ imageProps, lazy = false, inViewRootMargin, + releaseRootMargin, skeletonClassName = 'skeleton w-full h-full rounded-lg', fadeInWhenVisible = false, overlayBaseClassName, @@ -157,13 +165,40 @@ export const PhotoCard: React.FC = ({ } }, [isSelectionMode, hideOverlay]); - // Lazy loading with intersection observer - const { ref, inView: observedInView } = useInView({ - triggerOnce: true, + // Lazy loading with intersection observer. + // + // Two bands with a deliberate gap between them (#1287). The inner one, from + // `inViewRootMargin`, decides when a tile starts loading. The outer one, + // from `releaseRootMargin`, decides when it is far enough away to unmount — + // and unmounting is the part that frees anything, because AuthenticatedImage + // revokes its object URL and drops any protection canvas in its cleanup, and + // neither is reclaimable while the tile stays mounted. On a 546-photo grid + // the old latch meant every tile scrolled past was retained for the life of + // the page, which is the memory profile iOS Safari discards a tab over. + // + // The gap between the bands is the hysteresis: a tile is not released until + // it is well outside the band that would immediately reload it, so scrolling + // back and forth across one edge cannot thrash. Without a release band the + // observer keeps its original `triggerOnce` latch, so every other layout + // behaves exactly as before. + const releases = Boolean(lazy && releaseRootMargin); + const { ref: loadBandRef, inView: withinLoadBand } = useInView({ + triggerOnce: !releases, threshold: 0.1, rootMargin: inViewRootMargin, }); - const inView = !lazy || observedInView; + const { ref: keepBandRef, inView: withinKeepBand } = useInView({ + skip: !releases, + threshold: 0, + rootMargin: releaseRootMargin, + }); + const [rendered, setRendered] = useState(false); + useEffect(() => { + if (!releases) return; + if (withinLoadBand) setRendered(true); + else if (!withinKeepBand) setRendered(false); + }, [releases, withinLoadBand, withinKeepBand]); + const inView = !lazy || (releases ? rendered : withinLoadBand); // Tile width for the responsive tier (#1095), measured rather than inferred. // The observer entry only exists for `lazy` cards, and Mosaic, Masonry and @@ -183,8 +218,10 @@ export const PhotoCard: React.FC = ({ const [tile, setTile] = useState<{ width: number | null } | null>(null); const setContainerRef = useCallback((node: HTMLDivElement | null) => { containerRef.current = node; - if (lazy) ref(node); - }, [ref, lazy]); + if (!lazy) return; + loadBandRef(node); + if (releases) keepBandRef(node); + }, [loadBandRef, keepBandRef, lazy, releases]); useLayoutEffect(() => { if (!inView || tile) return; diff --git a/frontend/src/components/gallery/__tests__/PhotoCard.releaseBand.test.tsx b/frontend/src/components/gallery/__tests__/PhotoCard.releaseBand.test.tsx new file mode 100644 index 00000000..0892bb7b --- /dev/null +++ b/frontend/src/components/gallery/__tests__/PhotoCard.releaseBand.test.tsx @@ -0,0 +1,183 @@ +/** + * Releasing offscreen grid tiles (#1287). + * + * The pre-load band made tiles arrive in time; it did nothing about them never + * leaving. A tile that has been scrolled past keeps its object URL, and where + * image protection is on a full-resolution canvas the browser may not evict, + * for the life of the page — so a several-hundred-photo grid grows monotonically + * until the browser discards the tab. + * + * Unmounting is what frees those, so that is what these assert: the tile's + * subtree really goes away when it is far enough out, really comes back, and + * never flickers on ordinary scrolling. The `releaseRootMargin`-less path is + * pinned too, because every other layout still depends on the old latch. + */ +import React from 'react'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { render, screen } from '@testing-library/react'; + +import { PhotoCard } from '../PhotoCard'; +import type { Photo } from '../../../types'; + +const LOAD_BAND = '100% 0px'; +const KEEP_BAND = '300% 0px'; + +/** Which bands the tile is currently inside; the test drives these directly. */ +let bands: Record = {}; +// `triggerOnce` has to be modelled, not ignored: it is the whole difference +// between a layout that opts in to releasing and one that does not, so a mock +// that always reports live visibility would quietly turn the latch test into a +// test of the mock. +vi.mock('react-intersection-observer', () => ({ + useInView: ( + { rootMargin, skip, triggerOnce }: + { rootMargin?: string; skip?: boolean; triggerOnce?: boolean }, + ) => { + const latched = React.useRef(false); + const live = skip ? false : Boolean(bands[rootMargin ?? '']); + if (live) latched.current = true; + return { ref: () => {}, inView: triggerOnce ? latched.current : live }; + }, +})); + +// Counting mounts and unmounts is the whole point: it is the unmount that +// revokes the object URL and drops the canvas. +const lifecycle = { mounted: 0, unmounted: 0 }; +vi.mock('../../common', () => ({ + AuthenticatedImage: ({ src }: { src: string }) => { + React.useEffect(() => { + lifecycle.mounted += 1; + return () => { lifecycle.unmounted += 1; }; + }, []); + return ; + }, +})); +vi.mock('../../../contexts/GuestIdentityContext', () => ({ + useGuestIdentityOptional: () => null, +})); + +const PHOTO = { + id: 7, + filename: 'IMG_0001.jpg', + url: '/api/gallery/x/photo/7', + thumbnail_url: '/api/gallery/x/thumbnail/7', + type: 'individual', + size: 1, + uploaded_at: '2026-01-01T00:00:00Z', +} as Photo; + +function renderCard(props: Partial> = {}) { + return render( + {}} + onDownload={() => {}} + onToggleSelect={() => {}} + className="tile aspect-square" + skeletonClassName="skeleton aspect-square" + overlayBaseClassName="overlay" + imageProps={{ src: PHOTO.thumbnail_url!, alt: PHOTO.filename }} + lazy + inViewRootMargin={LOAD_BAND} + releaseRootMargin={KEEP_BAND} + {...props} + />, + ); +} + +const scrollTo = ( + rerender: (ui: React.ReactElement) => void, + next: Record, + props: Partial> = {}, +) => { + bands = next; + rerender( + {}} + onDownload={() => {}} + onToggleSelect={() => {}} + className="tile aspect-square" + skeletonClassName="skeleton aspect-square" + overlayBaseClassName="overlay" + imageProps={{ src: PHOTO.thumbnail_url!, alt: PHOTO.filename }} + lazy + inViewRootMargin={LOAD_BAND} + releaseRootMargin={KEEP_BAND} + {...props} + />, + ); +}; + +beforeEach(() => { + bands = {}; + lifecycle.mounted = 0; + lifecycle.unmounted = 0; + Object.defineProperty(HTMLElement.prototype, 'offsetWidth', { + configurable: true, + get() { return 200; }, + }); +}); + +describe('grid tiles release when they are far enough out of view', () => { + it('does not load a tile that is only inside the outer band', () => { + bands = { [KEEP_BAND]: true }; + renderCard(); + expect(screen.queryByTestId('tile')).toBeNull(); + expect(lifecycle.mounted).toBe(0); + }); + + it('loads inside the pre-load band and releases past the outer band', () => { + bands = { [LOAD_BAND]: true, [KEEP_BAND]: true }; + const { rerender } = renderCard(); + expect(screen.getByTestId('tile')).toBeTruthy(); + expect(lifecycle.mounted).toBe(1); + + // Scrolled well past: outside both bands. + scrollTo(rerender, {}); + expect(screen.queryByTestId('tile')).toBeNull(); + // The unmount is the release — object URL revoked, canvas dropped. + expect(lifecycle.unmounted).toBe(1); + }); + + it('holds the tile in the gap between the bands, so scrolling cannot thrash', () => { + bands = { [LOAD_BAND]: true, [KEEP_BAND]: true }; + const { rerender } = renderCard(); + + // Past the load band but still within the keep band — the hysteresis gap. + scrollTo(rerender, { [KEEP_BAND]: true }); + expect(screen.getByTestId('tile')).toBeTruthy(); + expect(lifecycle.unmounted).toBe(0); + + // Back towards the viewport without ever having been released. + scrollTo(rerender, { [LOAD_BAND]: true, [KEEP_BAND]: true }); + expect(lifecycle.mounted).toBe(1); + }); + + it('brings a released tile back when it returns', () => { + bands = { [LOAD_BAND]: true, [KEEP_BAND]: true }; + const { rerender } = renderCard(); + scrollTo(rerender, {}); + expect(screen.queryByTestId('tile')).toBeNull(); + + scrollTo(rerender, { [LOAD_BAND]: true, [KEEP_BAND]: true }); + expect(screen.getByTestId('tile')).toBeTruthy(); + expect(lifecycle.mounted).toBe(2); + }); + + it('keeps the old latch for layouts that do not opt in', () => { + bands = { [LOAD_BAND]: true }; + const { rerender } = renderCard({ releaseRootMargin: undefined }); + expect(screen.getByTestId('tile')).toBeTruthy(); + + // Far outside everything. A measured layout has no skeleton that holds the + // box, so releasing there would reflow — it must stay mounted. + scrollTo(rerender, {}, { releaseRootMargin: undefined }); + expect(screen.getByTestId('tile')).toBeTruthy(); + expect(lifecycle.unmounted).toBe(0); + }); +}); diff --git a/frontend/src/components/gallery/layouts/GridGalleryLayout.tsx b/frontend/src/components/gallery/layouts/GridGalleryLayout.tsx index 66a45cd5..e10f5c6e 100644 --- a/frontend/src/components/gallery/layouts/GridGalleryLayout.tsx +++ b/frontend/src/components/gallery/layouts/GridGalleryLayout.tsx @@ -101,6 +101,26 @@ const GridPhoto: React.FC = ({ * of lead in each direction, which is what vh would have meant. */ inViewRootMargin="100% 0px" + /* + * Release band (#1287). The pre-load band above fixed tiles arriving + * late; it did nothing about tiles never leaving. Every tile scrolled + * past kept its object URL — and, where image protection is on, a + * full-resolution canvas that the browser is not allowed to evict — for + * the life of the page. On a several-hundred-photo gallery that grows + * monotonically, which is the shape a memory-constrained browser + * discards the tab over. + * + * Three viewport heights, against a one-viewport load band: a tile has + * to travel two further viewport heights after it stops loading before + * it is released, so ordinary scrolling never crosses both edges. + * Thumbnails are served `private, max-age=1800`, so coming back costs a + * cache hit rather than a round trip. + * + * Grid only, and deliberately so: the skeleton here is `aspect-square` + * and holds the tile's box exactly, so releasing shifts nothing. The + * measured layouts have no such guarantee. + */ + releaseRootMargin="300% 0px" fadeInWhenVisible={animationType === 'fade'} skeletonClassName="skeleton aspect-square w-full rounded-lg" imageProps={{ diff --git a/frontend/src/components/gallery/layouts/__tests__/gridLazyMargin.test.ts b/frontend/src/components/gallery/layouts/__tests__/gridLazyMargin.test.ts index 13fc77c2..55688a40 100644 --- a/frontend/src/components/gallery/layouts/__tests__/gridLazyMargin.test.ts +++ b/frontend/src/components/gallery/layouts/__tests__/gridLazyMargin.test.ts @@ -45,6 +45,26 @@ describe('grid lazy pre-load band', () => { } }); + it('Grid releases what it loaded, and the outer band is legal and wider', () => { + // The pre-load band fixed tiles arriving late; it did nothing about them + // never leaving. Measured in Chrome on a seeded 546-photo grid: without a + // release band the mounted count climbs 24 → 100 → 212 → 364 → 546 and + // never falls, because a tile that has been scrolled past keeps its object + // URL and any protection canvas for the life of the page. With it the peak + // is 68. + const src = read('GridGalleryLayout.tsx'); + const release = src.match(/releaseRootMargin="([^"]+)"/); + expect(release, 'Grid declares no releaseRootMargin').toBeTruthy(); + expect(release![1]).toMatch(LEGAL_ROOT_MARGIN); + + // The gap between the bands is the hysteresis. If the outer band were not + // strictly wider, a tile would be released and immediately reloaded on + // every scroll across the edge. + const load = src.match(/inViewRootMargin="([^"]+)"/); + const percent = (value: string) => Number(value.split(/\s+/)[0].replace('%', '')); + expect(percent(release![1])).toBeGreaterThan(percent(load![1])); + }); + it('every layout that lazy-renders also declares a pre-load band', () => { // The defect was Grid being lazy with no margin. Any future layout that // opts into `lazy` and forgets the margin reintroduces it.