Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: Use spacing tokens in Grid component #1089

Merged
merged 20 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
3d17fd2
Derive grid tokens from common spacing tokens
VincentSmedinga Feb 15, 2024
51a2772
Use ‘gap’ instead of ‘white space’ in grid docs
VincentSmedinga Feb 15, 2024
918fe7f
Use column amounts to refer to grid variants
VincentSmedinga Feb 16, 2024
1b633e6
Generate grid gap and padding classes through configuration
VincentSmedinga Feb 21, 2024
eac0807
Merge branch 'develop' into feature/DES-612-use-gap-in-grid
VincentSmedinga Feb 21, 2024
ec5e7d2
Revert "Use column amounts to refer to grid variants"
VincentSmedinga Feb 21, 2024
fd2729b
Merge branch 'develop' into feature/DES-612-use-gap-in-grid
VincentSmedinga Feb 23, 2024
2abc63f
Merge branch 'develop' into feature/DES-612-use-gap-in-grid
VincentSmedinga Feb 27, 2024
e3dc302
Revert generating gap and margin classes
VincentSmedinga Feb 27, 2024
8f8bdaf
Test all gap and margin classes
VincentSmedinga Feb 27, 2024
ccf3478
Fix interpolation for tokens
VincentSmedinga Feb 27, 2024
973549a
Remove unneeded class
VincentSmedinga Feb 27, 2024
19cb6f2
Reuse properties in padding classes
VincentSmedinga Feb 27, 2024
90c9ba1
Merge branch 'develop' into feature/DES-612-use-gap-in-grid
VincentSmedinga Feb 28, 2024
9ce6e48
Revert "Reuse properties in padding classes"
VincentSmedinga Feb 28, 2024
3ff01f7
Make explicit that column gap doesn’t have multiple options
VincentSmedinga Feb 28, 2024
e90f13a
Remove unused options for row gap and vertical padding
VincentSmedinga Feb 28, 2024
5160335
Merge branch 'develop' into feature/DES-612-use-gap-in-grid
VincentSmedinga Feb 28, 2024
0cdadf2
Remove duplicate tests
VincentSmedinga Feb 28, 2024
b4b68ff
Merge branch 'develop' into feature/DES-612-use-gap-in-grid
VincentSmedinga Feb 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions packages/css/src/components/grid/grid.scss
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@
@import "../../common/breakpoint";

