Two independent causes of the same symptom — an aspect-ratio layout that does not lay anything out. gallery-premium discarded the tile height MasonryPhotoAlbum computed from photos.width/height and set height:auto on both card and image, so the rendered shape came from the intrinsic ratio of whatever rendition was served. With thumbnail_fit seeded 'cover' by migration 040 every rendition is square, so the layout drew identical squares and was indistinguishable from grid. The card now uses the height it is given and the stylesheet's existing height:100% applies. The bundled CSS templates pinned images to a fixed pixel height, which has specificity (0,1,1) and beats the .h-full utility (0,1,0) six of the seven layouts use. Elegant Dark is seeded is_default, so that was the out-of-the-box result for any layout other than grid/timeline. Migrations 052/053 corrected for fresh installs; 181 repairs the rows already seeded. Whitespace-tolerant because sanitizeCSS strips newlines from any template ever saved through the editor — an exact-text migration would have silently no-opped on most real installs. The height property is matched with a lookbehind so line-height/max-height/min-height are untouched, grouped selectors are handled, and nested rules are skipped rather than mis-rewritten. Both reported, measured in the live DOM, by @BraynArts.
This commit is contained in:
@@ -77,7 +77,12 @@ const DEFAULT_CSS_TEMPLATE = `/*
|
||||
|
||||
.photo-card img {
|
||||
width: 100%;
|
||||
height: 200px;
|
||||
/* 100%, not a fixed pixel height: every aspect-ratio layout (masonry,
|
||||
justified, mosaic, gallery-premium) gives .photo-card a definite height
|
||||
computed from photos.width/height, and this rule's specificity (0,1,1)
|
||||
beats the .h-full utility (0,1,0) the layouts rely on. A fixed height
|
||||
therefore pinned every image inside a correctly-shaped card — #1131. */
|
||||
height: 100%;
|
||||
object-fit: cover;
|
||||
transition: transform 0.3s ease;
|
||||
}
|
||||
|
||||
@@ -503,7 +503,9 @@ const LIQUID_GLASS_DARK = `/*
|
||||
|
||||
.photo-card img {
|
||||
width: 100%;
|
||||
height: 240px;
|
||||
/* See #1131: a fixed height here beats the layouts' .h-full utility and
|
||||
detaches the image from its aspect-ratio-sized card. */
|
||||
height: 100%;
|
||||
object-fit: cover;
|
||||
transition: transform 0.4s ease, filter 0.4s ease;
|
||||
filter: brightness(0.9);
|
||||
@@ -639,7 +641,7 @@ const LIQUID_GLASS_DARK = `/*
|
||||
}
|
||||
|
||||
.photo-card img {
|
||||
height: 180px;
|
||||
height: 100%;
|
||||
}
|
||||
|
||||
/* Reduce animation complexity on mobile */
|
||||
|
||||
@@ -0,0 +1,123 @@
|
||||
/**
|
||||
* The bundled CSS templates pinned every gallery image to a fixed pixel
|
||||
* height, which broke every aspect-ratio layout (#1131).
|
||||
*
|
||||
* Six of the seven layouts size a tile by putting a computed pixel height on
|
||||
* `.photo-card` and letting the image fill it with `h-full`. A template rule
|
||||
* of `.photo-card img { height: 200px }` has specificity (0,1,1) and beats
|
||||
* `.h-full` at (0,1,0), so the image detached from its card: masonry rendered
|
||||
* correctly-shaped cards with a 200px image glued to the top and empty
|
||||
* background below — or, where the computed card was shorter than 200px, an
|
||||
* image taller than its own container.
|
||||
*
|
||||
* "Elegant Dark" is seeded `is_default = true`, so this was the out-of-the-box
|
||||
* result for anyone choosing any layout other than grid/timeline (where a
|
||||
* fixed square happens to look deliberate).
|
||||
*
|
||||
* Migrations 052 and 053 are corrected for fresh installs; this repairs the
|
||||
* rows already seeded. Templates are referenced by `events.css_template_id`
|
||||
* and read at serve time rather than copied onto the event, so fixing the row
|
||||
* fixes every gallery using it.
|
||||
*
|
||||
* SCOPE: every `.photo-card img` rule that carries a fixed PIXEL height, in
|
||||
* every template — not just the two we seeded, and not just their pristine
|
||||
* copies.
|
||||
*
|
||||
* That is broader than it first looks, and deliberately so. It is also not the
|
||||
* scope this started with: matching the exact seeded text missed every install
|
||||
* where the template had ever been saved through the editor, because
|
||||
* `sanitizeCSS` strips newlines. Those are the majority, and a migration that
|
||||
* silently no-ops on them while being recorded as applied is worse than none.
|
||||
*
|
||||
* The cost is that a fixed pixel height a user wrote themselves is rewritten
|
||||
* too. That is judged acceptable because there is no layout it can be right
|
||||
* for: all seven give `.photo-card` a definite height and expect the image to
|
||||
* fill it, so a pixel height on the image can only detach it from its card.
|
||||
* Anything that is not a fixed px height — %, vh, auto — is left alone, as is
|
||||
* every declaration outside a `.photo-card img` body.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Every `.photo-card img { … }` rule body, however it is spaced.
|
||||
*
|
||||
* Matching the exact seeded text does NOT work, and the reason is worth
|
||||
* stating: `sanitizeCSS` strips all control characters (cssSanitizer.js:61),
|
||||
* so the moment an admin saves a template through the editor — even only to
|
||||
* rename it or toggle it — every newline is REMOVED from the stored CSS. The
|
||||
* shipped `.photo-card img {\n height: 200px;` becomes
|
||||
* `.photo-card img { height: 200px;`. An exact-match migration would find
|
||||
* nothing on those installs, be recorded as applied, and leave the galleries
|
||||
* broken with no second chance.
|
||||
*
|
||||
* Scoped to the rule body rather than the whole stylesheet, so the other pixel
|
||||
* heights in these same templates — a 1px gradient divider, an 8px scrollbar —
|
||||
* are untouched.
|
||||
*/
|
||||
/*
|
||||
* Two details in this pattern are deliberate:
|
||||
*
|
||||
* * the selector part is a LIST, so `.photo-card img, .thumbnail img { … }`
|
||||
* is recognised. Requiring `{` straight after `img` skipped grouped
|
||||
* selectors entirely — and the migration would still be recorded as
|
||||
* applied, so the template kept the bug with no second chance.
|
||||
*
|
||||
* * the body excludes braces, so a rule containing a NESTED block is not
|
||||
* matched at all. `.photo-card img { & + .caption { height: 200px } }` is
|
||||
* valid, passes the validator, and a `[^}]*` body would have captured the
|
||||
* nested block and rewritten the caption's height instead. Skipping it
|
||||
* means such a template keeps a fixed image height; corrupting unrelated
|
||||
* declarations in a migration that cannot be undone is the worse of the
|
||||
* two, and nesting does not appear in anything we ship.
|
||||
*/
|
||||
const PHOTO_CARD_IMG_RULE = /([^{}]*\.photo-card\s+img[^{}]*)\{([^{}]*)\}/g;
|
||||
|
||||
/**
|
||||
* Only a fixed PIXEL height is wrong here; %, vh, auto and the rest stay.
|
||||
*
|
||||
* The lookbehind is load-bearing rather than defensive: without it the pattern
|
||||
* matches the TAIL of `line-height`, `max-height`, `min-height` and any custom
|
||||
* property ending in `-height`, and silently rewrites those instead — in a
|
||||
* migration whose down() is deliberately irreversible.
|
||||
*/
|
||||
const FIXED_PX_HEIGHT = /(?<![\w-])height\s*:\s*\d+(?:\.\d+)?px/gi;
|
||||
|
||||
function relaxFixedImageHeights(css) {
|
||||
return css.replace(PHOTO_CARD_IMG_RULE, (whole, selectors, body) => {
|
||||
// .test() on a /g regex advances lastIndex, so it is reset on both sides
|
||||
// of the check — leaving it set makes the NEXT rule start matching from an
|
||||
// arbitrary offset and silently skip declarations.
|
||||
FIXED_PX_HEIGHT.lastIndex = 0;
|
||||
if (!FIXED_PX_HEIGHT.test(body)) return whole;
|
||||
FIXED_PX_HEIGHT.lastIndex = 0;
|
||||
return `${selectors}{${body.replace(FIXED_PX_HEIGHT, 'height: 100%')}}`;
|
||||
});
|
||||
}
|
||||
|
||||
exports.up = async function up(knex) {
|
||||
if (!(await knex.schema.hasTable('css_templates'))) return;
|
||||
|
||||
const rows = await knex('css_templates').select('id', 'css_content');
|
||||
let fixed = 0;
|
||||
|
||||
for (const row of rows) {
|
||||
const original = row.css_content;
|
||||
if (!original || typeof original !== 'string') continue;
|
||||
|
||||
const updated = relaxFixedImageHeights(original);
|
||||
|
||||
if (updated !== original) {
|
||||
await knex('css_templates').where({ id: row.id }).update({ css_content: updated });
|
||||
fixed += 1;
|
||||
}
|
||||
}
|
||||
|
||||
if (fixed > 0) {
|
||||
console.log(` 181: relaxed the fixed image height in ${fixed} CSS template(s)`);
|
||||
}
|
||||
};
|
||||
|
||||
exports.down = async function down() {
|
||||
// Deliberately irreversible. Putting the pixel heights back would re-break
|
||||
// every aspect-ratio layout, and the rows may have been edited since — there
|
||||
// is no version of "restore" here that is safer than doing nothing.
|
||||
};
|
||||
Reference in New Issue
Block a user