Skip to content

Commit 8a53a8b

Browse files
astro-factory[bot]factory[bot]matthewp
authored
Filter out invalid same-axis object-position pairs in responsive image styles (#18043)
* Fix `image.responsiveStyles` emitting invalid same-axis `object-position` values * test(assets): strengthen responsiveStyles object-position tests Assert all 16 valid cross-axis two-keyword pairs are emitted (both keyword orders, selector and declaration) and that the 4 invalid same-axis pairs are absent as both selectors and declarations. --------- Co-authored-by: factory[bot] <factory[bot]@users.noreply.github.com> Co-authored-by: Matthew Phillips <matthew@matthewphillips.info>
1 parent c08252d commit 8a53a8b

3 files changed

Lines changed: 62 additions & 9 deletions

File tree

‎.changeset/sweet-paths-open.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'astro': patch
3+
---
4+
5+
Fixes `image.responsiveStyles` emitting invalid `object-position` CSS values for same-axis keyword pairs (`top bottom`, `left right`, etc.)

‎packages/astro/src/assets/utils/generateImageStylesCSS.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,16 @@ import { cssFitValues } from '../internal.js';
55
// data-attribute-driven CSS rules so that no inline styles are needed (CSP-safe).
66
const POSITION_KEYWORDS = ['top', 'bottom', 'left', 'right', 'center'];
77

8+
// CSS <position> two-value syntax requires one keyword per axis.
9+
// Same-axis pairs like `top bottom` or `left right` are invalid.
10+
const VERTICAL_ONLY = new Set(['top', 'bottom']);
11+
const HORIZONTAL_ONLY = new Set(['left', 'right']);
12+
813
/**
9-
* Builds every 1- and 2-keyword combination of object-position values:
14+
* Builds every valid 1- and 2-keyword combination of object-position values:
1015
* center, top, bottom, left, right, top left, top center, top right, etc.
16+
* Same-axis pairs (e.g. `top bottom`, `left right`) are excluded because
17+
* the CSS `<position>` grammar forbids two keywords on the same axis.
1118
* Returns entries as `[dataAttrValue, cssValue]` pairs where the data-attr
1219
* value has spaces replaced with dashes (matching the normalisation in internal.ts).
1320
*/
@@ -19,10 +26,12 @@ function getPositionEntries(): Array<[dataAttr: string, cssValue: string]> {
1926
entries.push([kw, kw]);
2027
}
2128

22-
// Two-keyword combinations
29+
// Two-keyword combinations (cross-axis only)
2330
for (const a of POSITION_KEYWORDS) {
2431
for (const b of POSITION_KEYWORDS) {
2532
if (a === b) continue;
33+
if (VERTICAL_ONLY.has(a) && VERTICAL_ONLY.has(b)) continue;
34+
if (HORIZONTAL_ONLY.has(a) && HORIZONTAL_ONLY.has(b)) continue;
2635
const cssValue = `${a} ${b}`;
2736
const dataAttr = `${a}-${b}`;
2837
entries.push([dataAttr, cssValue]);

‎packages/astro/test/units/assets/utils.test.ts‎

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -326,13 +326,30 @@ describe('generateImageStylesCSS', () => {
326326

327327
it('emits position rules for two-keyword combinations', () => {
328328
const css = generateImageStylesCSS();
329-
// Spot-check several common combinations
330-
assert.ok(css.includes('[data-astro-image-pos="top-left"]'));
331-
assert.ok(css.includes('object-position: top left'));
332-
assert.ok(css.includes('[data-astro-image-pos="bottom-right"]'));
333-
assert.ok(css.includes('object-position: bottom right'));
334-
assert.ok(css.includes('[data-astro-image-pos="center-top"]'));
335-
assert.ok(css.includes('object-position: center top'));
329+
// Every two-keyword <position> must use one keyword per axis. `center` can
330+
// pair with a keyword from either axis, so all 16 ordered pairs below are
331+
// valid, in both keyword orders.
332+
const vertical = ['top', 'bottom'];
333+
const horizontal = ['left', 'right'];
334+
for (const v of [...vertical, 'center']) {
335+
for (const h of [...horizontal, 'center']) {
336+
if (v === 'center' && h === 'center') continue;
337+
for (const [a, b] of [
338+
[v, h],
339+
[h, v],
340+
]) {
341+
const dataAttr = `${a}-${b}`;
342+
assert.ok(
343+
css.includes(`[data-astro-image-pos="${dataAttr}"]`),
344+
`missing rule for pos="${dataAttr}"`,
345+
);
346+
assert.ok(
347+
css.includes(`object-position: ${a} ${b}`),
348+
`missing object-position: ${a} ${b}`,
349+
);
350+
}
351+
}
352+
}
336353
});
337354

338355
it('does not emit duplicate single-keyword combinations (e.g. top-top)', () => {
@@ -341,6 +358,28 @@ describe('generateImageStylesCSS', () => {
341358
assert.ok(!css.includes('[data-astro-image-pos="center-center"]'));
342359
});
343360

361+
it('does not emit invalid same-axis position pairs', () => {
362+
const css = generateImageStylesCSS();
363+
// Two-keyword <position> values must use one keyword per axis, so
364+
// same-axis pairs are invalid and must not be emitted.
365+
for (const [a, b] of [
366+
['top', 'bottom'],
367+
['bottom', 'top'],
368+
['left', 'right'],
369+
['right', 'left'],
370+
]) {
371+
const dataAttr = `${a}-${b}`;
372+
assert.ok(
373+
!css.includes(`[data-astro-image-pos="${dataAttr}"]`),
374+
`${dataAttr} is invalid (same axis)`,
375+
);
376+
assert.ok(
377+
!css.includes(`object-position: ${a} ${b}`),
378+
`object-position: ${a} ${b} is invalid (same axis)`,
379+
);
380+
}
381+
});
382+
344383
it('includes default position fallback when configured', () => {
345384
const css = generateImageStylesCSS(undefined, 'center');
346385
assert.ok(css.includes(':where([data-astro-image]:not([data-astro-image-pos]))'));

0 commit comments

Comments
 (0)