Skip to content

Commit 42529d4

Browse files
committed
Limit Grid gap props to accept only available spacing values (#402)
## Migration Path For `columnGap` and `rowGap`, reduce all values (responsive or not) in the format `var(--rui-spacing-X)` to corresponding numbers (just `X`), e.g.: ```jsx // Before <Grid columnGap="var(--rui-spacing-3)"> // After <Grid columnGap={3}> ``` (Please mind the correct value type.)
1 parent 1b0d005 commit 42529d4

File tree

6 files changed

+88
-48
lines changed

6 files changed

+88
-48
lines changed

src/docs/contribute/api.mdx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,9 @@ instance?
2727
- If not, put it into the theme. Developers can change it
2828
[in their theme](/customize/theming/overview) and it will be the same for all
2929
component instances.
30+
31+
## Measures
32+
33+
Always use [spacing values](/foundation/spacing) for all kinds of measures like
34+
offsets, gaps, or spacings. This helps keep the design consistent across
35+
components.

src/lib/components/Grid/Grid.jsx

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import { transferProps } from '../_helpers/transferProps';
66
import { generateResponsiveCustomProperties } from './_helpers/generateResponsiveCustomProperties';
77
import styles from './Grid.scss';
88

9+
const SPACING_VALUES = [0, 1, 2, 3, 4, 5, 6, 7];
10+
911
export const Grid = ({
1012
alignContent,
1113
alignItems,
@@ -32,9 +34,9 @@ export const Grid = ({
3234
className={styles.root}
3335
style={{
3436
...generateResponsiveCustomProperties(columns, 'columns'),
35-
...generateResponsiveCustomProperties(columnGap, 'column-gap'),
37+
...generateResponsiveCustomProperties(columnGap, 'column-gap', 'spacing'),
3638
...generateResponsiveCustomProperties(rows, 'rows'),
37-
...generateResponsiveCustomProperties(rowGap, 'row-gap'),
39+
...generateResponsiveCustomProperties(rowGap, 'row-gap', 'spacing'),
3840
...generateResponsiveCustomProperties(autoFlow, 'auto-flow'),
3941
...generateResponsiveCustomProperties(alignContent, 'align-content'),
4042
...generateResponsiveCustomProperties(alignItems, 'align-items'),
@@ -55,12 +57,12 @@ Grid.defaultProps = {
5557
alignItems: undefined,
5658
autoFlow: undefined,
5759
children: null,
58-
columnGap: 'var(--rui-spacing-4)',
60+
columnGap: 4,
5961
columns: '1fr',
6062
id: undefined,
6163
justifyContent: undefined,
6264
justifyItems: undefined,
63-
rowGap: 'var(--rui-spacing-4)',
65+
rowGap: 4,
6466
rows: 'auto',
6567
tag: 'div',
6668
};
@@ -119,19 +121,19 @@ Grid.propTypes = {
119121
*/
120122
children: PropTypes.node,
121123
/**
122-
* Gap between columns. Accepts any valid value of `grid-column-gap` CSS property.
123-
* See [MDN](https://developer.mozilla.org/en-US/docs/Web/CSS/column-gap) for more.
124+
* Gap between columns. Accepts any of [spacing values](/foundation/spacing-values) as number.
125+
* See [MDN](https://developer.mozilla.org/en-US/docs/Web/CSS/column-gap) for more about `column-gap`.
124126
*/
125127
columnGap: PropTypes.oneOfType([
126-
PropTypes.string,
128+
PropTypes.oneOf(SPACING_VALUES),
127129
PropTypes.shape({
128-
xs: PropTypes.string,
129-
sm: PropTypes.string,
130-
md: PropTypes.string,
131-
lg: PropTypes.string,
132-
xl: PropTypes.string,
133-
x2l: PropTypes.string,
134-
x3l: PropTypes.string,
130+
xs: PropTypes.oneOf(SPACING_VALUES),
131+
sm: PropTypes.oneOf(SPACING_VALUES),
132+
md: PropTypes.oneOf(SPACING_VALUES),
133+
lg: PropTypes.oneOf(SPACING_VALUES),
134+
xl: PropTypes.oneOf(SPACING_VALUES),
135+
x2l: PropTypes.oneOf(SPACING_VALUES),
136+
x3l: PropTypes.oneOf(SPACING_VALUES),
135137
}),
136138
]),
137139
/**
@@ -187,19 +189,19 @@ Grid.propTypes = {
187189
}),
188190
]),
189191
/**
190-
* Gap between rows. Accepts any valid value of `grid-row-gap` CSS property.
191-
* See [MDN](https://developer.mozilla.org/en-US/docs/Web/CSS/row-gap) for more.
192+
* Gap between rows. Accepts any of [spacing values](/foundation/spacing-values) as number.
193+
* See [MDN](https://developer.mozilla.org/en-US/docs/Web/CSS/row-gap) for more about `row-gap`.
192194
*/
193195
rowGap: PropTypes.oneOfType([
194-
PropTypes.string,
196+
PropTypes.oneOf(SPACING_VALUES),
195197
PropTypes.shape({
196-
xs: PropTypes.string,
197-
sm: PropTypes.string,
198-
md: PropTypes.string,
199-
lg: PropTypes.string,
200-
xl: PropTypes.string,
201-
x2l: PropTypes.string,
202-
x3l: PropTypes.string,
198+
xs: PropTypes.oneOf(SPACING_VALUES),
199+
sm: PropTypes.oneOf(SPACING_VALUES),
200+
md: PropTypes.oneOf(SPACING_VALUES),
201+
lg: PropTypes.oneOf(SPACING_VALUES),
202+
xl: PropTypes.oneOf(SPACING_VALUES),
203+
x2l: PropTypes.oneOf(SPACING_VALUES),
204+
x3l: PropTypes.oneOf(SPACING_VALUES),
203205
}),
204206
]),
205207
/**

src/lib/components/Grid/README.mdx

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,9 @@ See [API](#api) for all available options.
4242

4343
- This component implements the native [CSS grid layout][grid-layout], a
4444
powerful tool for two-dimensional layouts. You may use any value accepted by
45-
[grid-template-columns], [grid-template-rows], [grid-column-gap],
46-
[grid-row-gap], [grid-auto-flow], [align-content], [align-items],
47-
[justify-content], [justify-items], and CSS properties in corresponding API
48-
options of the component.
45+
[grid-template-columns], [grid-template-rows], [grid-auto-flow],
46+
[align-content], [align-items], [justify-content], [justify-items], and CSS
47+
properties in corresponding API options of the component.
4948

5049
- To align your items in the grid, **simply wrap** them with the Grid
5150
component. Unlike other grid frameworks and UI libraries, **no additional
@@ -126,10 +125,11 @@ Use `columns` and `rows` to specify a more complicated grid layout.
126125

127126
## Gaps
128127

129-
Both column and row gaps can be customized.
128+
Both column and row gaps can be customized. The value must correspond to any of
129+
[available spacings](/foundation/spacing).
130130

131131
<Playground>
132-
<Grid columns="repeat(3, 1fr)" columnGap="0.75rem" rowGap="2rem">
132+
<Grid columns="repeat(3, 1fr)" columnGap={2} rowGap={6}>
133133
<Placeholder bordered>Grid item</Placeholder>
134134
<Placeholder bordered>Grid item</Placeholder>
135135
<Placeholder bordered>Grid item</Placeholder>
@@ -197,17 +197,17 @@ breakpoints are used until they're overridden by a bigger breakpoint.
197197
md: '1fr 2fr',
198198
}}
199199
columnGap={{
200-
xs: 'var(--rui-spacing-4)',
201-
md: 'var(--rui-spacing-2)',
202-
lg: 'var(--rui-spacing-4)',
200+
xs: 4,
201+
md: 2,
202+
lg: 4,
203203
}}
204204
rows={{
205205
xs: 'auto auto 200px 200px',
206206
md: 'auto 200px auto',
207207
}}
208208
rowGap={{
209-
xs: 'var(--rui-spacing-3)',
210-
md: 'var(--rui-spacing-4)',
209+
xs: 3,
210+
md: 4,
211211
}}
212212
>
213213
<Placeholder bordered>Grid item</Placeholder>
@@ -276,8 +276,6 @@ Wrapper for content that should span multiple rows or columns.
276276
[grid-layout]: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Grid_Layout
277277
[grid-template-columns]: https://developer.mozilla.org/en-US/docs/Web/CSS/grid-template-columns
278278
[grid-template-rows]: https://developer.mozilla.org/en-US/docs/Web/CSS/grid-template-rows
279-
[grid-column-gap]: https://developer.mozilla.org/en-US/docs/Web/CSS/column-gap
280-
[grid-row-gap]: https://developer.mozilla.org/en-US/docs/Web/CSS/row-gap
281279
[grid-auto-flow]: https://developer.mozilla.org/en-US/docs/Web/CSS/grid-auto-flow
282280
[align-content]: https://developer.mozilla.org/en-US/docs/Web/CSS/align-content
283281
[align-items]: https://developer.mozilla.org/en-US/docs/Web/CSS/align-items

src/lib/components/Grid/__tests__/Grid.test.jsx

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,16 @@ const responsiveBreakpoints = {
1818
x2l: 'placeholder-x2l',
1919
x3l: 'placeholder-x3l',
2020
};
21+
22+
const responsiveSpacingBreakpoints = {
23+
xs: 1,
24+
sm: 2,
25+
md: 3,
26+
lg: 4,
27+
xl: 5,
28+
x2l: 6,
29+
x3l: 7,
30+
};
2131
/* eslint-enable sort-keys */
2232

2333
const responsiveStyles = (infix) => ({
@@ -30,6 +40,16 @@ const responsiveStyles = (infix) => ({
3040
[`--rui-local-${infix}-x3l`]: 'placeholder-x3l',
3141
});
3242

43+
const responsiveSpacingStyles = (infix) => ({
44+
[`--rui-local-${infix}-xs`]: 'var(--rui-spacing-1)',
45+
[`--rui-local-${infix}-sm`]: 'var(--rui-spacing-2)',
46+
[`--rui-local-${infix}-md`]: 'var(--rui-spacing-3)',
47+
[`--rui-local-${infix}-lg`]: 'var(--rui-spacing-4)',
48+
[`--rui-local-${infix}-xl`]: 'var(--rui-spacing-5)',
49+
[`--rui-local-${infix}-x2l`]: 'var(--rui-spacing-6)',
50+
[`--rui-local-${infix}-x3l`]: 'var(--rui-spacing-7)',
51+
});
52+
3353
const defaultProps = {
3454
children: <div>content</div>,
3555
};
@@ -66,12 +86,12 @@ describe('rendering', () => {
6686
(rootElement) => expect(within(rootElement).getByText('content text')),
6787
],
6888
[
69-
{ columnGap: responsiveBreakpoints },
70-
(rootElement) => expect(rootElement).toHaveStyle(responsiveStyles('column-gap')),
89+
{ columnGap: responsiveSpacingBreakpoints },
90+
(rootElement) => expect(rootElement).toHaveStyle(responsiveSpacingStyles('column-gap')),
7191
],
7292
[
73-
{ columnGap: 'placeholder' },
74-
(rootElement) => expect(rootElement).toHaveStyle({ '--rui-local-column-gap-xs': 'placeholder' }),
93+
{ columnGap: 0 },
94+
(rootElement) => expect(rootElement).toHaveStyle({ '--rui-local-column-gap-xs': 'var(--rui-spacing-0)' }),
7595
],
7696
[
7797
{ columns: responsiveBreakpoints },
@@ -99,12 +119,12 @@ describe('rendering', () => {
99119
(rootElement) => expect(rootElement).toHaveStyle({ '--rui-local-justify-items-xs': 'placeholder' }),
100120
],
101121
[
102-
{ rowGap: responsiveBreakpoints },
103-
(rootElement) => expect(rootElement).toHaveStyle(responsiveStyles('row-gap')),
122+
{ rowGap: responsiveSpacingBreakpoints },
123+
(rootElement) => expect(rootElement).toHaveStyle(responsiveSpacingStyles('row-gap')),
104124
],
105125
[
106-
{ rowGap: 'placeholder' },
107-
(rootElement) => expect(rootElement).toHaveStyle({ '--rui-local-row-gap-xs': 'placeholder' }),
126+
{ rowGap: 0 },
127+
(rootElement) => expect(rootElement).toHaveStyle({ '--rui-local-row-gap-xs': 'var(--rui-spacing-0)' }),
108128
],
109129
[
110130
{ rows: responsiveBreakpoints },

src/lib/components/Grid/_helpers/__tests__/generateResponsiveCustomProperties.test.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ describe('generateResponsiveCustomProperties', () => {
77
).toEqual(null);
88
});
99

10+
test('with prop that is a spacing value', () => {
11+
expect(
12+
generateResponsiveCustomProperties(3, 'columns', 'spacing'),
13+
).toEqual({ '--rui-local-columns-xs': 'var(--rui-spacing-3)' });
14+
});
15+
1016
test('with prop that is not an object', () => {
1117
expect(
1218
generateResponsiveCustomProperties('1fr 1fr', 'columns'),
Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,22 @@
1-
export const generateResponsiveCustomProperties = (prop, infix) => {
1+
const prepareValueByType = (value, type) => {
2+
if (type === 'spacing') {
3+
return `var(--rui-spacing-${value})`;
4+
}
5+
6+
return value;
7+
};
8+
9+
export const generateResponsiveCustomProperties = (prop, infix, type = null) => {
210
if (typeof prop === 'undefined') {
311
return null;
412
}
513

614
if (typeof prop !== 'object') {
7-
return { [`--rui-local-${infix}-xs`]: prop };
15+
return { [`--rui-local-${infix}-xs`]: prepareValueByType(prop, type) };
816
}
917

1018
return Object.keys(prop).reduce((acc, breakpoint) => ({
1119
...acc,
12-
[`--rui-local-${infix}-${breakpoint}`]: prop[breakpoint],
20+
[`--rui-local-${infix}-${breakpoint}`]: prepareValueByType(prop[breakpoint], type),
1321
}), {});
1422
};

0 commit comments

Comments
 (0)