diff --git a/core/src/components.d.ts b/core/src/components.d.ts index c47221f63cb..302c22a4b27 100644 --- a/core/src/components.d.ts +++ b/core/src/components.d.ts @@ -1478,7 +1478,7 @@ export namespace Components { */ "columns": GalleryColumns; /** - * The space between gallery items. Accepts valid CSS [length-percentage](https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/length-percentage) values like `16px`, `1rem`, `20%`, math functions like `calc(10px + 20%)`, or numbers (treated as pixel values). Can also be set as a breakpoint map (e.g. `{ xs: '8px', sm: '1rem', md: '24px' }`). Does not accept space-separated values or CSS keyword values like `inherit`, `auto`, etc. + * The space between gallery items. Accepts valid CSS [length-percentage](https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/length-percentage) values like `16px`, `1rem`, `20%`, math functions like `calc(10px + 20%)`, CSS variables like `var(--app-gallery-gap)`, or numbers (treated as pixel values). Can also be set as a breakpoint map (e.g. `{ xs: '8px', sm: '1rem', md: '24px' }`). Does not accept space-separated values or CSS keyword values like `inherit`, `auto`, etc. * @default DEFAULT_GAP */ "gap": GalleryGap; @@ -7526,7 +7526,7 @@ declare namespace LocalJSX { */ "columns"?: GalleryColumns; /** - * The space between gallery items. Accepts valid CSS [length-percentage](https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/length-percentage) values like `16px`, `1rem`, `20%`, math functions like `calc(10px + 20%)`, or numbers (treated as pixel values). Can also be set as a breakpoint map (e.g. `{ xs: '8px', sm: '1rem', md: '24px' }`). Does not accept space-separated values or CSS keyword values like `inherit`, `auto`, etc. + * The space between gallery items. Accepts valid CSS [length-percentage](https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/length-percentage) values like `16px`, `1rem`, `20%`, math functions like `calc(10px + 20%)`, CSS variables like `var(--app-gallery-gap)`, or numbers (treated as pixel values). Can also be set as a breakpoint map (e.g. `{ xs: '8px', sm: '1rem', md: '24px' }`). Does not accept space-separated values or CSS keyword values like `inherit`, `auto`, etc. * @default DEFAULT_GAP */ "gap"?: GalleryGap; diff --git a/core/src/components/gallery/gallery.spec.ts b/core/src/components/gallery/gallery.spec.ts index 321d0024441..609bf22dc5e 100644 --- a/core/src/components/gallery/gallery.spec.ts +++ b/core/src/components/gallery/gallery.spec.ts @@ -390,6 +390,26 @@ describe('gallery', () => { expect((sharedGallery as any).sanitizeGap('clamp(10px, 20%, 30px)')).toBe('clamp(10px, 20%, 30px)'); }); + it('should return undefined for malformed math functions', () => { + const malformedValues = ['calc', 'calc(', 'calc()', 'min(', 'clamp(', 'calc(10px + 20px']; + malformedValues.forEach((value) => { + expect((sharedGallery as any).sanitizeGap(value)).toBeUndefined(); + }); + }); + + it('should return the string for CSS variables', () => { + expect((sharedGallery as any).sanitizeGap('var(--app-gap)')).toBe('var(--app-gap)'); + expect((sharedGallery as any).sanitizeGap('var(--app-gap, 16px)')).toBe('var(--app-gap, 16px)'); + expect((sharedGallery as any).sanitizeGap(' var(--app-gap) ')).toBe('var(--app-gap)'); + }); + + it('should return undefined for malformed CSS variables', () => { + const malformedValues = ['var(--app-gap. 16px)', 'var(--app-gap', 'var()', 'var(16px)']; + malformedValues.forEach((value) => { + expect((sharedGallery as any).sanitizeGap(value)).toBeUndefined(); + }); + }); + it('should return the px value for positive integers', () => { expect((sharedGallery as any).sanitizeGap(0)).toBe('0px'); expect((sharedGallery as any).sanitizeGap('0')).toBe('0px'); @@ -613,6 +633,93 @@ describe('gallery', () => { }); }); + it('should resolve to the CSS variable for each breakpoint without warning when gap is a CSS variable', () => { + const breakpoints = DEFAULT_BREAKPOINTS; + const warningSpy = jest.spyOn(logging, 'printIonWarning').mockImplementation(() => {}); + + sharedGallery.gap = 'var(--app-gap)'; + + breakpoints.forEach(({ width }) => { + expect((sharedGallery as any).getGapForWidth(width)).toBe('var(--app-gap)'); + }); + + expect(warningSpy).not.toHaveBeenCalled(); + + warningSpy.mockRestore(); + }); + + it('should resolve to the CSS variable for breakpoints that set one when gap is a breakpoint map', () => { + const breakpoints = [ + { width: 0, expectedGap: DEFAULT_GAP }, + { width: 575, expectedGap: DEFAULT_GAP }, + { width: 576, expectedGap: DEFAULT_GAP }, + { width: 767, expectedGap: DEFAULT_GAP }, + { width: 768, expectedGap: 'var(--app-gap)' }, + { width: 991, expectedGap: 'var(--app-gap)' }, + { width: 992, expectedGap: DEFAULT_GAP }, + { width: 1199, expectedGap: DEFAULT_GAP }, + { width: 1200, expectedGap: DEFAULT_GAP }, + { width: 1399, expectedGap: DEFAULT_GAP }, + { width: 1400, expectedGap: DEFAULT_GAP }, + ]; + const warningSpy = jest.spyOn(logging, 'printIonWarning').mockImplementation(() => {}); + + sharedGallery.gap = { md: 'var(--app-gap)' }; + + breakpoints.forEach(({ width, expectedGap }) => { + expect((sharedGallery as any).getGapForWidth(width)).toBe(expectedGap); + }); + + expect(warningSpy).not.toHaveBeenCalled(); + + warningSpy.mockRestore(); + }); + + it('should resolve a breakpoint map mixing CSS variables, literals, and unset (default) breakpoints', () => { + const breakpoints = [ + { width: 0, expectedGap: '8px' }, + { width: 575, expectedGap: '8px' }, + { width: 576, expectedGap: 'var(--g-sm)' }, + { width: 767, expectedGap: 'var(--g-sm)' }, + { width: 768, expectedGap: 'var(--g-md)' }, + { width: 991, expectedGap: 'var(--g-md)' }, + { width: 992, expectedGap: DEFAULT_GAP }, + { width: 1199, expectedGap: DEFAULT_GAP }, + { width: 1200, expectedGap: '2rem' }, + { width: 1399, expectedGap: '2rem' }, + { width: 1400, expectedGap: DEFAULT_GAP }, + ]; + const warningSpy = jest.spyOn(logging, 'printIonWarning').mockImplementation(() => {}); + + sharedGallery.gap = { xs: '8px', sm: 'var(--g-sm)', md: 'var(--g-md)', xl: '2rem' }; + + breakpoints.forEach(({ width, expectedGap }) => { + expect((sharedGallery as any).getGapForWidth(width)).toBe(expectedGap); + }); + + expect(warningSpy).not.toHaveBeenCalled(); + + warningSpy.mockRestore(); + }); + + it('should warn and fallback to the default gap when gap is a malformed CSS variable', () => { + const breakpoints = DEFAULT_BREAKPOINTS; + const warningSpy = jest.spyOn(logging, 'printIonWarning').mockImplementation(() => {}); + + sharedGallery.gap = 'var(--app-gap. 16px)'; + + breakpoints.forEach(({ width, expectedGap }) => { + expect((sharedGallery as any).getGapForWidth(width)).toBe(expectedGap); + }); + + expect(warningSpy).toHaveBeenCalledWith( + expect.stringContaining('[ion-gallery] - Invalid "gap" value ("var(--app-gap. 16px)").'), + el + ); + + warningSpy.mockRestore(); + }); + it('should resolve to the proper gap when the gap property is set to an out of order object', () => { const breakpoints = [ { width: 0, expectedGap: '8px' }, diff --git a/core/src/components/gallery/gallery.tsx b/core/src/components/gallery/gallery.tsx index acaba155ac2..61d56ef6d07 100644 --- a/core/src/components/gallery/gallery.tsx +++ b/core/src/components/gallery/gallery.tsx @@ -1,6 +1,6 @@ import type { ComponentInterface } from '@stencil/core'; import { Component, Element, Host, Listen, Prop, Watch, h } from '@stencil/core'; -import { isValidLengthPercentage } from '@utils/css-value-validation'; +import { isCssVariable, isValidLengthPercentage } from '@utils/css-value-validation'; import { raf } from '@utils/helpers'; import { printIonWarning } from '@utils/logging'; @@ -79,7 +79,8 @@ export class Gallery implements ComponentInterface { /** * The space between gallery items. Accepts valid CSS [length-percentage](https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/length-percentage) * values like `16px`, `1rem`, `20%`, math functions like `calc(10px + 20%)`, - * or numbers (treated as pixel values). Can also be set as a breakpoint map + * CSS variables like `var(--app-gallery-gap)`, or numbers (treated as pixel + * values). Can also be set as a breakpoint map * (e.g. `{ xs: '8px', sm: '1rem', md: '24px' }`). Does not accept * space-separated values or CSS keyword values like `inherit`, `auto`, etc. */ @@ -201,9 +202,10 @@ export class Gallery implements ComponentInterface { } /** - * Normalize a single gap value (`gap` as a number, string, or one entry from - * a `gap` breakpoint map) to a CSS length string. Returns `undefined` when - * the input cannot be interpreted as a valid CSS length. + * Normalize a single gap value (`gap` as a number, a string such as a CSS + * length-percentage or `var()` reference, or one entry from a `gap` + * breakpoint map) to a CSS length string. Returns `undefined` when the + * input cannot be interpreted as a valid CSS length or `var()` reference. */ private sanitizeGap(gap: number | string | undefined): string | undefined { if (gap === undefined) { @@ -224,6 +226,10 @@ export class Gallery implements ComponentInterface { return undefined; } + if (isCssVariable(normalizedGap)) { + return normalizedGap; + } + const isValidCssLength = isValidLengthPercentage(normalizedGap); return isValidCssLength ? normalizedGap : undefined; @@ -346,7 +352,7 @@ export class Gallery implements ComponentInterface { printIonWarning( `[ion-gallery] - Invalid "gap" value (${JSON.stringify( gap - )}). Expected a non-negative number, CSS length string, or breakpoint map object (e.g. { xs: 8, md: "1rem" }).`, + )}). Expected a non-negative number, CSS length string, CSS variable (e.g. var(--app-gap)), or breakpoint map object (e.g. { xs: 8, md: "1rem" }).`, this.el ); this.hasWarnedInvalidGap = true; diff --git a/core/src/components/gallery/test/basic/gallery.e2e.ts b/core/src/components/gallery/test/basic/gallery.e2e.ts index 71ea1ac2317..460c7228881 100644 --- a/core/src/components/gallery/test/basic/gallery.e2e.ts +++ b/core/src/components/gallery/test/basic/gallery.e2e.ts @@ -1,13 +1,15 @@ import { expect } from '@playwright/test'; import { configs, test } from '@utils/test/playwright'; +import { DEFAULT_COLUMNS, DEFAULT_GAP } from '../../gallery-constants'; + const DEFAULT_COLUMNS_BREAKPOINTS = [ - { name: 'xs', width: 384, expectedColumns: 2 }, - { name: 'sm', width: 576, expectedColumns: 3 }, - { name: 'md', width: 768, expectedColumns: 4 }, - { name: 'lg', width: 992, expectedColumns: 6 }, - { name: 'xl', width: 1200, expectedColumns: 8 }, - { name: 'xxl', width: 1400, expectedColumns: 10 }, + { name: 'xs', width: 384, expectedColumns: DEFAULT_COLUMNS.xs }, + { name: 'sm', width: 576, expectedColumns: DEFAULT_COLUMNS.sm }, + { name: 'md', width: 768, expectedColumns: DEFAULT_COLUMNS.md }, + { name: 'lg', width: 992, expectedColumns: DEFAULT_COLUMNS.lg }, + { name: 'xl', width: 1200, expectedColumns: DEFAULT_COLUMNS.xl }, + { name: 'xxl', width: 1400, expectedColumns: DEFAULT_COLUMNS.xxl }, ]; /** @@ -121,6 +123,118 @@ configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ config, screenshot, t ) .toBe(`${breakpoint.expectedColumns}`); }); + + test(`should resolve the default gap value on ${breakpoint.name} screens`, async ({ page }) => { + await page.setViewportSize({ width: breakpoint.width, height: 900 }); + + await page.setContent( + ` + + One + Two + Three + Four + + `, + config + ); + + const gallery = page.locator('ion-gallery'); + + await expect.poll(() => gallery.evaluate((el) => getComputedStyle(el).rowGap)).toBe(DEFAULT_GAP); + await expect.poll(() => gallery.evaluate((el) => getComputedStyle(el).columnGap)).toBe(DEFAULT_GAP); + }); + + test(`should resolve the gap CSS variable on ${breakpoint.name} screens`, async ({ page }) => { + await page.setViewportSize({ width: breakpoint.width, height: 900 }); + + await page.setContent( + ` + + One + Two + Three + Four + + `, + config + ); + + const gallery = page.locator('ion-gallery'); + + await expect.poll(() => gallery.evaluate((el) => getComputedStyle(el).rowGap)).toBe('24px'); + await expect.poll(() => gallery.evaluate((el) => getComputedStyle(el).columnGap)).toBe('24px'); + }); + + test(`should resolve a gap breakpoint map of CSS variables on ${breakpoint.name} screens`, async ({ page }) => { + await page.setViewportSize({ width: breakpoint.width, height: 900 }); + + await page.setContent( + ` + + One + Two + Three + Four + + `, + config + ); + + const gallery = page.locator('ion-gallery'); + + // Breakpoint maps are objects, so they are set as a property rather + // than an attribute. + await gallery.evaluate((el) => { + (el as HTMLIonGalleryElement).gap = { + xs: 'var(--g-xs)', + sm: 'var(--g-sm)', + md: 'var(--g-md)', + lg: 'var(--g-lg)', + xl: 'var(--g-xl)', + xxl: 'var(--g-xxl)', + }; + }); + + // The resolved gap for each breakpoint, matching the variables set in + // the style above. + const expectedGap: Record = { + xs: '2px', + sm: '4px', + md: '8px', + lg: '16px', + xl: '24px', + xxl: '32px', + }; + + // Each breakpoint resolves its own gap variable. + await expect + .poll(() => gallery.evaluate((el) => getComputedStyle(el).rowGap)) + .toBe(expectedGap[breakpoint.name]); + }); + }); + + test('should resolve the gap CSS variable fallback when the variable is not defined', async ({ page }) => { + await page.setViewportSize({ width: 768, height: 900 }); + + // The CSS variable `--app-gap` is never declared, so the browser + // resolves the var() fallback (8px). + await page.setContent( + ` + + One + Two + Three + Four + + `, + config + ); + + const gallery = page.locator('ion-gallery'); + + await expect.poll(() => gallery.evaluate((el) => getComputedStyle(el).rowGap)).toBe('8px'); + await expect.poll(() => gallery.evaluate((el) => getComputedStyle(el).columnGap)).toBe('8px'); }); }); }); diff --git a/core/src/components/gallery/test/layout/gallery.e2e.ts b/core/src/components/gallery/test/layout/gallery.e2e.ts index 2ed856602ed..407648cebfd 100644 --- a/core/src/components/gallery/test/layout/gallery.e2e.ts +++ b/core/src/components/gallery/test/layout/gallery.e2e.ts @@ -366,4 +366,39 @@ configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ config, screenshot, t }); }); }); + + test.describe(title('gallery: masonry gap'), () => { + test('should resolve the gap CSS variable in the masonry layout', async ({ page }) => { + await page.setViewportSize({ width: 768, height: 900 }); + + // Twelve items so the first item is never the last in its column, whose + // bottom margin masonry zeroes out to remove trailing space. + await page.setContent( + ` + +
One
+
Two
+
Three
+
Four
+
Five
+
Six
+
Seven
+
Eight
+
Nine
+
Ten
+
Eleven
+
Twelve
+ + `, + config + ); + + const gallery = page.locator('ion-gallery'); + + // In the masonry layout the gap variable drives the column gap + // and the spacing below items (margin bottom). + await expect.poll(() => gallery.evaluate((el) => getComputedStyle(el).columnGap)).toBe('24px'); + await expect.poll(() => gallery.evaluate((el) => getComputedStyle(el.children[0]).marginBottom)).toBe('24px'); + }); + }); }); diff --git a/core/src/utils/css-value-validation.ts b/core/src/utils/css-value-validation.ts index b53c933ae85..e82e9c6a7a1 100644 --- a/core/src/utils/css-value-validation.ts +++ b/core/src/utils/css-value-validation.ts @@ -9,6 +9,10 @@ const LENGTH_PERCENTAGE_PATTERN = /^[-+]?(?:\d+\.?\d*|\.\d+)(?:%|[a-z]+)$/i; // Matches simple `calc` / `min` / `max` / `clamp(...)` functions. const MATH_FUNCTION_PATTERN = /^(calc|min|max|clamp)\s*\(.+\)$/i; +// Matches a `var(--name)` reference with an optional fallback, e.g. +// `var(--my-gap)` or `var(--my-gap, 16px)`. +const VAR_FUNCTION_PATTERN = /^var\(\s*--[^\s,)]+\s*(?:,[\s\S]*)?\)$/i; + /** * Returns whether `value` matches the [length-percentage](https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/length-percentage) * syntax. Accepts `` (`` + unit identifier) or `` (`%`). @@ -24,3 +28,15 @@ export function isValidLengthPercentage(value: string): boolean { return MATH_FUNCTION_PATTERN.test(v) || LENGTH_PERCENTAGE_PATTERN.test(v); } + +/** + * Returns whether `value` is a single [`var()`](https://developer.mozilla.org/en-US/docs/Web/CSS/var) + * reference, e.g. `var(--my-token)` or `var(--my-token, 16px)`. The referenced + * custom property is resolved by the browser, so the resolved value is not + * validated here. + * + * @param value String value to validate. + */ +export function isCssVariable(value: string): boolean { + return VAR_FUNCTION_PATTERN.test(value.trim()); +}