Skip to content

Commit e0fbf94

Browse files
brandyscarneybrandyscarney
authored
feat(gallery): support CSS variables being passed to gap (#31186)
## What is the current behavior? The Gallery component currently only supports CSS length strings and numbers being passed to `gap`. ## What is the new behavior? - Adds support for CSS variables being passed to `gap` as a string. - Adds spec & e2e tests verifying the behavior. --------- Co-authored-by: Brandy Smith <6577830+brandyscarney@users.noreply.github.com>
1 parent ece483c commit e0fbf94

6 files changed

Lines changed: 292 additions & 14 deletions

File tree

‎core/src/components.d.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1478,7 +1478,7 @@ export namespace Components {
14781478
*/
14791479
"columns": GalleryColumns;
14801480
/**
1481-
* 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.
1481+
* 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.
14821482
* @default DEFAULT_GAP
14831483
*/
14841484
"gap": GalleryGap;
@@ -7526,7 +7526,7 @@ declare namespace LocalJSX {
75267526
*/
75277527
"columns"?: GalleryColumns;
75287528
/**
7529-
* 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.
7529+
* 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.
75307530
* @default DEFAULT_GAP
75317531
*/
75327532
"gap"?: GalleryGap;

‎core/src/components/gallery/gallery.spec.ts‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,26 @@ describe('gallery', () => {
390390
expect((sharedGallery as any).sanitizeGap('clamp(10px, 20%, 30px)')).toBe('clamp(10px, 20%, 30px)');
391391
});
392392

393+
it('should return undefined for malformed math functions', () => {
394+
const malformedValues = ['calc', 'calc(', 'calc()', 'min(', 'clamp(', 'calc(10px + 20px'];
395+
malformedValues.forEach((value) => {
396+
expect((sharedGallery as any).sanitizeGap(value)).toBeUndefined();
397+
});
398+
});
399+
400+
it('should return the string for CSS variables', () => {
401+
expect((sharedGallery as any).sanitizeGap('var(--app-gap)')).toBe('var(--app-gap)');
402+
expect((sharedGallery as any).sanitizeGap('var(--app-gap, 16px)')).toBe('var(--app-gap, 16px)');
403+
expect((sharedGallery as any).sanitizeGap(' var(--app-gap) ')).toBe('var(--app-gap)');
404+
});
405+
406+
it('should return undefined for malformed CSS variables', () => {
407+
const malformedValues = ['var(--app-gap. 16px)', 'var(--app-gap', 'var()', 'var(16px)'];
408+
malformedValues.forEach((value) => {
409+
expect((sharedGallery as any).sanitizeGap(value)).toBeUndefined();
410+
});
411+
});
412+
393413
it('should return the px value for positive integers', () => {
394414
expect((sharedGallery as any).sanitizeGap(0)).toBe('0px');
395415
expect((sharedGallery as any).sanitizeGap('0')).toBe('0px');
@@ -613,6 +633,93 @@ describe('gallery', () => {
613633
});
614634
});
615635

636+
it('should resolve to the CSS variable for each breakpoint without warning when gap is a CSS variable', () => {
637+
const breakpoints = DEFAULT_BREAKPOINTS;
638+
const warningSpy = jest.spyOn(logging, 'printIonWarning').mockImplementation(() => {});
639+
640+
sharedGallery.gap = 'var(--app-gap)';
641+
642+
breakpoints.forEach(({ width }) => {
643+
expect((sharedGallery as any).getGapForWidth(width)).toBe('var(--app-gap)');
644+
});
645+
646+
expect(warningSpy).not.toHaveBeenCalled();
647+
648+
warningSpy.mockRestore();
649+
});
650+
651+
it('should resolve to the CSS variable for breakpoints that set one when gap is a breakpoint map', () => {
652+
const breakpoints = [
653+
{ width: 0, expectedGap: DEFAULT_GAP },
654+
{ width: 575, expectedGap: DEFAULT_GAP },
655+
{ width: 576, expectedGap: DEFAULT_GAP },
656+
{ width: 767, expectedGap: DEFAULT_GAP },
657+
{ width: 768, expectedGap: 'var(--app-gap)' },
658+
{ width: 991, expectedGap: 'var(--app-gap)' },
659+
{ width: 992, expectedGap: DEFAULT_GAP },
660+
{ width: 1199, expectedGap: DEFAULT_GAP },
661+
{ width: 1200, expectedGap: DEFAULT_GAP },
662+
{ width: 1399, expectedGap: DEFAULT_GAP },
663+
{ width: 1400, expectedGap: DEFAULT_GAP },
664+
];
665+
const warningSpy = jest.spyOn(logging, 'printIonWarning').mockImplementation(() => {});
666+
667+
sharedGallery.gap = { md: 'var(--app-gap)' };
668+
669+
breakpoints.forEach(({ width, expectedGap }) => {
670+
expect((sharedGallery as any).getGapForWidth(width)).toBe(expectedGap);
671+
});
672+
673+
expect(warningSpy).not.toHaveBeenCalled();
674+
675+
warningSpy.mockRestore();
676+
});
677+
678+
it('should resolve a breakpoint map mixing CSS variables, literals, and unset (default) breakpoints', () => {
679+
const breakpoints = [
680+
{ width: 0, expectedGap: '8px' },
681+
{ width: 575, expectedGap: '8px' },
682+
{ width: 576, expectedGap: 'var(--g-sm)' },
683+
{ width: 767, expectedGap: 'var(--g-sm)' },
684+
{ width: 768, expectedGap: 'var(--g-md)' },
685+
{ width: 991, expectedGap: 'var(--g-md)' },
686+
{ width: 992, expectedGap: DEFAULT_GAP },
687+
{ width: 1199, expectedGap: DEFAULT_GAP },
688+
{ width: 1200, expectedGap: '2rem' },
689+
{ width: 1399, expectedGap: '2rem' },
690+
{ width: 1400, expectedGap: DEFAULT_GAP },
691+
];
692+
const warningSpy = jest.spyOn(logging, 'printIonWarning').mockImplementation(() => {});
693+
694+
sharedGallery.gap = { xs: '8px', sm: 'var(--g-sm)', md: 'var(--g-md)', xl: '2rem' };
695+
696+
breakpoints.forEach(({ width, expectedGap }) => {
697+
expect((sharedGallery as any).getGapForWidth(width)).toBe(expectedGap);
698+
});
699+
700+
expect(warningSpy).not.toHaveBeenCalled();
701+
702+
warningSpy.mockRestore();
703+
});
704+
705+
it('should warn and fallback to the default gap when gap is a malformed CSS variable', () => {
706+
const breakpoints = DEFAULT_BREAKPOINTS;
707+
const warningSpy = jest.spyOn(logging, 'printIonWarning').mockImplementation(() => {});
708+
709+
sharedGallery.gap = 'var(--app-gap. 16px)';
710+
711+
breakpoints.forEach(({ width, expectedGap }) => {
712+
expect((sharedGallery as any).getGapForWidth(width)).toBe(expectedGap);
713+
});
714+
715+
expect(warningSpy).toHaveBeenCalledWith(
716+
expect.stringContaining('[ion-gallery] - Invalid "gap" value ("var(--app-gap. 16px)").'),
717+
el
718+
);
719+
720+
warningSpy.mockRestore();
721+
});
722+
616723
it('should resolve to the proper gap when the gap property is set to an out of order object', () => {
617724
const breakpoints = [
618725
{ width: 0, expectedGap: '8px' },

‎core/src/components/gallery/gallery.tsx‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { ComponentInterface } from '@stencil/core';
22
import { Component, Element, Host, Listen, Prop, Watch, h } from '@stencil/core';
3-
import { isValidLengthPercentage } from '@utils/css-value-validation';
3+
import { isCssVariable, isValidLengthPercentage } from '@utils/css-value-validation';
44
import { raf } from '@utils/helpers';
55
import { printIonWarning } from '@utils/logging';
66

@@ -79,7 +79,8 @@ export class Gallery implements ComponentInterface {
7979
/**
8080
* The space between gallery items. Accepts valid CSS [length-percentage](https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/length-percentage)
8181
* values like `16px`, `1rem`, `20%`, math functions like `calc(10px + 20%)`,
82-
* or numbers (treated as pixel values). Can also be set as a breakpoint map
82+
* CSS variables like `var(--app-gallery-gap)`, or numbers (treated as pixel
83+
* values). Can also be set as a breakpoint map
8384
* (e.g. `{ xs: '8px', sm: '1rem', md: '24px' }`). Does not accept
8485
* space-separated values or CSS keyword values like `inherit`, `auto`, etc.
8586
*/
@@ -201,9 +202,10 @@ export class Gallery implements ComponentInterface {
201202
}
202203

203204
/**
204-
* Normalize a single gap value (`gap` as a number, string, or one entry from
205-
* a `gap` breakpoint map) to a CSS length string. Returns `undefined` when
206-
* the input cannot be interpreted as a valid CSS length.
205+
* Normalize a single gap value (`gap` as a number, a string such as a CSS
206+
* length-percentage or `var()` reference, or one entry from a `gap`
207+
* breakpoint map) to a CSS length string. Returns `undefined` when the
208+
* input cannot be interpreted as a valid CSS length or `var()` reference.
207209
*/
208210
private sanitizeGap(gap: number | string | undefined): string | undefined {
209211
if (gap === undefined) {
@@ -224,6 +226,10 @@ export class Gallery implements ComponentInterface {
224226
return undefined;
225227
}
226228

229+
if (isCssVariable(normalizedGap)) {
230+
return normalizedGap;
231+
}
232+
227233
const isValidCssLength = isValidLengthPercentage(normalizedGap);
228234

229235
return isValidCssLength ? normalizedGap : undefined;
@@ -346,7 +352,7 @@ export class Gallery implements ComponentInterface {
346352
printIonWarning(
347353
`[ion-gallery] - Invalid "gap" value (${JSON.stringify(
348354
gap
349-
)}). Expected a non-negative number, CSS length string, or breakpoint map object (e.g. { xs: 8, md: "1rem" }).`,
355+
)}). 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" }).`,
350356
this.el
351357
);
352358
this.hasWarnedInvalidGap = true;

‎core/src/components/gallery/test/basic/gallery.e2e.ts‎

Lines changed: 120 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
import { expect } from '@playwright/test';
22
import { configs, test } from '@utils/test/playwright';
33

4+
import { DEFAULT_COLUMNS, DEFAULT_GAP } from '../../gallery-constants';
5+
46
const DEFAULT_COLUMNS_BREAKPOINTS = [
5-
{ name: 'xs', width: 384, expectedColumns: 2 },
6-
{ name: 'sm', width: 576, expectedColumns: 3 },
7-
{ name: 'md', width: 768, expectedColumns: 4 },
8-
{ name: 'lg', width: 992, expectedColumns: 6 },
9-
{ name: 'xl', width: 1200, expectedColumns: 8 },
10-
{ name: 'xxl', width: 1400, expectedColumns: 10 },
7+
{ name: 'xs', width: 384, expectedColumns: DEFAULT_COLUMNS.xs },
8+
{ name: 'sm', width: 576, expectedColumns: DEFAULT_COLUMNS.sm },
9+
{ name: 'md', width: 768, expectedColumns: DEFAULT_COLUMNS.md },
10+
{ name: 'lg', width: 992, expectedColumns: DEFAULT_COLUMNS.lg },
11+
{ name: 'xl', width: 1200, expectedColumns: DEFAULT_COLUMNS.xl },
12+
{ name: 'xxl', width: 1400, expectedColumns: DEFAULT_COLUMNS.xxl },
1113
];
1214

1315
/**
@@ -121,6 +123,118 @@ configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ config, screenshot, t
121123
)
122124
.toBe(`${breakpoint.expectedColumns}`);
123125
});
126+
127+
test(`should resolve the default gap value on ${breakpoint.name} screens`, async ({ page }) => {
128+
await page.setViewportSize({ width: breakpoint.width, height: 900 });
129+
130+
await page.setContent(
131+
`
132+
<ion-gallery>
133+
<img src="/src/components/gallery/test/assets/01.png" alt="One" />
134+
<img src="/src/components/gallery/test/assets/02.png" alt="Two" />
135+
<img src="/src/components/gallery/test/assets/03.png" alt="Three" />
136+
<img src="/src/components/gallery/test/assets/04.png" alt="Four" />
137+
</ion-gallery>
138+
`,
139+
config
140+
);
141+
142+
const gallery = page.locator('ion-gallery');
143+
144+
await expect.poll(() => gallery.evaluate((el) => getComputedStyle(el).rowGap)).toBe(DEFAULT_GAP);
145+
await expect.poll(() => gallery.evaluate((el) => getComputedStyle(el).columnGap)).toBe(DEFAULT_GAP);
146+
});
147+
148+
test(`should resolve the gap CSS variable on ${breakpoint.name} screens`, async ({ page }) => {
149+
await page.setViewportSize({ width: breakpoint.width, height: 900 });
150+
151+
await page.setContent(
152+
`
153+
<ion-gallery style="--app-gap: 24px" gap="var(--app-gap)">
154+
<img src="/src/components/gallery/test/assets/01.png" alt="One" />
155+
<img src="/src/components/gallery/test/assets/02.png" alt="Two" />
156+
<img src="/src/components/gallery/test/assets/03.png" alt="Three" />
157+
<img src="/src/components/gallery/test/assets/04.png" alt="Four" />
158+
</ion-gallery>
159+
`,
160+
config
161+
);
162+
163+
const gallery = page.locator('ion-gallery');
164+
165+
await expect.poll(() => gallery.evaluate((el) => getComputedStyle(el).rowGap)).toBe('24px');
166+
await expect.poll(() => gallery.evaluate((el) => getComputedStyle(el).columnGap)).toBe('24px');
167+
});
168+
169+
test(`should resolve a gap breakpoint map of CSS variables on ${breakpoint.name} screens`, async ({ page }) => {
170+
await page.setViewportSize({ width: breakpoint.width, height: 900 });
171+
172+
await page.setContent(
173+
`
174+
<ion-gallery style="--g-xs: 2px; --g-sm: 4px; --g-md: 8px; --g-lg: 16px; --g-xl: 24px; --g-xxl: 32px">
175+
<img src="/src/components/gallery/test/assets/01.png" alt="One" />
176+
<img src="/src/components/gallery/test/assets/02.png" alt="Two" />
177+
<img src="/src/components/gallery/test/assets/03.png" alt="Three" />
178+
<img src="/src/components/gallery/test/assets/04.png" alt="Four" />
179+
</ion-gallery>
180+
`,
181+
config
182+
);
183+
184+
const gallery = page.locator('ion-gallery');
185+
186+
// Breakpoint maps are objects, so they are set as a property rather
187+
// than an attribute.
188+
await gallery.evaluate((el) => {
189+
(el as HTMLIonGalleryElement).gap = {
190+
xs: 'var(--g-xs)',
191+
sm: 'var(--g-sm)',
192+
md: 'var(--g-md)',
193+
lg: 'var(--g-lg)',
194+
xl: 'var(--g-xl)',
195+
xxl: 'var(--g-xxl)',
196+
};
197+
});
198+
199+
// The resolved gap for each breakpoint, matching the variables set in
200+
// the style above.
201+
const expectedGap: Record<string, string> = {
202+
xs: '2px',
203+
sm: '4px',
204+
md: '8px',
205+
lg: '16px',
206+
xl: '24px',
207+
xxl: '32px',
208+
};
209+
210+
// Each breakpoint resolves its own gap variable.
211+
await expect
212+
.poll(() => gallery.evaluate((el) => getComputedStyle(el).rowGap))
213+
.toBe(expectedGap[breakpoint.name]);
214+
});
215+
});
216+
217+
test('should resolve the gap CSS variable fallback when the variable is not defined', async ({ page }) => {
218+
await page.setViewportSize({ width: 768, height: 900 });
219+
220+
// The CSS variable `--app-gap` is never declared, so the browser
221+
// resolves the var() fallback (8px).
222+
await page.setContent(
223+
`
224+
<ion-gallery gap="var(--app-gap, 8px)">
225+
<img src="/src/components/gallery/test/assets/01.png" alt="One" />
226+
<img src="/src/components/gallery/test/assets/02.png" alt="Two" />
227+
<img src="/src/components/gallery/test/assets/03.png" alt="Three" />
228+
<img src="/src/components/gallery/test/assets/04.png" alt="Four" />
229+
</ion-gallery>
230+
`,
231+
config
232+
);
233+
234+
const gallery = page.locator('ion-gallery');
235+
236+
await expect.poll(() => gallery.evaluate((el) => getComputedStyle(el).rowGap)).toBe('8px');
237+
await expect.poll(() => gallery.evaluate((el) => getComputedStyle(el).columnGap)).toBe('8px');
124238
});
125239
});
126240
});

‎core/src/components/gallery/test/layout/gallery.e2e.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,4 +366,39 @@ configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ config, screenshot, t
366366
});
367367
});
368368
});
369+
370+
test.describe(title('gallery: masonry gap'), () => {
371+
test('should resolve the gap CSS variable in the masonry layout', async ({ page }) => {
372+
await page.setViewportSize({ width: 768, height: 900 });
373+
374+
// Twelve items so the first item is never the last in its column, whose
375+
// bottom margin masonry zeroes out to remove trailing space.
376+
await page.setContent(
377+
`
378+
<ion-gallery layout="masonry" style="--app-gap: 24px" gap="var(--app-gap)">
379+
<div style="height: 40px">One</div>
380+
<div style="height: 80px">Two</div>
381+
<div style="height: 60px">Three</div>
382+
<div style="height: 100px">Four</div>
383+
<div style="height: 50px">Five</div>
384+
<div style="height: 70px">Six</div>
385+
<div style="height: 90px">Seven</div>
386+
<div style="height: 55px">Eight</div>
387+
<div style="height: 75px">Nine</div>
388+
<div style="height: 65px">Ten</div>
389+
<div style="height: 85px">Eleven</div>
390+
<div style="height: 45px">Twelve</div>
391+
</ion-gallery>
392+
`,
393+
config
394+
);
395+
396+
const gallery = page.locator('ion-gallery');
397+
398+
// In the masonry layout the gap variable drives the column gap
399+
// and the spacing below items (margin bottom).
400+
await expect.poll(() => gallery.evaluate((el) => getComputedStyle(el).columnGap)).toBe('24px');
401+
await expect.poll(() => gallery.evaluate((el) => getComputedStyle(el.children[0]).marginBottom)).toBe('24px');
402+
});
403+
});
369404
});

‎core/src/utils/css-value-validation.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ const LENGTH_PERCENTAGE_PATTERN = /^[-+]?(?:\d+\.?\d*|\.\d+)(?:%|[a-z]+)$/i;
99
// Matches simple `calc` / `min` / `max` / `clamp(...)` functions.
1010
const MATH_FUNCTION_PATTERN = /^(calc|min|max|clamp)\s*\(.+\)$/i;
1111

12+
// Matches a `var(--name)` reference with an optional fallback, e.g.
13+
// `var(--my-gap)` or `var(--my-gap, 16px)`.
14+
const VAR_FUNCTION_PATTERN = /^var\(\s*--[^\s,)]+\s*(?:,[\s\S]*)?\)$/i;
15+
1216
/**
1317
* Returns whether `value` matches the [length-percentage](https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/length-percentage)
1418
* syntax. Accepts `<length>` (`<number>` + unit identifier) or `<percentage>` (`<number>%`).
@@ -24,3 +28,15 @@ export function isValidLengthPercentage(value: string): boolean {
2428

2529
return MATH_FUNCTION_PATTERN.test(v) || LENGTH_PERCENTAGE_PATTERN.test(v);
2630
}
31+
32+
/**
33+
* Returns whether `value` is a single [`var()`](https://developer.mozilla.org/en-US/docs/Web/CSS/var)
34+
* reference, e.g. `var(--my-token)` or `var(--my-token, 16px)`. The referenced
35+
* custom property is resolved by the browser, so the resolved value is not
36+
* validated here.
37+
*
38+
* @param value String value to validate.
39+
*/
40+
export function isCssVariable(value: string): boolean {
41+
return VAR_FUNCTION_PATTERN.test(value.trim());
42+
}

0 commit comments

Comments
 (0)