diff --git a/.release-please-manifest-beta.json b/.release-please-manifest-beta.json index 9559ddcb..444c772f 100644 --- a/.release-please-manifest-beta.json +++ b/.release-please-manifest-beta.json @@ -1,3 +1,3 @@ { - ".": "3.126.1-beta.0" + ".": "3.126.2-beta.0" } diff --git a/CHANGELOG.md b/CHANGELOG.md index 7eb9b288..c53a0928 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,15 @@ All notable changes to PicPeak will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [3.126.2-beta.0](https://github.com/PicPeak/picpeak/compare/v3.126.1-beta.0...v3.126.2-beta.0) (2026-09-06) + + +### Bug Fixes + +* **gallery:** release grid tiles once they are far enough out of view ([b3937d0](https://github.com/PicPeak/picpeak/commit/b3937d0b8c54c8ddf8a428be88aa444ab2b4f2d2)) +* **gallery:** retry a failed image fetch once the tile is back on screen ([c4b03a8](https://github.com/PicPeak/picpeak/commit/c4b03a831f843ec447a54d712aefa89c9761e8e8)) +* **gallery:** retry a failed image fetch once the tile is back on screen ([77ae94e](https://github.com/PicPeak/picpeak/commit/77ae94e649f367bb0a44166d3215ce1884c660d2)) + ## [3.126.1-beta.0](https://github.com/PicPeak/picpeak/compare/v3.126.0-beta.0...v3.126.1-beta.0) (2026-09-06) diff --git a/backend/package.json b/backend/package.json index 2ca11a3a..2b51302b 100644 --- a/backend/package.json +++ b/backend/package.json @@ -1,6 +1,6 @@ { "name": "picpeak-backend", - "version": "3.126.1-beta.0", + "version": "3.126.2-beta.0", "description": "Backend for PicPeak event photo sharing platform", "main": "server.js", "engines": { diff --git a/backend/server.js b/backend/server.js index 33c3aa7a..02a7c715 100644 --- a/backend/server.js +++ b/backend/server.js @@ -239,7 +239,12 @@ const corsOptions = { // deployments can read the server's chosen download filename. Used // by the gallery/admin download flows to honour the #493 "original // camera filename" toggle on individual photo downloads (#507). - exposedHeaders: ['Content-Disposition'], + // + // Retry-After is not CORS-safelisted either. AuthenticatedImage reads it + // off a 429 to wait out the rate-limit window before retrying a thumbnail + // fetch; without it a split-origin deployment would spend its retry budget + // inside the window and leave the tile blank after the limit had lifted. + exposedHeaders: ['Content-Disposition', 'Retry-After'], }; // Only attach CORS to API endpoints, not static assets diff --git a/frontend/package.json b/frontend/package.json index c9bbdaa1..8e030c4f 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "picpeak-frontend", "private": true, - "version": "3.126.1-beta.0", + "version": "3.126.2-beta.0", "type": "module", "scripts": { "dev": "vite", diff --git a/frontend/src/components/common/AuthenticatedImage.tsx b/frontend/src/components/common/AuthenticatedImage.tsx index 0437413a..0db0b7ad 100644 --- a/frontend/src/components/common/AuthenticatedImage.tsx +++ b/frontend/src/components/common/AuthenticatedImage.tsx @@ -30,6 +30,44 @@ interface AuthenticatedImageProps extends Omit + status >= 400 && status < 500 && status !== 408 && status !== 429; + /** * Fetches an image with the gallery's bearer token and renders it. * @@ -76,6 +114,17 @@ export const AuthenticatedImage: React.FC = ({ const [canvasFailed, setCanvasFailed] = useState(false); const canvasRef = useRef(null); const imageRef = useRef(null); + // Retry state (#1287). The nonce is a dependency of the fetch effect, so + // bumping it is the retry; the counter is per src, so a new image gets a + // fresh budget without an extra effect run to reset it. + const [retryNonce, setRetryNonce] = useState(0); + const attemptsRef = useRef(0); + // Cooldown the server asked for on the last failure (#1287). The backoff + // alone would spend all three retries inside a 15-minute rate-limit window + // and leave the tile blank after the limit had actually lifted. + const retryAfterRef = useRef(0); + const lastSrcRef = useRef(undefined); + const retryRef = useRef(null); // Draw image to canvas when canvas rendering is enabled // Returns whether the pixels actually made it onto the canvas, so the @@ -108,9 +157,15 @@ export const AuthenticatedImage: React.FC = ({ // screen, which on a several-hundred-photo gallery is most of them. const controller = new AbortController(); + if (lastSrcRef.current !== src) { + lastSrcRef.current = src; + attemptsRef.current = 0; + } + // Determine which token to use based on context if (!src) { setImageSrc(fallbackSrc || ''); + setError(false); setIsLoading(false); return; } @@ -175,7 +230,11 @@ export const AuthenticatedImage: React.FC = ({ }); if (!response.ok) { - throw new Error(`Failed to fetch image: ${response.status} ${response.statusText}`); + throw new HttpError( + response.status, + response.statusText, + response.headers?.get?.('Retry-After') ?? null, + ); } return await response.blob(); @@ -203,6 +262,13 @@ export const AuthenticatedImage: React.FC = ({ // Returning here also stops the fallback below from firing a second // request against an already-aborted signal. if (aborted) return; + // A final 4xx exhausts the retry budget: on a 68-tile viewport, three + // retries per tile against an expired token would be ~200 requests + // that cannot succeed. + if (err instanceof HttpError && isFinalStatus(err.status)) { + attemptsRef.current = MAX_RETRIES; + } + retryAfterRef.current = err instanceof HttpError ? err.retryAfterMs : 0; setIsLoading(false); if (fallbackSrc && fallbackSrc !== src) { try { @@ -238,7 +304,65 @@ export const AuthenticatedImage: React.FC = ({ objectUrls.forEach((url) => URL.revokeObjectURL(url)); }; // eslint-disable-next-line react-hooks/exhaustive-deps - }, [src, fallbackSrc, slug, queuePriority]); + }, [src, fallbackSrc, slug, queuePriority, retryNonce]); + + // Retry a failed fetch once the tile is back on screen (#1287). + // + // Before this, a rejected fetch set `error` and nothing ever asked again: + // the effect above only re-runs when its inputs change, and for a grid + // tile they never do. On the original reporter's install that was the + // difference between a transient failure and a permanently blank tile — + // a hiccup on cellular, or Safari cancelling loads when the tab goes to + // the background, left a tile with no image, no request in flight and + // nothing in any log, for as long as the gallery stayed open. That is the + // retry-on-scrolling-back-into-view the reporter asked for in the issue. + // + // Bounded, and gated on visibility. The delay doubles per attempt so a + // server that is actually down is not hammered, and an attempt does not + // fire until the placeholder intersects the viewport and the document is + // visible — a tile that failed while backgrounded retries when the user + // comes back, not while they are still away. Without IntersectionObserver + // the placeholder counts as visible. + const retryable = error && !fallbackSrc && attemptsRef.current < MAX_RETRIES; + useEffect(() => { + if (!retryable) return; + const el = retryRef.current; + if (!el) return; + + let cancelled = false; + let delayElapsed = false; + let onScreen = typeof IntersectionObserver === 'undefined'; + + const retry = () => { + if (cancelled || !delayElapsed || !onScreen) return; + if (typeof document !== 'undefined' && document.visibilityState === 'hidden') return; + cancelled = true; + attemptsRef.current += 1; + setRetryNonce((n) => n + 1); + }; + + const timer = setTimeout(() => { + delayElapsed = true; + retry(); + }, Math.max(RETRY_BASE_DELAY_MS * 2 ** attemptsRef.current, retryAfterRef.current)); + + let observer: IntersectionObserver | null = null; + if (typeof IntersectionObserver !== 'undefined') { + observer = new IntersectionObserver((entries) => { + onScreen = entries.some((entry) => entry.isIntersecting); + retry(); + }); + observer.observe(el); + } + document.addEventListener('visibilitychange', retry); + + return () => { + cancelled = true; + clearTimeout(timer); + observer?.disconnect(); + document.removeEventListener('visibilitychange', retry); + }; + }, [retryable, retryNonce]); // Effect to draw to canvas when image is loaded and canvas rendering is enabled useEffect(() => { @@ -314,6 +438,15 @@ export const AuthenticatedImage: React.FC = ({ return {alt}; } + if (error) { + // Same box as the loading placeholder, and the element the retry effect + // observes. Returning null here (as this used to) left nothing to watch + // and nothing for the user to see either. + return ( +
+ ); + } + if (!imageSrc) { return null; } diff --git a/frontend/src/components/common/__tests__/AuthenticatedImage.retry.test.tsx b/frontend/src/components/common/__tests__/AuthenticatedImage.retry.test.tsx new file mode 100644 index 00000000..fb3c82ef --- /dev/null +++ b/frontend/src/components/common/__tests__/AuthenticatedImage.retry.test.tsx @@ -0,0 +1,216 @@ +/** + * Retry after a failed fetch (#1287). + * + * A rejected fetch used to set `error`, render nothing, and never ask again: + * the fetch effect only re-runs when its inputs change, and for a grid tile + * they never do. A transient failure — a hiccup on cellular, Safari + * cancelling loads when the tab is backgrounded — was therefore a + * permanently blank tile with nothing in any log. + * + * The retry is bounded (three attempts, doubling delay) and gated on the + * placeholder being on screen and the document being visible. + */ +import { act, render } from '@testing-library/react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; + +vi.mock('../../../utils/galleryAuthStorage', () => ({ + getActiveGallerySlug: () => 'demo', + getGalleryToken: () => 'token', + inferGallerySlugFromLocation: () => 'demo', + resolveSlugFromRequestUrl: () => 'demo', +})); +vi.mock('../../../utils/url', () => ({ buildResourceUrl: (u: string) => `http://localhost${u}` })); + +import { AuthenticatedImage } from '../AuthenticatedImage'; + +const ok = () => Promise.resolve({ + ok: true, + blob: async () => new Blob(['x'], { type: 'image/png' }), +}); +const loadFailed = () => Promise.reject(new TypeError('Load failed')); +const status = (code: number, headers: Record = {}) => () => Promise.resolve({ + ok: false, + status: code, + statusText: 'x', + headers: { get: (name: string) => headers[name] ?? null }, +}); + +/** The observer callbacks registered by the component, so a test can drive them. */ +type IOCallback = (entries: Array<{ isIntersecting: boolean }>) => void; +let observers: IOCallback[]; + +beforeEach(() => { + vi.useFakeTimers(); + observers = []; + URL.createObjectURL = vi.fn(() => 'blob:mock') as unknown as typeof URL.createObjectURL; + URL.revokeObjectURL = vi.fn() as unknown as typeof URL.revokeObjectURL; +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.useRealTimers(); +}); + +/** Let every pending promise in the fetch/queue chain settle. */ +const flush = () => act(async () => { await vi.advanceTimersByTimeAsync(0); }); +const advance = (ms: number) => act(async () => { await vi.advanceTimersByTimeAsync(ms); }); + +/** IntersectionObserver stub that never fires on its own; tests drive it. */ +function stubIntersectionObserver() { + vi.stubGlobal('IntersectionObserver', class { + constructor(cb: IOCallback) { observers.push(cb); } + observe() {} + disconnect() {} + unobserve() {} + }); +} + +describe('AuthenticatedImage retry', () => { + it('retries a failed fetch after the delay and renders the image', async () => { + const fetchMock = vi.fn().mockImplementationOnce(loadFailed).mockImplementation(ok); + vi.stubGlobal('fetch', fetchMock); + + const { container } = render(); + await flush(); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(container.querySelector('img')).toBeNull(); + + await advance(1999); + expect(fetchMock).toHaveBeenCalledTimes(1); + + await advance(1); + await flush(); + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(container.querySelector('img')?.getAttribute('src')).toBe('blob:mock'); + }); + + it('gives up after three retries with a doubling delay', async () => { + const fetchMock = vi.fn().mockImplementation(loadFailed); + vi.stubGlobal('fetch', fetchMock); + + render(); + await flush(); + expect(fetchMock).toHaveBeenCalledTimes(1); + + await advance(2000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(2); + await advance(4000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(3); + await advance(8000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(4); + + await advance(60_000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(4); + }); + + it('waits until the placeholder is on screen before retrying', async () => { + stubIntersectionObserver(); + const fetchMock = vi.fn().mockImplementationOnce(loadFailed).mockImplementation(ok); + vi.stubGlobal('fetch', fetchMock); + + const { container } = render(); + await flush(); + expect(observers).toHaveLength(1); + + // Delay elapsed, but the tile is not on screen: no retry. + await advance(2000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(1); + + // Scrolled back into view: retry fires now, without another delay. + act(() => observers[0]([{ isIntersecting: true }])); + await flush(); + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(container.querySelector('img')).not.toBeNull(); + }); + + it('does not retry while the document is hidden, and does when it comes back', async () => { + const fetchMock = vi.fn().mockImplementationOnce(loadFailed).mockImplementation(ok); + vi.stubGlobal('fetch', fetchMock); + const visibility = vi.spyOn(document, 'visibilityState', 'get').mockReturnValue('hidden'); + + render(); + await flush(); + await advance(2000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(1); + + visibility.mockReturnValue('visible'); + act(() => { document.dispatchEvent(new Event('visibilitychange')); }); + await flush(); + expect(fetchMock).toHaveBeenCalledTimes(2); + visibility.mockRestore(); + }); + + it('does not retry a final 4xx such as an expired token', async () => { + const fetchMock = vi.fn().mockImplementation(status(401)); + vi.stubGlobal('fetch', fetchMock); + + render(); + await flush(); + await advance(60_000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('retries a 5xx and a 429', async () => { + const fetchMock = vi.fn() + .mockImplementationOnce(status(503)) + .mockImplementationOnce(status(429)) + .mockImplementation(ok); + vi.stubGlobal('fetch', fetchMock); + + const { container } = render(); + await flush(); + await advance(2000); await flush(); + await advance(4000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(3); + expect(container.querySelector('img')?.getAttribute('src')).toBe('blob:mock'); + }); + + it('waits out a Retry-After longer than the backoff before spending a retry', async () => { + const fetchMock = vi.fn() + .mockImplementationOnce(status(429, { 'Retry-After': '30' })) + .mockImplementation(ok); + vi.stubGlobal('fetch', fetchMock); + + const { container } = render(); + await flush(); + // The 2 s backoff alone would have fired here. + await advance(29_999); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(1); + await advance(1); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(container.querySelector('img')).not.toBeNull(); + }); + + it('leaves the fallbackSrc path alone', async () => { + const fetchMock = vi.fn().mockImplementation(loadFailed); + vi.stubGlobal('fetch', fetchMock); + + const { container } = render( + , + ); + await flush(); + // primary + fallback, then the plain takes over — no retry loop. + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(container.querySelector('img')?.getAttribute('src')).toBe('/static/fallback.png'); + await advance(60_000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it('a new src gets a fresh retry budget', async () => { + const fetchMock = vi.fn().mockImplementation(loadFailed); + vi.stubGlobal('fetch', fetchMock); + + const { rerender } = render(); + await flush(); + await advance(2000); await flush(); + await advance(4000); await flush(); + await advance(8000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(4); + + rerender(); + await flush(); + expect(fetchMock).toHaveBeenCalledTimes(5); + await advance(2000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(6); + }); +}); 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.