fix(import): capture photo dimensions in fileWatcher + s3AutoImporter (#447)
The aspect-aware gallery layouts (masonry / mosaic / justified) read photo.width and photo.height to size each card to the source's real proportions. Two import paths were inserting rows without those fields, which forced MasonryGalleryLayout to fall back to a hard-coded 800×600 default — every card came out the same shape, so users reported masonry as "always cropped to 1:1ish" no matter which thumbnail fit mode they chose. - fileWatcher.js: extract dims with sharp.metadata() before insert. - s3AutoImporter.js: same, materialising a tmp local copy via withLocalCopy so it works in S3 mode. - migration 090: backfill any pre-existing rows with NULL dims (skips videos, skips S3 deployments — those need the writer fix alone since migrations cannot reach the storage backend). - imageProcessor.js: change DEFAULT_THUMBNAIL_FIT from 'cover' to 'inside' (only kicks in when the seed setting is missing — existing installs keep their saved value). Add UI tooltip recommending 'inside' for masonry/mosaic/justified, 'cover' for uniform grids. i18n covers all six locales.
This commit is contained in:
@@ -0,0 +1,102 @@
|
|||||||
|
/**
|
||||||
|
* Migration: Backfill photo dimensions (v2)
|
||||||
|
*
|
||||||
|
* Re-runs the dimension backfill from migration 064 for any rows that are
|
||||||
|
* still NULL. Migration 064 only ran once at upgrade time; new photos
|
||||||
|
* imported via fileWatcher.js or s3AutoImporter.js between then and now
|
||||||
|
* had their width/height columns left NULL because those code paths did
|
||||||
|
* not capture metadata on insert. This PR fixes both writers, but
|
||||||
|
* pre-existing rows still need a backfill — that is what this does.
|
||||||
|
*
|
||||||
|
* Without dimensions, MasonryGalleryLayout falls back to a hard-coded
|
||||||
|
* 800×600 default, which is why every card in masonry mode looks like
|
||||||
|
* the same 4:3 box (#447).
|
||||||
|
*
|
||||||
|
* Local-fs only — S3 deployments cannot read source objects in a
|
||||||
|
* migration without instantiating the storage backend. Those
|
||||||
|
* deployments rely on the writer fix in s3AutoImporter.js for new
|
||||||
|
* photos and can run a one-shot script if a backfill is needed.
|
||||||
|
*/
|
||||||
|
|
||||||
|
const path = require('path');
|
||||||
|
const fs = require('fs');
|
||||||
|
|
||||||
|
exports.up = async function(knex) {
|
||||||
|
const hasWidth = await knex.schema.hasColumn('photos', 'width');
|
||||||
|
const hasHeight = await knex.schema.hasColumn('photos', 'height');
|
||||||
|
if (!hasWidth || !hasHeight) {
|
||||||
|
console.log('[Migration 090] width/height columns not present, skipping');
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
const backend = (process.env.STORAGE_BACKEND || 'local').toLowerCase();
|
||||||
|
if (backend !== 'local') {
|
||||||
|
console.log(`[Migration 090] STORAGE_BACKEND=${backend} — backfill skipped (S3 deployments not supported in-migration)`);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
const storagePath = process.env.STORAGE_PATH || path.join(__dirname, '../../../storage');
|
||||||
|
|
||||||
|
const photos = await knex('photos')
|
||||||
|
.where(function () {
|
||||||
|
this.whereNull('width').orWhereNull('height');
|
||||||
|
})
|
||||||
|
.andWhere(function () {
|
||||||
|
// Skip videos — sharp can't handle them; they need ffprobe.
|
||||||
|
this.where('media_type', '!=', 'video').orWhereNull('media_type');
|
||||||
|
})
|
||||||
|
.select('id', 'path', 'filename');
|
||||||
|
|
||||||
|
if (photos.length === 0) {
|
||||||
|
console.log('[Migration 090] no photos missing dimensions');
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
console.log(`[Migration 090] backfilling ${photos.length} photos`);
|
||||||
|
|
||||||
|
let sharp;
|
||||||
|
try {
|
||||||
|
sharp = require('sharp');
|
||||||
|
} catch (err) {
|
||||||
|
console.error('[Migration 090] sharp unavailable, skipping:', err.message);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
let updated = 0;
|
||||||
|
let failed = 0;
|
||||||
|
for (const photo of photos) {
|
||||||
|
try {
|
||||||
|
if (!photo.path) {
|
||||||
|
failed++;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
const fullPath = path.join(storagePath, 'events/active', photo.path);
|
||||||
|
if (!fs.existsSync(fullPath)) {
|
||||||
|
failed++;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
const metadata = await sharp(fullPath).metadata();
|
||||||
|
if (metadata.width && metadata.height) {
|
||||||
|
await knex('photos').where('id', photo.id).update({
|
||||||
|
width: metadata.width,
|
||||||
|
height: metadata.height,
|
||||||
|
});
|
||||||
|
updated++;
|
||||||
|
if (updated % 100 === 0) {
|
||||||
|
console.log(`[Migration 090] ${updated}/${photos.length}`);
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
failed++;
|
||||||
|
}
|
||||||
|
} catch (err) {
|
||||||
|
console.error(`[Migration 090] photo ${photo.id}: ${err.message}`);
|
||||||
|
failed++;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
console.log(`[Migration 090] done — ${updated} updated, ${failed} skipped`);
|
||||||
|
};
|
||||||
|
|
||||||
|
exports.down = async function() {
|
||||||
|
// Data-only migration; no rollback action.
|
||||||
|
};
|
||||||
@@ -1,6 +1,7 @@
|
|||||||
const chokidar = require('chokidar');
|
const chokidar = require('chokidar');
|
||||||
const path = require('path');
|
const path = require('path');
|
||||||
const fs = require('fs').promises;
|
const fs = require('fs').promises;
|
||||||
|
const sharp = require('sharp');
|
||||||
const { db } = require('../database/db');
|
const { db } = require('../database/db');
|
||||||
const { formatBoolean } = require('../utils/dbCompat');
|
const { formatBoolean } = require('../utils/dbCompat');
|
||||||
const { generateThumbnail, generateVideoPlaceholder } = require('./imageProcessor');
|
const { generateThumbnail, generateVideoPlaceholder } = require('./imageProcessor');
|
||||||
@@ -92,6 +93,22 @@ async function processNewPhoto(filePath) {
|
|||||||
const relativeThumbPath = thumbnailPath; // thumbnailPath is already relative to storage root
|
const relativeThumbPath = thumbnailPath; // thumbnailPath is already relative to storage root
|
||||||
const mimeType = detectedMime || (isVideo ? 'video/mp4' : 'image/jpeg');
|
const mimeType = detectedMime || (isVideo ? 'video/mp4' : 'image/jpeg');
|
||||||
|
|
||||||
|
// Capture image dimensions so aspect-aware layouts (masonry / mosaic /
|
||||||
|
// justified) can size each card to the photo's real proportions
|
||||||
|
// instead of the 800×600 fallback in MasonryGalleryLayout (#447).
|
||||||
|
// Skip videos — those would need ffprobe.
|
||||||
|
let dimensions = null;
|
||||||
|
if (!isVideo) {
|
||||||
|
try {
|
||||||
|
const metadata = await sharp(filePath).metadata();
|
||||||
|
if (metadata.width && metadata.height) {
|
||||||
|
dimensions = { width: metadata.width, height: metadata.height };
|
||||||
|
}
|
||||||
|
} catch (err) {
|
||||||
|
logger.debug(`Could not read image dimensions for ${filename}: ${err.message}`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Check if photo already exists (by filename or path, to handle replacements)
|
// Check if photo already exists (by filename or path, to handle replacements)
|
||||||
const existingPhoto = await db('photos')
|
const existingPhoto = await db('photos')
|
||||||
.where({ event_id: event.id })
|
.where({ event_id: event.id })
|
||||||
@@ -110,7 +127,8 @@ async function processNewPhoto(filePath) {
|
|||||||
thumbnail_path: relativeThumbPath,
|
thumbnail_path: relativeThumbPath,
|
||||||
type: isVideo ? 'video' : photoType,
|
type: isVideo ? 'video' : photoType,
|
||||||
size_bytes: stats.size,
|
size_bytes: stats.size,
|
||||||
mime_type: mimeType
|
mime_type: mimeType,
|
||||||
|
...(dimensions && { width: dimensions.width, height: dimensions.height })
|
||||||
}).returning('id');
|
}).returning('id');
|
||||||
const photoId = insertResult[0]?.id || insertResult[0];
|
const photoId = insertResult[0]?.id || insertResult[0];
|
||||||
|
|
||||||
|
|||||||
@@ -15,7 +15,13 @@ sharp.concurrency(2); // Limit concurrent operations
|
|||||||
// Default thumbnail settings
|
// Default thumbnail settings
|
||||||
const DEFAULT_THUMBNAIL_WIDTH = 300;
|
const DEFAULT_THUMBNAIL_WIDTH = 300;
|
||||||
const DEFAULT_THUMBNAIL_HEIGHT = 300;
|
const DEFAULT_THUMBNAIL_HEIGHT = 300;
|
||||||
const DEFAULT_THUMBNAIL_FIT = 'cover'; // 'cover' for square crops
|
// 'inside' preserves the source aspect ratio (output ≤ width × height).
|
||||||
|
// This is the right default for masonry / mosaic / justified layouts —
|
||||||
|
// the gallery sizes each card from photo.width/height and renders the
|
||||||
|
// thumbnail with object-cover, so a thumb that already matches the
|
||||||
|
// source aspect doesn't get re-cropped (#447). Admins who want
|
||||||
|
// uniform 1:1 grid tiles can switch to 'cover' in the thumbnail settings.
|
||||||
|
const DEFAULT_THUMBNAIL_FIT = 'inside';
|
||||||
const DEFAULT_THUMBNAIL_QUALITY = 85;
|
const DEFAULT_THUMBNAIL_QUALITY = 85;
|
||||||
const DEFAULT_THUMBNAIL_FORMAT = 'jpeg';
|
const DEFAULT_THUMBNAIL_FORMAT = 'jpeg';
|
||||||
|
|
||||||
|
|||||||
@@ -17,9 +17,11 @@
|
|||||||
|
|
||||||
const path = require('path');
|
const path = require('path');
|
||||||
const mime = require('mime-types');
|
const mime = require('mime-types');
|
||||||
|
const sharp = require('sharp');
|
||||||
const { db } = require('../database/db');
|
const { db } = require('../database/db');
|
||||||
const { formatBoolean } = require('../utils/dbCompat');
|
const { formatBoolean } = require('../utils/dbCompat');
|
||||||
const { getStorage } = require('./storage');
|
const { getStorage } = require('./storage');
|
||||||
|
const { withLocalCopy } = require('./imageProcessor');
|
||||||
const logger = require('../utils/logger');
|
const logger = require('../utils/logger');
|
||||||
|
|
||||||
const POLL_INTERVAL_MS = parseInt(process.env.STORAGE_AUTO_IMPORT_INTERVAL_MS || `${5 * 60 * 1000}`, 10);
|
const POLL_INTERVAL_MS = parseInt(process.env.STORAGE_AUTO_IMPORT_INTERVAL_MS || `${5 * 60 * 1000}`, 10);
|
||||||
@@ -98,6 +100,26 @@ async function processEvent(event, storage) {
|
|||||||
const isVideo = mimeType.startsWith('video/');
|
const isVideo = mimeType.startsWith('video/');
|
||||||
if (!isImage && !isVideo) continue;
|
if (!isImage && !isVideo) continue;
|
||||||
|
|
||||||
|
// Capture image dimensions so aspect-aware layouts (masonry /
|
||||||
|
// mosaic / justified) can size each card to the photo's real
|
||||||
|
// proportions instead of the 800×600 fallback (#447). Materialize
|
||||||
|
// a tmp local copy via withLocalCopy — withLocalCopy handles the
|
||||||
|
// S3 download + cleanup. Skip videos (would need ffprobe).
|
||||||
|
let dimensions = null;
|
||||||
|
if (isImage) {
|
||||||
|
try {
|
||||||
|
dimensions = await withLocalCopy(entry.key, async (localPath) => {
|
||||||
|
const metadata = await sharp(localPath).metadata();
|
||||||
|
if (metadata.width && metadata.height) {
|
||||||
|
return { width: metadata.width, height: metadata.height };
|
||||||
|
}
|
||||||
|
return null;
|
||||||
|
});
|
||||||
|
} catch (err) {
|
||||||
|
logger.debug(`[s3AutoImporter] could not read dimensions for ${entry.key}: ${err.message}`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const insertResult = await db('photos').insert({
|
const insertResult = await db('photos').insert({
|
||||||
event_id: event.id,
|
event_id: event.id,
|
||||||
@@ -110,6 +132,7 @@ async function processEvent(event, storage) {
|
|||||||
mime_type: mimeType,
|
mime_type: mimeType,
|
||||||
source_origin: 'managed',
|
source_origin: 'managed',
|
||||||
uploaded_at: new Date().toISOString(),
|
uploaded_at: new Date().toISOString(),
|
||||||
|
...(dimensions && { width: dimensions.width, height: dimensions.height }),
|
||||||
}).returning('id');
|
}).returning('id');
|
||||||
const photoId = insertResult[0]?.id || insertResult[0];
|
const photoId = insertResult[0]?.id || insertResult[0];
|
||||||
|
|
||||||
|
|||||||
@@ -229,6 +229,9 @@ export const ThumbnailsTab: React.FC = () => {
|
|||||||
<p className="text-xs text-neutral-500 dark:text-neutral-400 mt-1">
|
<p className="text-xs text-neutral-500 dark:text-neutral-400 mt-1">
|
||||||
{t('settings.thumbnails.fitHelp', 'How images are resized to fit the thumbnail dimensions. "Cover" crops to fill, "Contain" fits within bounds.')}
|
{t('settings.thumbnails.fitHelp', 'How images are resized to fit the thumbnail dimensions. "Cover" crops to fill, "Contain" fits within bounds.')}
|
||||||
</p>
|
</p>
|
||||||
|
<p className="text-xs text-neutral-500 dark:text-neutral-400 mt-2">
|
||||||
|
{t('settings.thumbnails.fitRecommendation', 'Recommendation: use "Inside" for masonry / mosaic / justified layouts (preserves aspect ratio); "Cover" for uniform 1:1 grid tiles.')}
|
||||||
|
</p>
|
||||||
</div>
|
</div>
|
||||||
</Card>
|
</Card>
|
||||||
|
|
||||||
|
|||||||
@@ -1246,7 +1246,8 @@
|
|||||||
"fit_contain": "Einpassen (innerhalb)",
|
"fit_contain": "Einpassen (innerhalb)",
|
||||||
"fit_fill": "Strecken",
|
"fit_fill": "Strecken",
|
||||||
"fit_inside": "Innen (verkleinern)",
|
"fit_inside": "Innen (verkleinern)",
|
||||||
"fit_outside": "Außen (vergrößern)"
|
"fit_outside": "Außen (vergrößern)",
|
||||||
|
"fitRecommendation": "Empfehlung: „Inside\" für Masonry-/Mosaic-/Justified-Layouts (bewahrt das Seitenverhältnis), „Cover\" für gleichförmige 1:1-Kacheln."
|
||||||
},
|
},
|
||||||
"categories": {
|
"categories": {
|
||||||
"title": "Kategorien",
|
"title": "Kategorien",
|
||||||
|
|||||||
@@ -952,7 +952,8 @@
|
|||||||
"fit_contain": "Contain (fit within)",
|
"fit_contain": "Contain (fit within)",
|
||||||
"fit_fill": "Fill (stretch)",
|
"fit_fill": "Fill (stretch)",
|
||||||
"fit_inside": "Inside (shrink to fit)",
|
"fit_inside": "Inside (shrink to fit)",
|
||||||
"fit_outside": "Outside (expand to cover)"
|
"fit_outside": "Outside (expand to cover)",
|
||||||
|
"fitRecommendation": "Recommendation: use \"Inside\" for masonry / mosaic / justified layouts (preserves aspect ratio); \"Cover\" for uniform 1:1 grid tiles."
|
||||||
},
|
},
|
||||||
"seo": {
|
"seo": {
|
||||||
"title": "SEO & Robots",
|
"title": "SEO & Robots",
|
||||||
|
|||||||
@@ -967,7 +967,8 @@
|
|||||||
"fit_cover": "Recouvrir (recadrer pour remplir)",
|
"fit_cover": "Recouvrir (recadrer pour remplir)",
|
||||||
"fit_contain": "Contenir (ajuster dans les limites)",
|
"fit_contain": "Contenir (ajuster dans les limites)",
|
||||||
"fit_inside": "Intérieur (réduire pour ajuster)",
|
"fit_inside": "Intérieur (réduire pour ajuster)",
|
||||||
"fit_outside": "Extérieur (agrandir pour recouvrir)"
|
"fit_outside": "Extérieur (agrandir pour recouvrir)",
|
||||||
|
"fitRecommendation": "Recommandation : utilisez « Inside » pour les mises en page masonry/mosaic/justified (préserve les proportions) ; « Cover » pour des tuiles 1:1 uniformes."
|
||||||
},
|
},
|
||||||
"seo": {
|
"seo": {
|
||||||
"title": "SEO et Robots",
|
"title": "SEO et Robots",
|
||||||
|
|||||||
@@ -952,7 +952,8 @@
|
|||||||
"fit_contain": "Contain (passend binnen)",
|
"fit_contain": "Contain (passend binnen)",
|
||||||
"fit_fill": "Vullen (uitrekken)",
|
"fit_fill": "Vullen (uitrekken)",
|
||||||
"fit_inside": "Binnenkant (verkleinen om te passen)",
|
"fit_inside": "Binnenkant (verkleinen om te passen)",
|
||||||
"fit_outside": "Buitenkant (vergroten om te bedekken)"
|
"fit_outside": "Buitenkant (vergroten om te bedekken)",
|
||||||
|
"fitRecommendation": "Aanbeveling: gebruik \"Inside\" voor masonry/mosaic/justified-lay-outs (behoudt beeldverhouding); \"Cover\" voor uniforme 1:1-tegels."
|
||||||
},
|
},
|
||||||
"seo": {
|
"seo": {
|
||||||
"title": "SEO & Robots",
|
"title": "SEO & Robots",
|
||||||
|
|||||||
@@ -969,7 +969,8 @@
|
|||||||
"fit_contain": "Contain (ajustar ao espaço)",
|
"fit_contain": "Contain (ajustar ao espaço)",
|
||||||
"fit_fill": "Fill (esticar)",
|
"fit_fill": "Fill (esticar)",
|
||||||
"fit_inside": "Inside (encolher para caber)",
|
"fit_inside": "Inside (encolher para caber)",
|
||||||
"fit_outside": "Outside (expandir para cobrir)"
|
"fit_outside": "Outside (expandir para cobrir)",
|
||||||
|
"fitRecommendation": "Recomendação: use \"Inside\" para layouts masonry/mosaic/justified (preserva proporção); \"Cover\" para tiles 1:1 uniformes."
|
||||||
},
|
},
|
||||||
"seo": {
|
"seo": {
|
||||||
"title": "SEO e Robots",
|
"title": "SEO e Robots",
|
||||||
|
|||||||
@@ -1027,7 +1027,8 @@
|
|||||||
"fit_contain": "Вписывание (внутри)",
|
"fit_contain": "Вписывание (внутри)",
|
||||||
"fit_fill": "Растягивание",
|
"fit_fill": "Растягивание",
|
||||||
"fit_inside": "Внутри (уменьшить)",
|
"fit_inside": "Внутри (уменьшить)",
|
||||||
"fit_outside": "Снаружи (увеличить)"
|
"fit_outside": "Снаружи (увеличить)",
|
||||||
|
"fitRecommendation": "Рекомендация: «Inside» для масонри/мозаики/выровненных раскладок (сохраняет пропорции); «Cover» для одинаковых плиток 1:1."
|
||||||
},
|
},
|
||||||
"photoDimensions": {
|
"photoDimensions": {
|
||||||
"title": "Размеры фотографий",
|
"title": "Размеры фотографий",
|
||||||
|
|||||||
Reference in New Issue
Block a user