diff --git a/src/__tests__/publisher/classStyleInjector.test.ts b/src/__tests__/publisher/classStyleInjector.test.ts index 4b5f5efc2..f7620ee81 100644 --- a/src/__tests__/publisher/classStyleInjector.test.ts +++ b/src/__tests__/publisher/classStyleInjector.test.ts @@ -662,6 +662,84 @@ describe('generateClassCSS', () => { expect(desktopIdx).toBeGreaterThan(tabletIdx) }) + it('keeps min-width contexts narrowest-first when a max-width context sits between them', () => { + // Regression: width order is only defined between two contexts of the same + // query kind, so ordering used to fall back to registry index for a + // min-vs-max pair. That made the comparison non-transitive — the two + // min-width contexts below are never compared to each other, and the + // 1024px block was emitted before the 768px one. Both match above 1024px, + // specificity is equal, so the 768px block won and mobile-first inverted. + const breakpoints = [ + { id: 'desktop', width: 1024, mediaQuery: '(min-width: 1024px)' }, + { id: 'mobile', width: 900, mediaQuery: '(max-width: 900px)' }, + { id: 'tablet', width: 768, mediaQuery: '(min-width: 768px)' }, + ] + const classes = { + hero: makeClass('hero', { color: 'rgb(1, 2, 3)' }, { + desktop: { color: 'rgb(13, 14, 15)' }, + mobile: { color: 'rgb(7, 8, 9)' }, + tablet: { color: 'rgb(10, 11, 12)' }, + }), + } + const css = generateClassCSS(classes, breakpoints) + const tabletIdx = css.indexOf('@media (min-width: 768px)') + const desktopIdx = css.indexOf('@media (min-width: 1024px)') + expect(tabletIdx).toBeGreaterThanOrEqual(0) + expect(desktopIdx).toBeGreaterThan(tabletIdx) + }) + + it('keeps max-width contexts widest-first when a min-width context sits between them', () => { + const breakpoints = [ + { id: 'narrow', width: 600, mediaQuery: '(max-width: 600px)' }, + { id: 'desktop', width: 1024, mediaQuery: '(min-width: 1024px)' }, + { id: 'wide', width: 900, mediaQuery: '(max-width: 900px)' }, + ] + const classes = { + hero: makeClass('hero', { color: 'rgb(1, 2, 3)' }, { + narrow: { color: 'rgb(4, 5, 6)' }, + desktop: { color: 'rgb(13, 14, 15)' }, + wide: { color: 'rgb(7, 8, 9)' }, + }), + } + const css = generateClassCSS(classes, breakpoints) + const wideIdx = css.indexOf('@media (max-width: 900px)') + const narrowIdx = css.indexOf('@media (max-width: 600px)') + expect(wideIdx).toBeGreaterThanOrEqual(0) + expect(narrowIdx).toBeGreaterThan(wideIdx) + }) + + it('orders viewport contexts by the registry, not by contextStyles key order', () => { + // `contextStyles` is keyed in authoring order, which is not registry order. + // Emission must not depend on it: the same registry and the same overrides + // produce the same CSS whichever order the keys were written in. + const breakpoints = [ + { id: 'desktop', width: 1024, mediaQuery: '(min-width: 1024px)' }, + { id: 'mid', width: 1023, mediaQuery: '(max-width: 1023px)' }, + { id: 'tablet', width: 768, mediaQuery: '(min-width: 768px)' }, + ] + const overrides = { + desktop: { color: 'rgb(13, 14, 15)' }, + mid: { color: 'rgb(7, 8, 9)' }, + tablet: { color: 'rgb(10, 11, 12)' }, + } + const authoredOneWay = generateClassCSS( + { hero: makeClass('hero', { color: 'rgb(1, 2, 3)' }, overrides) }, + breakpoints, + ) + const authoredAnother = generateClassCSS( + { hero: makeClass('hero', { color: 'rgb(1, 2, 3)' }, { + tablet: overrides.tablet, + desktop: overrides.desktop, + mid: overrides.mid, + }) }, + breakpoints, + ) + expect(authoredAnother).toBe(authoredOneWay) + expect(authoredOneWay.indexOf('@media (min-width: 1024px)')).toBeGreaterThan( + authoredOneWay.indexOf('@media (min-width: 768px)'), + ) + }) + it('rewrites class background images with responsive image-set candidates', () => { const classes = { hero: makeClass('hero', { backgroundImage: "url('/uploads/hero.png')" }), diff --git a/src/core/publisher/classCss.ts b/src/core/publisher/classCss.ts index 87321ea25..6c2bbaa7e 100644 --- a/src/core/publisher/classCss.ts +++ b/src/core/publisher/classCss.ts @@ -270,6 +270,11 @@ export function bagToReactStyle( * narrowest matching query wins. Pure min-width contexts emit narrowest first * so the widest matching query wins. Mixed/custom viewport queries keep the * user's registry order. + * + * Those two width orderings are *within* a query kind: a registry holding both + * `min-width` and `max-width` contexts keeps the two groups in registry order + * and sorts each group by width in its own slots. See + * `sortViewportContextCascade`. */ export interface ViewportContext { id: string @@ -296,15 +301,49 @@ function viewportQuerySort(breakpoint: ViewportContext): ViewportQuerySort { return { kind: 'other', width: breakpoint.width } } -export function compareViewportContextCascade( - a: { breakpoint: ViewportContext; index: number }, - b: { breakpoint: ViewportContext; index: number }, -): number { - const aQuery = viewportQuerySort(a.breakpoint) - const bQuery = viewportQuerySort(b.breakpoint) - if (aQuery.kind === 'max' && bQuery.kind === 'max') return bQuery.width - aQuery.width - if (aQuery.kind === 'min' && bQuery.kind === 'min') return aQuery.width - bQuery.width - return a.index - b.index +/** + * Order viewport contexts for emission. + * + * This cannot be a plain `Array.prototype.sort` comparator. Width order is only + * meaningful between two contexts of the same query kind — comparing a + * `min-width` against a `max-width` has no answer, and falling back to registry + * index for those pairs makes the comparator non-transitive: with + * `[min-width: 1024px, max-width: 900px, min-width: 768px]` the two min-width + * contexts never get compared to each other, so the 1024px block is emitted + * before the 768px one and wins the equal-specificity tie at every viewport + * above 1024px. Mobile-first CSS silently inverts. + * + * Instead, partition by kind over registry order and sort each kind group + * within the slots that group already occupies. Cross-kind order stays registry + * order, `min`/`max` groups get their width order regardless of how the kinds + * interleave, and `other` (mixed or non-pixel queries, where width order is not + * defined) is never reordered. + */ +export function sortViewportContextCascade( + entries: readonly T[], +): T[] { + // Registry order first, so the result never depends on the caller's array + // order (`contextStyles` key order is authoring order, not registry order). + const ordered = entries.slice().sort((a, b) => a.index - b.index) + + for (const kind of ['min', 'max'] as const) { + const slots: number[] = [] + for (let i = 0; i < ordered.length; i++) { + if (viewportQuerySort(ordered[i].breakpoint).kind === kind) slots.push(i) + } + if (slots.length < 2) continue + + const group = slots.map((slot) => ordered[slot]) + group.sort((a, b) => { + const aWidth = viewportQuerySort(a.breakpoint).width + const bWidth = viewportQuerySort(b.breakpoint).width + if (aWidth !== bWidth) return kind === 'min' ? aWidth - bWidth : bWidth - aWidth + return a.index - b.index + }) + slots.forEach((slot, i) => { ordered[slot] = group[i] }) + } + + return ordered } /** @@ -318,7 +357,7 @@ export function compareViewportContextCascade( * between what the editor shows and what a publish ships. * * Cascade order (precedence Q-A): base → custom conditions (registry order) → - * viewport @media contexts (see `compareViewportContextCascade`). Context keys + * viewport @media contexts (see `sortViewportContextCascade`). Context keys * matching neither registry are skipped (orphaned overrides). */ export interface StyleRuleDeclarationLayers { @@ -390,8 +429,7 @@ export function createStyleRuleCssEmitter( blocks.push(`${prelude} {\n ${selector} {\n${decls}\n }\n}`) } - bpEntries.sort(compareViewportContextCascade) - for (const { contextId, bag, breakpoint } of bpEntries) { + for (const { contextId, bag, breakpoint } of sortViewportContextCascade(bpEntries)) { const decls = bagToCSS(bag, options, layers.contextStylePriorities?.[contextId]) if (!decls) continue const prelude = conditionPrelude({ kind: 'media', query: breakpointMediaQuery(breakpoint) }) diff --git a/src/core/publisher/sizesResolver.ts b/src/core/publisher/sizesResolver.ts index 5856b2482..5c1cfd412 100644 --- a/src/core/publisher/sizesResolver.ts +++ b/src/core/publisher/sizesResolver.ts @@ -62,7 +62,7 @@ * tier — the caller falls back to `100vw`. */ import { breakpointMediaQuery, type Page, type PageNode, type SiteDocument } from '@core/page-tree' -import { compareViewportContextCascade } from './classCss' +import { sortViewportContextCascade } from './classCss' // --------------------------------------------------------------------------- // Linear width candidates @@ -491,10 +491,9 @@ export function resolveAutoSizes( // Viewport tiers in `sizes` first-match order — the reverse of the CSS // cascade, so the candidate that would win in CSS is hit first. - const tiers = site.breakpoints - .map((breakpoint, index) => ({ breakpoint, index })) - .sort(compareViewportContextCascade) - .reverse() + const tiers = sortViewportContextCascade( + site.breakpoints.map((breakpoint, index) => ({ breakpoint, index })), + ).reverse() const entries: Array<{ query: string | null; value: string }> = [] for (const { breakpoint } of tiers) {