Skip to content

Commit

Permalink
Use common Space type across components (#1433)
Browse files Browse the repository at this point in the history
## What are you changing?

- Use the common `Space` type in components that accept spacing values
as props.
- Fixes a bug where spacing styles are not applied if `space.0` is used
due to `0` being falsy.
- Update `Stack` stories to better show how spacing is applied.
- Expands `Inline` and `Stack` stories to include examples of _all_
spacing values.

<img width="654" alt="Screenshot 2024-05-08 at 15 00 27"
src="https://github.com/guardian/csnx/assets/1166188/658fe4a8-ed26-4d8b-a037-84927618290e">

## Why?

- It removes a small amount of duplication and avoids having to update
spacing values in multiple places. [New spacing values were added last
year](#897), but some components
still lack support for these as their types were not updated at the same
time.
  • Loading branch information
jamesmockett committed May 31, 2024
2 parents 02d95c3 + ee350ef commit 88a09b3
Show file tree
Hide file tree
Showing 11 changed files with 149 additions and 56 deletions.
5 changes: 5 additions & 0 deletions .changeset/forty-comics-scream.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@guardian/source': patch
---

Fixes bug where `space.0` was not applied to `Inline` layouts and updates `Column` and `Stack` to support all spacing units
2 changes: 1 addition & 1 deletion configs/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ export const config = {
clearMocks: true,
moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx'],
testPathIgnorePatterns: ['/node_modules/', '/.wireit/'],
transformIgnorePatterns: ['node_modules/(?!@guardian)'],
transformIgnorePatterns: ['node_modules/.pnpm/(?!@guardian)'],
transform: {
'^.+\\.[tj]sx?$': [
'ts-jest',
Expand Down
1 change: 1 addition & 0 deletions libs/@guardian/source/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
"@babel/core": "7.24.0",
"@emotion/react": "11.11.1",
"@guardian/design-tokens": "workspace:*",
"@guardian/libs": "16.1.3",
"@storybook/manager-api": "8.0.5",
"@storybook/react": "8.0.5",
"@svgr/babel-preset": "8.1.0",
Expand Down
10 changes: 5 additions & 5 deletions libs/@guardian/source/src/react-components/columns/Columns.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import type { SerializedStyles } from '@emotion/react';
import { isUndefined } from '@guardian/libs';
import type { HTMLAttributes } from 'react';
import type { Breakpoint } from '../../foundations';
import type { Props } from '../@types/Props';
import type { Space } from '../@types/Space';
import {
collapseBelowColumnsCSS,
collapseBelowDesktop,
collapseBelowleftCol,
collapseBelowSpaceY,
collapseBelowSpaceYCSS,
collapseBelowTablet,
collapseBelowWide,
columns,
Expand All @@ -22,8 +24,6 @@ export type CollapseBreakpoint = Extract<
'tablet' | 'desktop' | 'leftCol' | 'wide'
>;

export type ColumnsSpaceY = 1 | 2 | 3 | 4 | 5 | 6 | 9 | 12 | 24;

const collapseBelowMap: { [key in CollapseBreakpoint]: SerializedStyles } = {
tablet: collapseBelowTablet,
desktop: collapseBelowDesktop,
Expand Down Expand Up @@ -60,7 +60,7 @@ export interface ColumnsProps extends HTMLAttributes<HTMLDivElement>, Props {
* space](https://www.theguardian.design/2a1e5182b/p/449bd5-space) between
* between columns vertically when collapsed (one unit is 4px).
*/
spaceY?: ColumnsSpaceY;
spaceY?: Space;
}

/**
Expand Down Expand Up @@ -90,7 +90,7 @@ export const Columns = ({
columns,
collapseUntil ? collapseBelowColumnsMap[collapseUntil] : '',
collapseUntil ? collapseBelowMap[collapseUntil] : '',
spaceY ? collapseBelowSpaceY[spaceY] : '',
isUndefined(spaceY) ? '' : collapseBelowSpaceYCSS(spaceY),
cssOverrides,
]}
{...props}
Expand Down
18 changes: 2 additions & 16 deletions libs/@guardian/source/src/react-components/columns/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { SerializedStyles } from '@emotion/react';
import { css } from '@emotion/react';
import type { Breakpoint } from '../../foundations';
import { between, from, space, until } from '../../foundations';
import type { ColumnsSpaceY } from './Columns';
import type { Space } from '../@types/Space';

type ColumnBreakpoint = {
totalColumns: number;
Expand Down Expand Up @@ -30,27 +30,13 @@ export const collapseBelowColumnsCSS = (
}
`;

const collapseBelowSpaceYCSS = (spaceY: ColumnsSpaceY): SerializedStyles => css`
export const collapseBelowSpaceYCSS = (spaceY: Space): SerializedStyles => css`
margin-bottom: ${-space[spaceY]}px;
& > * {
margin-bottom: ${space[spaceY]}px;
}
`;

export const collapseBelowSpaceY: {
[key in ColumnsSpaceY]: SerializedStyles;
} = {
1: collapseBelowSpaceYCSS(1),
2: collapseBelowSpaceYCSS(2),
3: collapseBelowSpaceYCSS(3),
4: collapseBelowSpaceYCSS(4),
5: collapseBelowSpaceYCSS(5),
6: collapseBelowSpaceYCSS(6),
9: collapseBelowSpaceYCSS(9),
12: collapseBelowSpaceYCSS(12),
24: collapseBelowSpaceYCSS(24),
};

const collapseBelowWidth = css`
width: 100% !important;
`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ export const NoSpace: StoryFn<typeof Inline> = Template.bind({});

// *****************************************************************************

export const Space0: StoryFn<typeof Inline> = Template.bind({});
Space0.args = {
space: 0,
};

// *****************************************************************************

export const Space1: StoryFn<typeof Inline> = Template.bind({});
Space1.args = {
space: 1,
Expand Down Expand Up @@ -83,20 +90,55 @@ Space6.args = {

// *****************************************************************************

export const Space8: StoryFn<typeof Inline> = Template.bind({});
Space8.args = {
space: 8,
};

// *****************************************************************************

export const Space9: StoryFn<typeof Inline> = Template.bind({});
Space9.args = {
space: 9,
};

// *****************************************************************************

export const Space10: StoryFn<typeof Inline> = Template.bind({});
Space10.args = {
space: 10,
};

// *****************************************************************************

export const Space12: StoryFn<typeof Inline> = Template.bind({});
Space12.args = {
space: 12,
};

// *****************************************************************************

export const Space14: StoryFn<typeof Inline> = Template.bind({});
Space14.args = {
space: 14,
};

// *****************************************************************************

export const Space16: StoryFn<typeof Inline> = Template.bind({});
Space16.args = {
space: 16,
};

// *****************************************************************************

export const Space18: StoryFn<typeof Inline> = Template.bind({});
Space18.args = {
space: 18,
};

// *****************************************************************************

export const Space24: StoryFn<typeof Inline> = Template.bind({});
Space24.args = {
space: 24,
Expand Down
7 changes: 6 additions & 1 deletion libs/@guardian/source/src/react-components/inline/Inline.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { isUndefined } from '@guardian/libs';
import type { HTMLAttributes } from 'react';
import type { Props } from '../@types/Props';
import type { Space } from '../@types/Space';
Expand Down Expand Up @@ -28,7 +29,11 @@ export const Inline = ({
return (
<div css={inline}>
<div
css={[inlineWrapper, space && inlineSpace(space), cssOverrides]}
css={[
inlineWrapper,
isUndefined(space) ? '' : inlineSpace(space),
cssOverrides,
]}
{...props}
>
{children}
Expand Down
67 changes: 62 additions & 5 deletions libs/@guardian/source/src/react-components/stack/Stack.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { css } from '@emotion/react';
import type { Meta, StoryFn } from '@storybook/react';
import { palette, space } from '../../foundations';
import type { StackProps } from './Stack';
import { Stack } from './Stack';

Expand All @@ -9,18 +11,38 @@ const meta: Meta<typeof Stack> = {

export default meta;

const wrapper = css`
outline: 1px dashed ${palette.neutral[46]};
`;

const box = css`
display: grid;
place-items: center;
height: ${space[12]}px;
background: ${palette.news[600]};
`;

const Template: StoryFn<typeof Stack> = (args: StackProps) => (
<Stack {...args}>
<div>Item 1</div>
<div>Item 2</div>
<div>Item 3</div>
</Stack>
<div css={wrapper}>
<Stack {...args}>
<div css={box}>1</div>
<div css={box}>2</div>
<div css={box}>3</div>
</Stack>
</div>
);

export const Default: StoryFn<typeof Stack> = Template.bind({});

// *****************************************************************************

export const Space0: StoryFn<typeof Stack> = Template.bind({});
Space0.args = {
space: 0,
};

// *****************************************************************************

export const Space1: StoryFn<typeof Stack> = Template.bind({});
Space1.args = {
space: 1,
Expand Down Expand Up @@ -63,20 +85,55 @@ Space6.args = {

// *****************************************************************************

export const Space8: StoryFn<typeof Stack> = Template.bind({});
Space8.args = {
space: 8,
};

// *****************************************************************************

export const Space9: StoryFn<typeof Stack> = Template.bind({});
Space9.args = {
space: 9,
};

// *****************************************************************************

export const Space10: StoryFn<typeof Stack> = Template.bind({});
Space10.args = {
space: 10,
};

// *****************************************************************************

export const Space12: StoryFn<typeof Stack> = Template.bind({});
Space12.args = {
space: 12,
};

// *****************************************************************************

export const Space14: StoryFn<typeof Stack> = Template.bind({});
Space14.args = {
space: 14,
};

// *****************************************************************************

export const Space16: StoryFn<typeof Stack> = Template.bind({});
Space16.args = {
space: 16,
};

// *****************************************************************************

export const Space18: StoryFn<typeof Stack> = Template.bind({});
Space18.args = {
space: 18,
};

// *****************************************************************************

export const Space24: StoryFn<typeof Stack> = Template.bind({});
Space24.args = {
space: 24,
Expand Down
11 changes: 7 additions & 4 deletions libs/@guardian/source/src/react-components/stack/Stack.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { isUndefined } from '@guardian/libs';
import type { HTMLAttributes } from 'react';
import type { Props } from '../@types/Props';
import type { Space } from '../@types/Space';
import { stack, stackSpace } from './styles';

export type StackSpace = 1 | 2 | 3 | 4 | 5 | 6 | 9 | 12 | 24;

export interface StackProps extends HTMLAttributes<HTMLDivElement>, Props {
/**
* [Units of space](https://www.theguardian.design/2a1e5182b/p/449bd5-space) between inline items (one unit is 4px).
*/
space?: StackSpace;
space?: Space;
}

/**
Expand All @@ -26,7 +26,10 @@ export const Stack = ({
...props
}: StackProps) => {
return (
<div css={[stack, space ? stackSpace[space] : '', cssOverrides]} {...props}>
<div
css={[stack, isUndefined(space) ? '' : stackSpace(space), cssOverrides]}
{...props}
>
{children}
</div>
);
Expand Down
26 changes: 2 additions & 24 deletions libs/@guardian/source/src/react-components/stack/styles.ts
Original file line number Diff line number Diff line change
@@ -1,38 +1,16 @@
import type { SerializedStyles } from '@emotion/react';
import { css } from '@emotion/react';
import { space } from '../../foundations';
import type { StackSpace } from './Stack';
import type { Space } from '../@types/Space';

export const stack = css`
& > * {
width: 100%;
}
`;

const stackSpaceStyle = (number: StackSpace): SerializedStyles => css`
export const stackSpace = (number: Space): SerializedStyles => css`
& > * + * {
margin-top: ${space[number]}px;
}
`;

export const stackSpace: {
1: SerializedStyles;
2: SerializedStyles;
3: SerializedStyles;
4: SerializedStyles;
5: SerializedStyles;
6: SerializedStyles;
9: SerializedStyles;
12: SerializedStyles;
24: SerializedStyles;
} = {
1: stackSpaceStyle(1),
2: stackSpaceStyle(2),
3: stackSpaceStyle(3),
4: stackSpaceStyle(4),
5: stackSpaceStyle(5),
6: stackSpaceStyle(6),
9: stackSpaceStyle(9),
12: stackSpaceStyle(12),
24: stackSpaceStyle(24),
};
16 changes: 16 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 88a09b3

Please sign in to comment.