.amsterdam-grid {
column-gap: var(--amsterdam-grid-column-gap);
display: grid;
gap: var(--amsterdam-grid-gap);
grid-template-columns: repeat(var(--amsterdam-grid-column-count), 1fr);
padding-inline: var(--amsterdam-grid-padding-inline);
row-gap: var(--amsterdam-grid-row-gap-md);

@media screen and (min-width: $amsterdam-breakpoint-medium) {
grid-template-columns: repeat(var(--amsterdam-grid-medium-column-count), 1fr);
Expand All @@ -25,47 +26,47 @@
}

.amsterdam-grid--gap-vertical--small {
row-gap: calc(var(--amsterdam-grid-gap) / 2);
row-gap: var(--amsterdam-grid-row-gap-sm);
}

.amsterdam-grid--gap-vertical--large {
row-gap: calc(var(--amsterdam-grid-gap) * 2);
row-gap: var(--amsterdam-grid-row-gap-lg);
}

.amsterdam-grid--padding-bottom--small {
padding-block-end: calc(var(--amsterdam-grid-gap) / 2);
padding-block-end: var(--amsterdam-grid-padding-block-sm);
}

.amsterdam-grid--padding-bottom--medium {
padding-block-end: var(--amsterdam-grid-gap);
padding-block-end: var(--amsterdam-grid-padding-block-md);
}

.amsterdam-grid--padding-bottom--large {
padding-block-end: calc(var(--amsterdam-grid-gap) * 2);
padding-block-end: var(--amsterdam-grid-padding-block-lg);
}

.amsterdam-grid--padding-top--small {
padding-block-start: calc(var(--amsterdam-grid-gap) / 2);
padding-block-start: var(--amsterdam-grid-padding-block-sm);
}

.amsterdam-grid--padding-top--medium {
padding-block-start: var(--amsterdam-grid-gap);
padding-block-start: var(--amsterdam-grid-padding-block-md);
}

.amsterdam-grid--padding-top--large {
padding-block-start: calc(var(--amsterdam-grid-gap) * 2);
padding-block-start: var(--amsterdam-grid-padding-block-lg);
}

.amsterdam-grid--padding-vertical--small {
padding-block: calc(var(--amsterdam-grid-gap) / 2);
padding-block: var(--amsterdam-grid-padding-block-sm);
}

.amsterdam-grid--padding-vertical--medium {
padding-block: var(--amsterdam-grid-gap);
padding-block: var(--amsterdam-grid-padding-block-md);
}

.amsterdam-grid--padding-vertical--large {
padding-block: calc(var(--amsterdam-grid-gap) * 2);
padding-block: var(--amsterdam-grid-padding-block-lg);
}

.amsterdam-grid__cell--span-all {
Expand Down
84 changes: 69 additions & 15 deletions packages/react/src/Grid/Grid.test.tsx
Original file line number Diff line number Diff line change
@@ -1,63 +1,117 @@
import { render } from '@testing-library/react'
import { createRef } from 'react'
import { Grid } from './Grid'
import type { GridPaddingSize } from './Grid'
import '@testing-library/jest-dom'

const paddingSizes = ['small', 'medium', 'large']

describe('Grid', () => {
it('renders', () => {
const { container } = render(<Grid />)

const component = container.querySelector(':only-child')

expect(component).toBeInTheDocument()

expect(component).toBeVisible()
})

it('renders a design system BEM class name', () => {
const { container } = render(<Grid />)

const component = container.querySelector(':only-child')

expect(component).toHaveClass('amsterdam-grid')
})

it('renders an additional class name', () => {
const { container } = render(<Grid className="extra" />)

const component = container.querySelector(':only-child')

expect(component).toHaveClass('extra')

expect(component).toHaveClass('amsterdam-grid')
})

it('renders a medium vertical padding class name', () => {
const { container } = render(<Grid paddingVertical="medium" />)
const component = container.querySelector(':only-child')
expect(component).toHaveClass('amsterdam-grid--padding-vertical--medium')
})
it('renders the correct class name for a zero gap', () => {
const { container } = render(<Grid gapVertical="none" />)

it('renders a small top class name', () => {
const { container } = render(<Grid paddingTop="small" />)
const component = container.querySelector(':only-child')
expect(component).toHaveClass('amsterdam-grid--padding-top--small')

expect(component).toHaveClass('amsterdam-grid--gap-vertical--none')
})

it('renders a large bottom class name', () => {
const { container } = render(<Grid paddingBottom="large" />)
it(`renders the correct class name for a small gap`, () => {
const { container } = render(<Grid gapVertical="small" />)

const component = container.querySelector(':only-child')
expect(component).toHaveClass('amsterdam-grid--padding-bottom--large')

expect(component).toHaveClass('amsterdam-grid--gap-vertical--small')
})

it('renders a class name for a vertical gap', () => {
it(`renders the correct class name for a large gap`, () => {
const { container } = render(<Grid gapVertical="large" />)

const component = container.querySelector(':only-child')

expect(component).toHaveClass('amsterdam-grid--gap-vertical--large')
})

it('renders a class name for a vertical gap and a vertical padding', () => {
const { container } = render(<Grid gapVertical="small" paddingVertical="large" />)
it(`renders the correct class name for a small bottom padding`, () => {
const { container } = render(<Grid paddingBottom="small" />)

const component = container.querySelector(':only-child')

expect(component).toHaveClass('amsterdam-grid--padding-bottom--small')
})

it(`renders the correct class name for a small bottom padding`, () => {
const { container } = render(<Grid paddingBottom="small" />)

const component = container.querySelector(':only-child')
expect(component).toHaveClass('amsterdam-grid--gap-vertical--small amsterdam-grid--padding-vertical--large')

expect(component).toHaveClass('amsterdam-grid--padding-bottom--small')
})
alimpens marked this conversation as resolved.
Show resolved Hide resolved

paddingSizes.forEach((size) => {
it(`renders the correct class name for a ${size} bottom padding`, () => {
const { container } = render(<Grid paddingBottom={size as GridPaddingSize} />)

const component = container.querySelector(':only-child')

expect(component).toHaveClass(`amsterdam-grid--padding-bottom--${size}`)
})
})

paddingSizes.forEach((size) => {
it(`renders the correct class name for a ${size} top padding`, () => {
const { container } = render(<Grid paddingTop={size as GridPaddingSize} />)

const component = container.querySelector(':only-child')

expect(component).toHaveClass(`amsterdam-grid--padding-top--${size}`)
})
})

paddingSizes.forEach((size) => {
it(`renders the correct class name for a ${size} vertical padding`, () => {
const { container } = render(<Grid paddingVertical={size as GridPaddingSize} />)

const component = container.querySelector(':only-child')

expect(component).toHaveClass(`amsterdam-grid--padding-vertical--${size}`)
})
})

it('supports ForwardRef in React', () => {
const ref = createRef<HTMLDivElement>()

const { container } = render(<Grid ref={ref} />)

const component = container.querySelector(':only-child')

expect(ref.current).toBe(component)
})
})
2 changes: 1 addition & 1 deletion packages/react/src/Grid/Grid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export type GridColumnNumbers = {
wide: GridColumnNumber
}

type GridPaddingSize = 'small' | 'medium' | 'large'
export type GridPaddingSize = 'small' | 'medium' | 'large'

type GridPaddingVerticalProp = {
paddingBottom?: never
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
"space": {
"xs": { "value": "clamp(0.25rem, calc(0.390625vw - 0.015625rem), 0.625rem)" },
"sm": { "value": "clamp(0.5rem, calc(0.78125vw - 0.03125rem), 1.25rem)" },
"md": { "value": "clamp(1rem, calc(1.5625vw - 0.0625rem), 2.5rem)" },
"md": {
"value": "clamp(1rem, calc(1.5625vw - 0.0625rem), 2.5rem)",
"comment": "Grows from 16px at 1088px wide to 40px at 2624px wide."
dlnr marked this conversation as resolved.
Show resolved Hide resolved
},
"lg": { "value": "clamp(1.5rem, calc(2.34375vw - 0.09375rem), 3.75rem)" },
"xl": { "value": "clamp(2rem, calc(3.125vw - 0.125rem), 5rem)" }
}
Expand Down
5 changes: 4 additions & 1 deletion proprietary/tokens/src/brand/amsterdam/space.tokens.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
"space": {
"xs": { "value": "clamp(0.25rem, calc(0.78125vw + 0.09375rem), 0.875rem)" },
"sm": { "value": "clamp(0.5rem, calc(1.5625vw + 0.1875rem), 1.75rem)" },
"md": { "value": "clamp(1rem, calc(3.125vw + 0.375rem), 3.5rem)" },
"md": {
"value": "clamp(1rem, calc(3.125vw + 0.375rem), 3.5rem)",
"comment": "Grows from 16px at 320px wide to 56px at 1600px wide."
},
"lg": { "value": "clamp(1.5rem, calc(4.6875vw + 0.5625rem), 5.25rem)" },
"xl": { "value": "clamp(2rem, calc(6.25vw + 0.75rem), 7rem)" }
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,7 @@
{
"amsterdam": {
"grid": {
"gap": {
"value": "clamp(1rem, calc(1.5625vw - 0.0625rem), 2.5rem)",
"comment": "Grows from 16px at 1088px wide to 40px at 2624px wide."
},
"padding-inline": {
"value": "clamp(1rem, calc(1.5625vw - 0.0625rem), 2.5rem)",
"comment": "Equals the gap."
}
"padding-inline": { "value": "{amsterdam.space.md}" }
}
}
}
16 changes: 10 additions & 6 deletions proprietary/tokens/src/components/amsterdam/grid.tokens.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@
"amsterdam": {
"grid": {
"column-count": { "value": "4" },
"gap": {
"value": "clamp(1rem, calc(3.125vw + 0.375rem), 3.5rem)",
"comment": "Grows from 16px at 320px wide to 56px at 1600px wide."
"column-gap": { "value": "{amsterdam.space.md}" },
"padding-block": {
"sm": { "value": "{amsterdam.space.sm}" },
"md": { "value": "{amsterdam.space.md}" },
"lg": { "value": "{amsterdam.space.lg}" }
},
"padding-inline": {
"value": "clamp(1.5rem, calc(4.6875vw + 0.5625rem), 5.25rem)",
"comment": "Equals 1.5 times the gap."
"padding-inline": { "value": "{amsterdam.space.lg}" },
"row-gap": {
"sm": { "value": "{amsterdam.space.sm}" },
"md": { "value": "{amsterdam.space.md}" },
"lg": { "value": "{amsterdam.space.lg}" }
},
"medium": {
"column-count": { "value": "8" }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
{
"amsterdam": {
"header": {
"column-gap": { "value": "{amsterdam.grid.gap}" }
"column-gap": {
"value": "{amsterdam.space.md}",
"comment": "Must have the same value as `amsterdam.grid.column-gap`."
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,16 @@
"amsterdam": {
"mega-menu": {
"list-category": {
"column-gap": { "value": "{amsterdam.grid.gap}" },
"column-gap": {
"value": "{amsterdam.space.md}",
"comment": "Must have the same value as `amsterdam.grid.column-gap`."
},
"column-width": { "value": "20rem" },
"padding-block-start": { "value": "1rem" },
"padding-block-end": { "value": "{amsterdam.grid.gap}" }
"padding-block-end": {
"value": "{amsterdam.space.md}",
"comment": "Must have the same value as `amsterdam.grid.row-gap.md`."
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion storybook/src/components/Grid/Grid.docs.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ On narrow screens, you will see three rows of four columns; on medium-wide scree
### Vertical Margin

Unlike the horizontal margins between columns, the vertical ones above and below are adjustable.
The options `paddingVertical`, `paddingTop`, and `paddingBottom` add white space.
The`paddingVertical`, `paddingTop`, and `paddingBottom` props add white space above and below the grid.
This is useful in a coloured area like [Footer](/docs/components-containers-footer--docs) or [Spotlight](/docs/components-containers-spotlight--docs) or between two consecutive grids.

Specify a value of `medium` for vertical white space as wide as the horizontal.
Expand Down
Loading
Loading