diff --git a/backend/__tests__/integration/previewTiers.test.js b/backend/__tests__/integration/previewTiers.test.js index 648ca2e8..8364aceb 100644 --- a/backend/__tests__/integration/previewTiers.test.js +++ b/backend/__tests__/integration/previewTiers.test.js @@ -188,4 +188,188 @@ describe('preview tiers (#1095)', () => { expect(meta.height).toBe(Math.round(640 * (2000 / 3000))); }); }); + + describe('thumbnail tiers', () => { + async function seedThumbPhoto(w = 3000, h = 2000) { + const [e] = await db('events').insert({ + slug: `tt-${Math.random().toString(36).slice(2, 8)}`, + event_type: 'wedding', event_name: 'tt', event_date: '2026-01-01', + host_email: 'h@example.com', admin_email: 'a@example.com', + password_hash: 'x', share_link: `tt-${Math.random()}`, + expires_at: new Date().toISOString(), + }).returning('id'); + const eventId = typeof e === 'object' ? e.id : e; + const rel = `events/active/tt/${Math.random().toString(36).slice(2, 8)}.jpg`; + const abs = path.join(process.env.STORAGE_PATH, rel); + await fs.promises.mkdir(path.dirname(abs), { recursive: true }); + await sharp({ create: { width: w, height: h, channels: 3, background: { r: 5, g: 5, b: 5 } } }) + .jpeg().toFile(abs); + const [p2] = await db('photos').insert({ + event_id: eventId, filename: path.basename(rel), + path: rel.replace(/^events\/active\//, ''), type: 'individual', + width: w, height: h, processing_status: 'complete', source_origin: 'managed', + }).returning('id'); + return db('photos').where({ id: typeof p2 === 'object' ? p2.id : p2 }).first(); + } + + it('scopes thumbnail tier keys by photo id', async () => { + // Same cross-gallery hazard the preview tiers had: a cache hit serves + // without re-reading the source, so a shared basename leaks across events. + const keys = imageProcessor.thumbnailTierKeys({ id: 42, path: 'a/IMG_0001.jpg', source_origin: 'managed' }); + expect(keys.every((k) => k.includes('p42_'))).toBe(true); + // Every width, canonical included: which one is canonical depends on the + // thumbnail_width setting, so on a 600-configured install w300 is the + // tier file. Deleting a key that was never written is a no-op; missing + // one strands it forever. + expect(keys).toHaveLength(3); + }); + + it('tags the tier against the configured width, not the 300 default', async () => { + // Regression: with thumbnail_width=600 a w=300 request wrote + // `thumb_` while the caller probed `thumb_w300_`. The cache + // never hit, so every request re-downloaded the original and ran Sharp, + // and the file it left behind was in no cleanup list. + await db('app_settings').where('setting_key', 'thumbnail_width') + .update({ setting_value: 600 }); + try { + const photo = await seedThumbPhoto(); + + const first = await imageProcessor.ensureThumbnailAtWidth(photo, 300); + expect(first).toContain('thumb_w300_'); + + // The second call must be a cache hit on the key the first one wrote. + const before = fs.statSync(path.join(process.env.STORAGE_PATH, first)).mtimeMs; + const second = await imageProcessor.ensureThumbnailAtWidth(photo, 300); + expect(second).toBe(first); + expect(fs.statSync(path.join(process.env.STORAGE_PATH, second)).mtimeMs).toBe(before); + + // ...and 600 is now the canonical, so it resolves to the plain thumbnail. + const canonical = await imageProcessor.ensureThumbnailAtWidth(photo, 600); + expect(canonical).not.toContain('thumb_w600_'); + + // Cleanup still reaches the w300 tier this install actually generated. + expect(imageProcessor.thumbnailTierKeys(photo)).toContain(first); + } finally { + await db('app_settings').where('setting_key', 'thumbnail_width') + .update({ setting_value: 300 }); + } + }); + + it('generates a tier at the requested size', async () => { + const photo = await seedThumbPhoto(); + const key = await imageProcessor.ensureThumbnailAtWidth(photo, 600); + expect(key).toContain('thumb_w600_'); + const meta = await sharp(path.join(process.env.STORAGE_PATH, key)).metadata(); + expect(Math.max(meta.width, meta.height)).toBe(600); + }); + + it('does not upscale past the source, which is why the tier is clamped', async () => { + // The reason tileThumbnailWidth checks the short edge: ask a 400px + // source for 900 and withoutEnlargement caps it, so the request buys a + // Sharp run and a second cache entry for a file identical to the 300. + const small = await seedThumbPhoto(500, 400); + const key = await imageProcessor.ensureThumbnailAtWidth(small, 900); + const meta = await sharp(path.join(process.env.STORAGE_PATH, key)).metadata(); + expect(Math.max(meta.width, meta.height)).toBeLessThan(900); + }); + + it('resolves the canonical width to the normal thumbnail', async () => { + const photo = await seedThumbPhoto(); + const key = await imageProcessor.ensureThumbnailAtWidth(photo, 300); + expect(key).not.toContain('thumb_w300_'); + }); + + it('keeps the configured aspect ratio instead of forcing a square', async () => { + // Thumbnails are square by default, but the settings API takes any + // width/height in 50..1000. With fit:'cover' a 300x200 canonical and a + // 600x600 tier are two different crops, so the photo would visibly + // reframe as the tile size changed. + await db('app_settings').where('setting_key', 'thumbnail_height') + .update({ setting_value: 200 }); + try { + const photo = await seedThumbPhoto(); + const key = await imageProcessor.ensureThumbnailAtWidth(photo, 600); + const meta = await sharp(path.join(process.env.STORAGE_PATH, key)).metadata(); + expect(meta.width).toBe(600); + expect(meta.height).toBe(400); // 600 * (200/300), not 600 + } finally { + await db('app_settings').where('setting_key', 'thumbnail_height') + .update({ setting_value: 300 }); + } + }); + + it('never hands a video to Sharp', async () => { + // A video's thumbnail is a poster frame from videoProcessor, not a + // resize of the stored file. Without the short-circuit the tier path + // would download the whole video (withLocalCopy, in full on S3) and + // then fail to decode it — every request, since nothing caches a miss. + const photo = await seedThumbPhoto(); + await db('photos').where({ id: photo.id }) + .update({ media_type: 'video', mime_type: 'video/mp4' }); + const video = await db('photos').where({ id: photo.id }).first(); + + const key = await imageProcessor.ensureThumbnailAtWidth(video, 900); + expect(key).not.toContain('thumb_w900_'); + }); + + it('drops tiers when a rename moves the basename they are keyed on', async () => { + // The key embeds the basename, so the DB update in renamePhotoFiles is + // the point past which the old keys cannot be derived at all — a later + // delete or archive computes the new ones and leaves these behind. + const renameService = require('../../src/services/eventRenameService'); + const photo = await seedThumbPhoto(); + const event = await db('events').where({ id: photo.event_id }).first(); + + // Give it a filename the rename will actually rewrite. + const dir = path.join(process.env.STORAGE_PATH, 'events/active', event.slug, 'individual'); + await fs.promises.mkdir(dir, { recursive: true }); + await sharp({ create: { width: 1200, height: 900, channels: 3, background: { r: 7, g: 7, b: 7 } } }) + .jpeg().toFile(path.join(dir, 'Old_Name_001.jpg')); + await db('photos').where({ id: photo.id }).update({ + filename: 'Old_Name_001.jpg', + path: `${event.slug}/individual/Old_Name_001.jpg`, + }); + const renamable = await db('photos').where({ id: photo.id }).first(); + + const key = await imageProcessor.ensureThumbnailAtWidth(renamable, 600); + const abs = path.join(process.env.STORAGE_PATH, key); + expect(fs.existsSync(abs)).toBe(true); + + await renameService.renamePhotoFiles( + event.id, 'Old Name', 'New Name', event.slug, event.slug + ); + + expect(await db('photos').where({ id: photo.id }).first()) + .toMatchObject({ filename: 'New_Name_001.jpg' }); + expect(fs.existsSync(abs)).toBe(false); + }); + + it('leaves tiers alone when a rename does not move the basename', async () => { + // Four storage deletes per photo is 20k calls against S3 for a + // 5,000-photo event whose slug merely changed, so the sweep is gated on + // the filename actually moving. + const renameService = require('../../src/services/eventRenameService'); + const photo = await seedThumbPhoto(); + const event = await db('events').where({ id: photo.event_id }).first(); + + const key = await imageProcessor.ensureThumbnailAtWidth(photo, 600); + const abs = path.join(process.env.STORAGE_PATH, key); + + // The photo's filename carries no event-name prefix, so nothing moves. + await renameService.renamePhotoFiles( + event.id, 'Old Name', 'New Name', event.slug, event.slug + ); + + expect(fs.existsSync(abs)).toBe(true); + }); + + it('deleteThumbnailTiers removes them', async () => { + const photo = await seedThumbPhoto(); + const key = await imageProcessor.ensureThumbnailAtWidth(photo, 600); + const abs = path.join(process.env.STORAGE_PATH, key); + expect(fs.existsSync(abs)).toBe(true); + await imageProcessor.deleteThumbnailTiers(await db('photos').where({ id: photo.id }).first()); + expect(fs.existsSync(abs)).toBe(false); + }); + }); }); diff --git a/backend/src/routes/adminEvents/helpers.js b/backend/src/routes/adminEvents/helpers.js index 5b24a345..674129b0 100644 --- a/backend/src/routes/adminEvents/helpers.js +++ b/backend/src/routes/adminEvents/helpers.js @@ -224,6 +224,16 @@ async function deleteEventCascade(eventId, adminContext) { throw err; } + // Responsive tiers (#1095 / #492) live in the top-level thumbnails/ and + // previews/ directories, not under the event folder the filesystem sweep + // below removes, and their keys are derived from the photo rows — which the + // transaction is about to delete. So they are read here, while the rows + // still exist, and swept after the commit; miss that window and every tier + // this event generated is orphaned with nothing left to derive its key from. + const tieredPhotos = await db('photos') + .where('event_id', eventId) + .select('id', 'path', 'filename', 'source_origin', 'external_relpath'); + await db.transaction(async (trx) => { // 1. Delete activity logs (audit trail) await trx('activity_logs').where('event_id', eventId).del(); @@ -292,6 +302,19 @@ async function deleteEventCascade(eventId, adminContext) { } }); + // Tier sweep, post-commit and best-effort for the same reason as the folder + // removal above: an orphaned derivative is recoverable noise, a rolled-back + // delete is not. + try { + const { deleteThumbnailTiers, deletePreviewTiers } = require('../../services/imageProcessor'); + for (const photo of tieredPhotos) { + await deleteThumbnailTiers(photo); + await deletePreviewTiers(photo); + } + } catch (tierErr) { + logger.warn('Failed to delete responsive tiers during cascade delete', { eventId, error: tierErr.message }); + } + // Audit trail (outside the transaction so a logging failure can't undo // the actual delete). await logActivity('event_deleted', diff --git a/backend/src/routes/adminPhotos.js b/backend/src/routes/adminPhotos.js index 0c3e400f..40abd792 100644 --- a/backend/src/routes/adminPhotos.js +++ b/backend/src/routes/adminPhotos.js @@ -682,6 +682,7 @@ router.delete('/:eventId/photos/:photoId', adminAuth, requirePermission('photos. // independently, on demand — so keying their cleanup off preview_path // would strand exactly the photos that were only ever viewed on a phone. await require('../services/imageProcessor').deletePreviewTiers(photo); + await require('../services/imageProcessor').deleteThumbnailTiers(photo); // Delete pre-generated watermark if exists if (photo.watermark_path) { @@ -846,6 +847,7 @@ router.post('/:eventId/photos/bulk-delete', adminAuth, requirePermission('photos // Outside the guard: a tier can exist when the canonical rendition never // did, so keying cleanup off preview_path would strand phone-only photos. await require('../services/imageProcessor').deletePreviewTiers(photo); + await require('../services/imageProcessor').deleteThumbnailTiers(photo); if (photo.watermark_path) { await watermarkGeneratorService.deleteForPhoto(photo.id); } diff --git a/backend/src/routes/adminThumbnails.js b/backend/src/routes/adminThumbnails.js index 97a09fac..4bd303b2 100644 --- a/backend/src/routes/adminThumbnails.js +++ b/backend/src/routes/adminThumbnails.js @@ -125,7 +125,11 @@ router.post('/regenerate', adminAuth, requirePermission('photos.edit'), async (r try { const { eventId } = req.body; // Optional: regenerate for specific event only - let query = db('photos').select('id', 'event_id', 'path'); + // source_origin/external_relpath/filename are selected for + // deleteThumbnailTiers below — it derives the tier keys from the same + // fields ensureThumbnailAtWidth used to write them. + let query = db('photos') + .select('id', 'event_id', 'path', 'source_origin', 'external_relpath', 'filename'); if (eventId) { query = query.where('event_id', eventId); } @@ -151,7 +155,21 @@ router.post('/regenerate', adminAuth, requirePermission('photos.edit'), async (r try { const storagePath = getStoragePath(); const originalPath = path.join(storagePath, 'events/active', photo.path); - + + // Drop the responsive tiers first (#1095), same as the preview + // endpoint below. They are cached by width outside thumbnail_path + // and their key carries no settings version, so regenerating only + // the canonical rendition leaves phones served the old fit, quality + // or format indefinitely — which is exactly what this endpoint is + // invoked to undo after a settings change. + // + // Above the fs.access below, not after it: that check only passes + // for managed photos on a local filesystem. On S3, and for external + // or reference rows, it fails and skips the photo — so invalidating + // after it would leave stale tiers on precisely the deployments + // where they are hardest to notice. + await require('../services/imageProcessor').deleteThumbnailTiers(photo); + // Check if original file exists try { await fs.access(originalPath); @@ -160,7 +178,7 @@ router.post('/regenerate', adminAuth, requirePermission('photos.edit'), async (r errorCount++; continue; } - + // Regenerate thumbnail const thumbnailPath = await generateThumbnail(originalPath, { regenerate: true }); diff --git a/backend/src/routes/gallery.js b/backend/src/routes/gallery.js index f20b806c..b9977288 100644 --- a/backend/src/routes/gallery.js +++ b/backend/src/routes/gallery.js @@ -2251,7 +2251,26 @@ router.get('/:slug/thumbnail/:photoId', } // Ensure thumbnail exists and is valid, regenerate if needed - const thumbnailPath = await ensureThumbnail(photo); + // Responsive tier (#1095), whitelisted the same way the preview route's + // is. Unrecognised or absent falls through to the canonical 300px + // thumbnail, so existing clients are untouched. + const { THUMBNAIL_WIDTHS, normalizeTierWidth, ensureThumbnailAtWidth } = + require('../services/imageProcessor'); + const thumbTier = normalizeTierWidth(req.query.w, THUMBNAIL_WIDTHS); + + const thumbnailPath = thumbTier + ? (await ensureThumbnailAtWidth(photo, thumbTier)) || (await ensureThumbnail(photo)) + : await ensureThumbnail(photo); + + // What was actually resolved, not what was asked for. A tier request can + // land on the canonical thumbnail — generation failed, or the row is a + // video — and stamping the requested tier into the ETag below would then + // have the client cache a 300px image under its 900px key for the full + // max-age, with no way to notice. + const servedTier = thumbTier && thumbnailPath + && path.basename(thumbnailPath).startsWith(`thumb_w${thumbTier}_`) + ? thumbTier + : null; if (!thumbnailPath) { logger.error(`Failed to generate thumbnail for photo ${photoId}`); @@ -2285,7 +2304,10 @@ router.get('/:slug/thumbnail/:photoId', const watermarkHash = watermarkSettings?.enabled ? `-wm${watermarkSettings.opacity}${watermarkSettings.position}${watermarkSettings.size}` : '-nowm'; - const etag = `"thumb-${photoId}-${mtimeMs}${watermarkHash}"`; + // Tier in the ETag, same reason as the preview route: without it a + // client holding the 300px thumbnail gets a 304 for its 600px request + // and renders the small one, which is this feature inverted. + const etag = `"thumb-${photoId}-${servedTier || 'def'}-${mtimeMs}${watermarkHash}"`; // Check if client has valid cached version if (req.headers['if-none-match'] === etag) { diff --git a/backend/src/services/archiveService.js b/backend/src/services/archiveService.js index fad7d998..8fb3c7da 100644 --- a/backend/src/services/archiveService.js +++ b/backend/src/services/archiveService.js @@ -222,6 +222,7 @@ async function archiveEvent(event) { // Outside the guard: a tier can exist when the canonical rendition never // did, so keying cleanup off preview_path would strand phone-only photos. await require('./imageProcessor').deletePreviewTiers(photo); + await require('./imageProcessor').deleteThumbnailTiers(photo); // Best effort: remove watermarked variants too if a refactor added them. if (photo.watermark_path) { await storage.delete(photo.watermark_path).catch(() => {}); diff --git a/backend/src/services/eventRenameService.js b/backend/src/services/eventRenameService.js index 4f16580a..9488aa9f 100644 --- a/backend/src/services/eventRenameService.js +++ b/backend/src/services/eventRenameService.js @@ -181,6 +181,20 @@ class EventRenameService { logger.warn('Could not rename photo file', { oldFilename, error: error.message }); } } + + // Responsive tiers (#1095 / #492) are keyed off the basename, so the + // update below is the point past which the old keys can no longer be + // derived — a later delete or archive would compute the new ones and + // leave these in storage forever. Dropped rather than renamed: they + // are a pure cache and the next request regenerates. + // + // Inside this branch, not the loop body: only a filename change moves + // the key. Sweeping unconditionally would fire four storage deletes + // per photo on every rename, which is 20k calls against S3 for a + // 5,000-photo event that merely had its slug adjusted. + const imageProcessor = require('./imageProcessor'); + await imageProcessor.deleteThumbnailTiers(photo); + await imageProcessor.deletePreviewTiers(photo); } // Update database record diff --git a/backend/src/services/imageProcessor.js b/backend/src/services/imageProcessor.js index de7a0d1e..9e8a8842 100644 --- a/backend/src/services/imageProcessor.js +++ b/backend/src/services/imageProcessor.js @@ -212,13 +212,24 @@ const contentTypeFor = (format) => { async function generateThumbnail(imagePath, options = {}) { const sourceBasename = path.basename(imagePath); const outputBasename = options.outputBasename || sourceBasename; - const thumbnailFilename = `thumb_${outputBasename}`; - const thumbnailRelKey = path.posix.join('thumbnails', thumbnailFilename); const storage = getStorage(); // Get thumbnail settings const settings = await getThumbnailSettings(); + // Tag against the CONFIGURED canonical width, not the 300 default — the tag + // has to agree with the key ensureThumbnailAtWidth probed for. On an install + // with thumbnail_width=600 a w=300 request used to write `thumb_` while + // the caller looked for `thumb_w300_`: the cache never hit, so every + // single request re-downloaded the original and ran Sharp, and the file it + // left behind was in no cleanup list. + const canonicalWidth = settings.width || DEFAULT_THUMBNAIL_WIDTH; + const widthTag = options.width && options.width !== canonicalWidth + ? `w${options.width}_` + : ''; + const thumbnailFilename = `thumb_${widthTag}${outputBasename}`; + const thumbnailRelKey = path.posix.join('thumbnails', thumbnailFilename); + // Force regeneration: drop the existing object before writing the new one if (options.regenerate) { await storage.delete(thumbnailRelKey).catch(() => {}); @@ -241,7 +252,12 @@ async function generateThumbnail(imagePath, options = {}) { // Strip EXIF/metadata from thumbnails (privacy: prevent GPS leak etc.) sharpInstance = sharpInstance.withMetadata(false); - sharpInstance = sharpInstance.resize(settings.width, settings.height, { + // options.width/height override the admin setting for responsive tiers + // (#1095). The configured `fit` is kept deliberately: the grid renders + // with object-cover, so every tier must be cropped the same way or the + // browser would swap between differently-framed images as the viewport + // changes. + sharpInstance = sharpInstance.resize(options.width || settings.width, options.height || settings.height, { withoutEnlargement: true, fit: settings.fit, position: 'center' @@ -731,6 +747,119 @@ async function deletePreviewTiers(photo) { await Promise.all(previewTierKeys(photo).map((k) => storage.delete(k).catch(() => {}))); } +/** + * Thumbnail storage keys for every responsive tier of a photo (#1095). + * Mirrors previewTierKeys — see there for why they are derived rather than + * tracked. + * + * Every width is listed, the canonical one included, and deliberately: which + * width is canonical depends on the thumbnail_width setting, so on a + * 600-configured install it is w300 that exists as a tier file. Reading the + * setting here would make the whole cleanup path async for no gain — deleting + * a key that was never written is already a swallowed no-op, so the inclusive + * list is both simpler and the one that cannot strand a file. + */ +function thumbnailTierKeys(photo) { + if (!photo) return []; + const isExternal = photo.source_origin === 'external' || photo.source_origin === 'reference'; + const sourceBasename = path.basename( + (isExternal ? (photo.external_relpath || photo.filename) : photo.path) || `photo-${photo.id}` + ); + const outputBasename = `p${photo.id}_${sourceBasename}`; + return THUMBNAIL_WIDTHS + .map((w) => path.posix.join('thumbnails', `thumb_w${w}_${outputBasename}`)); +} + +async function deleteThumbnailTiers(photo) { + const storage = getStorage(); + await Promise.all(thumbnailTierKeys(photo).map((k) => storage.delete(k).catch(() => {}))); +} + +/** + * A thumbnail at a specific tier width (#1095). + * + * Same contract as ensurePreviewImageAtWidth: pure cache, keyed by width, + * never written to photos.thumbnail_path. The key is scoped by photo id for + * every source type — basenames are not unique across events, and a tier is + * served from a cache hit without re-reading the source, so an unscoped key + * would hand one gallery's photo to another. + */ +async function ensureThumbnailAtWidth(photo, width) { + if (!width) return ensureThumbnail(photo); + + // Against the CONFIGURED canonical width, not the 300 default. An install + // that set thumbnail_width to 600 already has a 600px thumbnail; generating + // a w600 tier for it would download the original and run Sharp to produce a + // byte-equivalent duplicate, once per photo. + const settings = await getThumbnailSettings(); + const canonicalWidth = settings.width || DEFAULT_THUMBNAIL_WIDTH; + if (width === canonicalWidth) return ensureThumbnail(photo); + + // Videos never take the tier path. Their thumbnail is a poster frame from + // videoProcessor, not a resize of the stored file, so the code below would + // hand the video itself to Sharp — after withLocalCopy has downloaded the + // whole thing on an S3 backend. Nothing caches that failure, so a crawler + // walking ?w= over a gallery of videos repeats the download every request. + if (photo.media_type === 'video' || String(photo.mime_type || '').startsWith('video/')) { + return ensureThumbnail(photo); + } + + const { resolvePhotoStorageKey, resolvePhotoFilePath } = require('./photoResolver'); + const storage = getStorage(); + + let event; + try { + event = await db('events').where('id', photo.event_id).first(); + } catch (e) { + return null; + } + if (!event) return null; + + const isExternal = photo.source_origin === 'external' || photo.source_origin === 'reference'; + const sourceBasename = path.basename( + (isExternal ? (photo.external_relpath || photo.filename) : photo.path) || `photo-${photo.id}` + ); + const outputBasename = `p${photo.id}_${sourceBasename}`; + const key = path.posix.join('thumbnails', `thumb_w${width}_${outputBasename}`); + + try { + if (await storage.stat(key)) return key; + } catch (e) { + // regenerate below + } + + // Scale the height from the configured aspect ratio rather than forcing a + // square. Thumbnails are square on a default install, but the settings API + // accepts any width/height in 50..1000 — and with fit:'cover' a 300x200 + // canonical next to a 600x600 tier are two different crops, so the photo + // would visibly reframe as the tile size changes. + const height = Math.round(width * (settings.height / canonicalWidth)); + + try { + if (isExternal) { + const localPath = resolvePhotoFilePath(event, photo); + return await generateThumbnail(localPath, { + regenerate: true, outputBasename, width, height, + }); + } + const sourceKey = resolvePhotoStorageKey(event, photo); + if (!sourceKey) return null; + return await withLocalCopy(sourceKey, async (localPath) => { + const proc = await withProcessableImage(localPath, sourceKey); + try { + return await generateThumbnail(proc.path, { + regenerate: true, outputBasename, width, height, + }); + } finally { + proc.cleanup(); + } + }); + } catch (e) { + logger.warn(`Thumbnail tier w${width} failed for photo ${photo.id}: ${e.message}`); + return null; + } +} + async function ensurePreviewImageAtWidth(photo, width) { if (!width || width === DEFAULT_PREVIEW_LONG_EDGE) return ensurePreviewImage(photo); @@ -981,6 +1110,9 @@ async function resizeToBox(inputBuffer, box, options = {}) { module.exports = { ensurePreviewImageAtWidth, + ensureThumbnailAtWidth, + thumbnailTierKeys, + deleteThumbnailTiers, previewTierKeys, deletePreviewTiers, PREVIEW_WIDTHS, diff --git a/backend/src/services/photoReplacementService.js b/backend/src/services/photoReplacementService.js index 9fc918c9..4f98be78 100644 --- a/backend/src/services/photoReplacementService.js +++ b/backend/src/services/photoReplacementService.js @@ -10,7 +10,10 @@ const path = require('path'); const fsp = require('fs/promises'); const sharp = require('sharp'); const { db } = require('../database/db'); -const { generateThumbnail, extractCaptureDate, withProcessableImage } = require('./imageProcessor'); +const { + generateThumbnail, extractCaptureDate, withProcessableImage, + deleteThumbnailTiers, deletePreviewTiers, +} = require('./imageProcessor'); const { generatePhotoFilename } = require('../utils/filenameSanitizer'); const watermarkGeneratorService = require('./watermarkGeneratorService'); const { getStorage } = require('./storage'); @@ -98,6 +101,12 @@ async function replacePhoto(existingPhoto, newFileTempPath, { originalFilename, if (existingPhoto.thumbnail_path && existingPhoto.thumbnail_path !== thumbnailPath) { await storage.delete(existingPhoto.thumbnail_path).catch(() => {}); } + // Responsive tiers, keyed off the OLD row (#1095 / #492). Their key embeds + // the basename, which the update below replaces — so this is the last + // moment they can be derived at all. Miss it and a later delete or archive + // computes keys from the new basename and leaves them in storage forever. + await deleteThumbnailTiers(existingPhoto); + await deletePreviewTiers(existingPhoto); try { await watermarkGeneratorService.deleteForPhoto(existingPhoto.id); } catch { diff --git a/frontend/src/components/gallery/PhotoCard.tsx b/frontend/src/components/gallery/PhotoCard.tsx index 6c9cd73e..acd09954 100644 --- a/frontend/src/components/gallery/PhotoCard.tsx +++ b/frontend/src/components/gallery/PhotoCard.tsx @@ -1,7 +1,8 @@ -import React, { useCallback, useEffect, useRef, useState } from 'react'; +import React, { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react'; import { Download, Maximize2, Check, MessageSquare, Heart } from 'lucide-react'; import { useInView } from 'react-intersection-observer'; import { AuthenticatedImage } from '../common'; +import { thumbnailUrlForTile } from './imageTiers'; import { FeedbackIdentityModal } from './FeedbackIdentityModal'; import { feedbackService } from '../../services/feedback.service'; import { useGuestIdentityOptional } from '../../contexts/GuestIdentityContext'; @@ -193,6 +194,34 @@ export const PhotoCard: React.FC = ({ }); const inView = !lazy || observedInView; + // Tile width for the responsive tier (#1095), measured rather than inferred. + // The observer entry only exists for `lazy` cards, and Mosaic, Masonry and + // Timeline do not pass it — Mosaic is 1-up on mobile where Grid is 2-up, so + // those are exactly the layouts a breakpoint guess gets most wrong. + // + // Gated: the image is not rendered until this has run, so AuthenticatedImage + // never mounts with a src it would have to replace. Attaching the observer + // ref unconditionally instead would refetch every tile — React flushes + // passive effects before the synchronous re-render a layout effect triggers, + // so the fetch fires once with the fallback and again with the measurement. + // + // useLayoutEffect, so the extra commit lands before paint and the skeleton + // branch below is never actually seen. One reflow per commit, not per card: + // nothing writes to the DOM between the reads, so the browser batches them. + const containerRef = useRef(null); + const [tile, setTile] = useState<{ width: number | null } | null>(null); + const setContainerRef = useCallback((node: HTMLDivElement | null) => { + containerRef.current = node; + if (lazy) ref(node); + }, [ref, lazy]); + + useLayoutEffect(() => { + if (!inView || tile) return; + // 0 means not laid out (a hidden tab, say), not a 0px tile — null falls + // back to the viewport estimate rather than pinning the smallest tier. + setTile({ width: containerRef.current?.offsetWidth || null }); + }, [inView, tile]); + const showFeedbackActions = feedbackEnabled && Boolean(feedbackOptions); const overlayVisibilityClass = overlayVisible @@ -322,17 +351,32 @@ export const PhotoCard: React.FC = ({ ) : null; + // Responsive grid tier (#1095). Applied here rather than in each layout + // because six of the seven funnel their tile through this one image; the + // seventh, Carousel, renders 80px filmstrip thumbs that the canonical 300 + // already covers at DPR 3. + // + // Only when the src IS the thumbnail route: layouts fall back to photo.url + // when thumbnail_url is null, and ?w= on the original-photo route means + // something else. Videos are excluded because their thumbnail is a poster + // frame from the video pipeline — the tier route would hand the video file + // itself to Sharp. + const isVideo = photo.media_type === 'video' || photo.type === 'video'; + const tileSrc = !isVideo && photo.thumbnail_url && imageProps.src === photo.thumbnail_url + ? (thumbnailUrlForTile(photo.thumbnail_url, photo, tile?.width) ?? imageProps.src) + : imageProps.src; + return (
- {inView ? ( + {inView && tile ? ( <> - + {beforeOverlay} diff --git a/frontend/src/components/gallery/__tests__/PhotoCard.tierSizing.test.tsx b/frontend/src/components/gallery/__tests__/PhotoCard.tierSizing.test.tsx new file mode 100644 index 00000000..92e5135c --- /dev/null +++ b/frontend/src/components/gallery/__tests__/PhotoCard.tierSizing.test.tsx @@ -0,0 +1,139 @@ +/** + * Grid tile sizing (#1095). + * + * The tier has to be decided from the tile's real width, and it has to be + * decided BEFORE the image is requested. Both halves are easy to break without + * anything looking wrong: the picture still renders, just at the wrong size, or + * at the right size after fetching the wrong one first. + */ +import React from 'react'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { render, screen } from '@testing-library/react'; + +import { PhotoCard } from '../PhotoCard'; +import type { Photo } from '../../../types'; + +// AuthenticatedImage really fetches; all this test cares about is the src it +// was handed, and how many distinct ones it saw. +const seenSrcs: string[] = []; +vi.mock('../../common', () => ({ + AuthenticatedImage: ({ src, alt }: { src: string; alt?: string }) => { + seenSrcs.push(src); + return {alt}; + }, +})); + +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', + width: 4000, + height: 3000, +} as Photo; + +/** Every tile in the document reports `width` CSS px. */ +function stubTileWidth(width: number) { + Object.defineProperty(HTMLElement.prototype, 'offsetWidth', { + configurable: true, + get() { return width; }, + }); +} + +function renderCard(props: Partial> = {}) { + return render( + {}} + onDownload={() => {}} + onToggleSelect={() => {}} + className="tile" + overlayBaseClassName="overlay" + imageProps={{ src: PHOTO.thumbnail_url!, alt: PHOTO.filename }} + {...props} + />, + ); +} + +beforeEach(() => { + seenSrcs.length = 0; + Object.defineProperty(window, 'devicePixelRatio', { value: 3, configurable: true }); + Object.defineProperty(window, 'innerWidth', { value: 390, configurable: true }); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('PhotoCard tier sizing', () => { + it('sizes from the measured tile, not the viewport', () => { + // A 1-up Mosaic tile on a DPR-3 phone needs 1170 device px. The viewport + // fallback assumes 2-up and would land on 600 — visibly soft. + stubTileWidth(390); + renderCard(); + expect(screen.getByTestId('tile')).toHaveAttribute( + 'src', '/api/gallery/x/thumbnail/7?w=900', + ); + }); + + it('gives a dense grid the small file', () => { + stubTileWidth(96); + renderCard(); + // The canonical tier carries no ?w=, so these URLs stay byte-identical to + // the ones already in browser caches. + expect(screen.getByTestId('tile')).toHaveAttribute( + 'src', '/api/gallery/x/thumbnail/7', + ); + }); + + it('requests exactly one URL — never a fallback then a correction', () => { + // The measurement gate exists for this. Reading the tile from a plain + // effect would mount the image with the viewport guess, fetch it, then + // swap the src and fetch again — every tile in the gallery, twice. + stubTileWidth(390); + renderCard(); + expect(new Set(seenSrcs).size).toBe(1); + expect(seenSrcs[0]).toContain('?w=900'); + }); + + it('measures non-lazy cards too', () => { + // Mosaic, Masonry and Timeline do not pass `lazy`, so the observer entry + // is never populated for them — they are precisely the layouts a + // breakpoint guess gets most wrong. + stubTileWidth(390); + renderCard({ lazy: false }); + expect(screen.getByTestId('tile')).toHaveAttribute( + 'src', '/api/gallery/x/thumbnail/7?w=900', + ); + }); + + it('leaves videos on the canonical thumbnail', () => { + // A video's thumbnail is a poster frame, so the tier route would hand the + // video file itself to Sharp. + stubTileWidth(390); + renderCard({ photo: { ...PHOTO, media_type: 'video' } as Photo }); + expect(screen.getByTestId('tile')).toHaveAttribute( + 'src', '/api/gallery/x/thumbnail/7', + ); + }); + + it('does not put ?w= on the original-photo route', () => { + // Layouts fall back to photo.url when thumbnail_url is null, and ?w= means + // something else there. + stubTileWidth(390); + renderCard({ + photo: { ...PHOTO, thumbnail_url: undefined } as Photo, + imageProps: { src: PHOTO.url, alt: PHOTO.filename }, + }); + expect(screen.getByTestId('tile')).toHaveAttribute('src', '/api/gallery/x/photo/7'); + }); +}); diff --git a/frontend/src/components/gallery/__tests__/imageTiers.test.ts b/frontend/src/components/gallery/__tests__/imageTiers.test.ts index 4e7ac303..bed196ce 100644 --- a/frontend/src/components/gallery/__tests__/imageTiers.test.ts +++ b/frontend/src/components/gallery/__tests__/imageTiers.test.ts @@ -8,7 +8,10 @@ */ import { describe, it, expect, afterEach, vi } from 'vitest'; -import { previewUrlForViewport, viewportPreviewWidth, PREVIEW_WIDTHS } from '../imageTiers'; +import { + previewUrlForViewport, viewportPreviewWidth, thumbnailUrlForTile, tileThumbnailWidth, + PREVIEW_WIDTHS, +} from '../imageTiers'; const realWidth = window.innerWidth; const realDpr = window.devicePixelRatio; @@ -131,3 +134,107 @@ describe('previewUrlForViewport', () => { expect(previewUrlForViewport('/p', LANDSCAPE)).toBe('/p?w=640'); }); }); + +// A source big enough to fill every tier, so the tier comes from the tile +// geometry rather than the short-edge clamp. +const BIG = { width: 4000, height: 3000 }; + +describe('tileThumbnailWidth', () => { + it('sizes from the tile, not the viewport', () => { + // The bug in #1095: a 2-up phone tile is ~195 CSS px, which is 585 device + // px at DPR 3 — the 300px thumbnail upscaled ~1.9x. + setViewport(390, 3); + expect(tileThumbnailWidth(BIG)).toBe(600); + + // Same tile on a DPR-1 phone genuinely only needs 195, so it keeps the + // small file. Sizing off the viewport alone would have shipped 600 here. + setViewport(390, 1); + expect(tileThumbnailWidth(BIG)).toBe(300); + + setViewport(768, 2); // 3 up -> 256 CSS px -> 512 + expect(tileThumbnailWidth(BIG)).toBe(600); + + setViewport(1920, 2); // 4 up -> 480 CSS px -> 960, past the top tier + expect(tileThumbnailWidth(BIG)).toBe(900); + }); + + it('stops at the first tier that already covers the source', () => { + // generateThumbnail resizes withoutEnlargement, so once a tier exceeds the + // source every larger one returns the same pixels — for a second Sharp run + // and a second cache entry holding a byte-identical file. + setViewport(1920, 2); // wants 900 + expect(tileThumbnailWidth({ width: 500, height: 400 })).toBe(600); + expect(tileThumbnailWidth({ width: 260, height: 200 })).toBe(300); + }); + + it('does not drop a tier and throw away source pixels', () => { + // A 400px short edge fits inside no tier but 300, so clamping to the + // largest tier it FITS IN would serve 300 and discard 100 real pixels. + // Asking for 600 returns all 400 of them. + setViewport(1920, 2); + expect(tileThumbnailWidth({ width: 500, height: 400 })).not.toBe(300); + + // And a source that comfortably clears 600 is not held back at it. + expect(tileThumbnailWidth({ width: 800, height: 700 })).toBe(900); + }); + + it('measures the SHORT edge, because thumbnails are square', () => { + // A 4000x600 panorama has width to spare and can still only fill a 600 + // square. Measuring the long edge would over-ask for every panorama. + setViewport(1920, 2); + expect(tileThumbnailWidth({ width: 4000, height: 600 })).toBe(600); + }); + + it('prefers the measured tile over the breakpoint fallback', () => { + // Mosaic is 1-up on mobile where Grid is 2-up, and thumbnailScale shifts + // every layout's column count, so the fallback is wrong for most installs + // whenever a real measurement is available. + setViewport(390, 3); + expect(tileThumbnailWidth(BIG)).toBe(600); // fallback: 2 up + expect(tileThumbnailWidth(BIG, 390)).toBe(900); // measured: 1 up + expect(tileThumbnailWidth(BIG, 96)).toBe(300); // measured: a dense grid + + // A zero-width measurement is a not-yet-laid-out tile, not a 0px one. + expect(tileThumbnailWidth(BIG, 0)).toBe(600); + }); + + it('falls back to tile geometry when dimensions are unknown', () => { + // No guard available; the server clamps with withoutEnlargement anyway, so + // the worst case is a tier that returns the source size. + setViewport(390, 3); + expect(tileThumbnailWidth()).toBe(600); + expect(tileThumbnailWidth({ width: null, height: null })).toBe(600); + }); +}); + +describe('thumbnailUrlForTile', () => { + it('appends the tier the tile needs', () => { + setViewport(390, 3); + expect(thumbnailUrlForTile('/api/gallery/x/thumbnail/7', BIG)) + .toBe('/api/gallery/x/thumbnail/7?w=600'); + }); + + it('leaves the canonical URL byte-identical', () => { + // No parameter for the default tier, so existing caches and ETags stay + // valid for every client that is already holding one. + setViewport(390, 1); + expect(thumbnailUrlForTile('/t', BIG)).toBe('/t'); + }); + + it('preserves an existing query string', () => { + setViewport(390, 3); + expect(thumbnailUrlForTile('/t?wm=1', BIG)).toBe('/t?wm=1&w=600'); + }); + + it('downshifts a tier on save-data', () => { + setViewport(390, 3, { saveData: true }); + expect(thumbnailUrlForTile('/t', BIG)).toBe('/t'); + }); + + it('returns null when there is no thumbnail to size', () => { + // The caller is about to fall back to the original; ?w= on that route + // means something else entirely. + expect(thumbnailUrlForTile(null)).toBeNull(); + expect(thumbnailUrlForTile(undefined)).toBeNull(); + }); +}); diff --git a/frontend/src/components/gallery/imageTiers.ts b/frontend/src/components/gallery/imageTiers.ts index 819ba8bb..738fa86d 100644 --- a/frontend/src/components/gallery/imageTiers.ts +++ b/frontend/src/components/gallery/imageTiers.ts @@ -21,12 +21,7 @@ export const FACE_CROP_WIDTH = 640; /** CSS px of the avatar the crop has to fill; used to size the tier. */ const FACE_AVATAR_PX = 64; -// Thumbnail tiers are NOT here yet. Emitting a srcset whose candidates the -// server ignores is worse than emitting none: the browser would pick the -// "600w" candidate, receive the 300px image, and upscale it — the exact -// softness #1095 reports, made slightly worse. generateThumbnail resolves its -// width from admin settings rather than an argument, so tiering it is a -// separate change and lands separately. +export const THUMBNAIL_WIDTHS = [300, 600, 900] as const; /** Smallest tier that still covers `needed`, or the largest if none does. */ function smallestCovering(needed: number, tiers: readonly number[]): number { @@ -195,3 +190,78 @@ export function adminFacePreviewUrl( const width = photo ? faceTierWidth(photo, cover) : FACE_CROP_WIDTH; return `/api/admin/photos/${eventId}/preview/${photoId}?w=${width}`; } + +/** + * Device pixels one grid tile occupies, resolved to a tier (#1095). + * + * `tileCssWidth` is the tile's measured rendered width, which is the only + * honest input: column counts differ per layout (Mosaic is 1-up on mobile + * where Grid is 2-up) and every layout shifts again with the thumbnailScale + * theme setting, so no breakpoint table is right for all of them. When it is + * unavailable the viewport falls back to the default grid's columns — 2 up on + * phones, 3 on tablets, 4 on desktop — which is approximate but never worse + * than the flat 300 it replaces. + * + * At the mobile default a tile is ~195 CSS px, about 585 device px on a DPR-3 + * phone, so the 300px thumbnail is upscaled ~1.9x and faces visibly mush. + * That is the symptom #1095 reports. + * + * DPR is capped at 3 for the same reason as the preview tier: a DPR-10 device + * would otherwise ask for thousands of pixels and land on the top tier for a + * thumbnail nobody can see that much of. + */ +export function tileThumbnailWidth( + photo?: { width?: number | null; height?: number | null }, + tileCssWidth?: number | null, +): number { + if (typeof window === 'undefined') return THUMBNAIL_WIDTHS[0]; + const dpr = Math.min(window.devicePixelRatio || 1, 3); + const vw = window.innerWidth; + const cssWidth = tileCssWidth && tileCssWidth > 0 + ? tileCssWidth + : vw / (vw <= 640 ? 2 : vw <= 1024 ? 3 : 4); + const target = smallestCovering(Math.round(cssWidth * dpr), THUMBNAIL_WIDTHS); + + // Thumbnails are square, so the source's SHORT edge is what bounds them: a + // 4000x600 panorama can still only fill a 600 tile. Clamp to the first tier + // that already covers the whole source — past that, withoutEnlargement means + // every larger tier returns the same pixels, so asking buys a second Sharp + // run and a second cache entry for a byte-identical file. + // + // Clamping to the largest tier the source *fits inside* would be the wrong + // rule: a 400px source would drop to 300 and lose 100 real pixels, when + // asking for 600 returns all 400 of them. + const shortEdge = photo?.width && photo?.height + ? Math.min(photo.width, photo.height) + : null; + if (!shortEdge) return target; + return Math.min(target, smallestCovering(shortEdge, THUMBNAIL_WIDTHS)); +} + +/** + * The grid thumbnail URL sized for this device (#1095). + * + * One URL rather than a srcset, for the same reason the lightbox picks one: + * AuthenticatedImage fetches its `src` with the gallery bearer token and + * renders the resulting blob. An `` carrying a `w`-descriptor srcset + * ignores `src` entirely, so that authenticated fetch would be thrown away and + * the browser would issue its own — unauthenticated, and resolved against the + * page origin rather than the configured API host. + * + * Returns the input untouched when there is nothing to size — a null + * thumbnail_url means the caller is about to fall back to the original, and + * adding ?w= to that URL would mean something else entirely. + */ +export function thumbnailUrlForTile( + thumbnailUrl: string | null | undefined, + photo?: { width?: number | null; height?: number | null }, + tileCssWidth?: number | null, +): string | null { + if (!thumbnailUrl) return null; + const width = applyDataSaver(tileThumbnailWidth(photo, tileCssWidth), THUMBNAIL_WIDTHS); + // The canonical tier is what the server already serves without a parameter; + // leaving it off keeps those URLs byte-identical to today's, so existing + // caches and ETags stay valid. + if (width === THUMBNAIL_WIDTHS[0]) return thumbnailUrl; + return withWidth(thumbnailUrl, width); +} diff --git a/frontend/src/components/gallery/layouts/GalleryPremiumLayout.tsx b/frontend/src/components/gallery/layouts/GalleryPremiumLayout.tsx index b293e07c..3e123759 100644 --- a/frontend/src/components/gallery/layouts/GalleryPremiumLayout.tsx +++ b/frontend/src/components/gallery/layouts/GalleryPremiumLayout.tsx @@ -19,6 +19,7 @@ import { useInView } from 'react-intersection-observer'; import type { BaseGalleryLayoutProps } from './BaseGalleryLayout'; import type { Photo } from '../../../types'; import { AuthenticatedImage, PoweredBy } from '../../common'; +import { thumbnailUrlForTile } from '../imageTiers'; import { feedbackService } from '../../../services/feedback.service'; import { PhotoReactions } from '../PhotoReactions'; import { useGuestIdentityOptional } from '../../../contexts/GuestIdentityContext'; @@ -79,6 +80,15 @@ const PhotoCard: React.FC = ({ threshold: 0.1, }); + // Responsive tier (#1095). This layout has its own card rather than the + // shared PhotoCard, so it needs its own call — but MasonryPhotoAlbum hands + // the laid-out tile width straight to the render prop, so the measurement + // the shared card has to take is simply a parameter here. + const isVideo = photo.media_type === 'video' || photo.type === 'video'; + const tieredSrc = (!isVideo && photo.thumbnail_url + ? thumbnailUrlForTile(photo.thumbnail_url, photo, width) + : null) || photo.thumbnail_url || photo.url; + const likeCount = photo.like_count ?? 0; const averageRating = photo.average_rating ?? 0; const commentCount = photo.comment_count ?? 0; @@ -95,7 +105,7 @@ const PhotoCard: React.FC = ({ data-testid={`photo-card-${photo.id}`} > = ({ className="photo-grid flex gap-4" style={{ gap: `${gutter}px` }} > - {photoColumns.map((column, columnIndex) => ( + {containerWidth === 0 ? ( + // Same gate the rows mode above already applies, for the same reason: + // the column count starts at 3 and the greedy distribution runs with a + // hardcoded 300px estimate until the container has been measured. + // Mounting cards into that guess costs a full remount when it settles + // — photos move to a different parent column, so React tears them down + // — and since #1095 each mount picks a tier from its own width, the two + // mounts request two DIFFERENT urls. On a 1440px desktop that was 45 of + // 62 photos downloading twice, plus 17 left on the larger file. +
+ {photos.slice(0, 8).map((photo) => ( +
+ ))} +
+ ) : photoColumns.map((column, columnIndex) => (
({ + AuthenticatedImage: ({ src, alt }: { src: string; alt?: string }) => { + mounted.push(src); + return {alt}; + }, + PoweredBy: () => null, +})); + +vi.mock('../../../../contexts/ThemeContext', () => ({ + useTheme: () => ({ theme: { gallerySettings: { masonryMode: 'columns' } } }), +})); + +vi.mock('../../../../contexts/GuestIdentityContext', () => ({ + useGuestIdentityOptional: () => null, +})); + +const photos: Photo[] = Array.from({ length: 6 }, (_, i) => ({ + id: i + 1, + filename: `IMG_${i}.jpg`, + url: `/api/gallery/x/photo/${i + 1}`, + thumbnail_url: `/api/gallery/x/thumbnail/${i + 1}`, + type: 'individual', + size: 1, + uploaded_at: '2026-01-01T00:00:00Z', + width: 4000, + height: 3000, +} as Photo)); + +/** + * jsdom reports 0 for every offsetWidth, so both the grid container and the + * individual tiles have to be stood up. They need different values — the + * container is what picks the column count, the tile is what picks the tier — + * so the stub keys off the container's own class. + */ +function stubWidths({ container, tile }: { container: number; tile: number }) { + Object.defineProperty(HTMLElement.prototype, 'offsetWidth', { + configurable: true, + get(this: HTMLElement) { + return String(this.className).includes('photo-grid') ? container : tile; + }, + }); +} + +const props = { + photos, + slug: 'x', + onPhotoClick: () => {}, + onDownload: () => {}, + selectedPhotos: new Set(), + isSelectionMode: false, + allowDownloads: true, +} as never; + +beforeEach(() => { + mounted.length = 0; + Object.defineProperty(window, 'devicePixelRatio', { value: 1, configurable: true }); + Object.defineProperty(window, 'innerWidth', { value: 1440, configurable: true }); + vi.stubGlobal('ResizeObserver', class { + observe() {} unobserve() {} disconnect() {} + }); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +describe('MasonryGalleryLayout — columns mode mounts cards once', () => { + it('shows placeholders instead of cards until the container is measured', () => { + stubWidths({ container: 0, tile: 0 }); // never measured + render(); + expect(screen.queryAllByTestId('tile')).toHaveLength(0); + expect(mounted).toHaveLength(0); + }); + + it('requests exactly one url per photo once measured', () => { + stubWidths({ container: 1440, tile: 275 }); + render(); + + // Six photos, six mounts — not twelve. A card mounted into the unmeasured + // 3-column guess and remounted at the settled width would show up here as + // a second entry for the same photo. + expect(mounted).toHaveLength(photos.length); + expect(new Set(mounted).size).toBe(photos.length); + }); + + it('sizes tiles from the settled column count, not the initial 3', () => { + // 1440 measured -> 5 columns -> ~275 CSS px tiles at DPR 1, which the + // canonical thumbnail covers. The unmeasured 3-column guess would be + // ~470px and would have pulled the 600 tier for every photo. + stubWidths({ container: 1440, tile: 275 }); + render(); + expect(mounted.some((s) => s.includes('?w='))).toBe(false); + }); +});