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(806) container queries and storybook #877

Open
wants to merge 37 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
a162a43
feat(806): implement #400 container queries
LukeFinch Apr 24, 2023
1608c53
feat(806): define queries on rules object
LukeFinch Apr 27, 2023
98504da
feat(806): extend MQ to allow CQ rules
LukeFinch Apr 27, 2023
53d1307
feat(806): wip
LukeFinch May 2, 2023
1478cea
feat(806): update tests
LukeFinch May 2, 2023
2341ccc
feat(806): handle responsive props container rules
LukeFinch May 3, 2023
4d78efe
feat(806): typescript fixes
LukeFinch May 4, 2023
8f1d77e
feat(806): fix types update test
LukeFinch May 5, 2023
d16da5a
fix(806): fix emotion package versions
LukeFinch May 5, 2023
754c35c
feat(806): fix type for getAreasList
LukeFinch May 5, 2023
6afb330
feat(806): merge main
LukeFinch May 5, 2023
c1ede9e
feat(806): change types for get typography from theme
LukeFinch May 5, 2023
e2f5585
feat(806): update examples, add block story
LukeFinch May 5, 2023
99a49f3
Merge branch 'main' into feat/806-container-queries-storybook
LukeFinch May 5, 2023
2a3befd
feat(806): update snapshots
LukeFinch May 5, 2023
d69a9f1
feat(806): update snapshots
LukeFinch May 5, 2023
71aeceb
feat(806): address comments
LukeFinch May 5, 2023
1507631
feat(806): test coverage
LukeFinch May 9, 2023
3088f7c
Merge branch 'main' into feat/806-container-queries-storybook
LukeFinch May 9, 2023
44b882f
feat(806): move container props to own function
LukeFinch May 10, 2023
667e4dd
feat(806): move container props to own function
LukeFinch May 10, 2023
4a1fcd6
feat(806): address comments
LukeFinch May 10, 2023
db0ddfe
feat(806): remove a test
LukeFinch May 10, 2023
a3becab
Merge branch 'main' into feat/806-container-queries-storybook
LukeFinch May 11, 2023
f316a46
feat(806): remove unused code
LukeFinch May 12, 2023
80110a6
feat(806): move props out of overrides
LukeFinch May 16, 2023
b2425fb
feat(806): move props out of overrides
LukeFinch May 16, 2023
43900b8
Merge branch 'main' into feat/806-container-queries-storybook
LukeFinch May 17, 2023
ba253fc
feat(806): test coverage, peer deps versions
LukeFinch May 19, 2023
5c0e7b7
feat(806): container props tests
LukeFinch May 19, 2023
04ad044
feat(806): merge main
LukeFinch May 19, 2023
48cb5c5
feat(806): update package json snapshot
LukeFinch May 19, 2023
49d001b
feat(806): fix merge conflicts
LukeFinch May 19, 2023
3f5ca77
Merge branch 'main' into feat/806-container-queries-storybook
mutebg Jul 7, 2023
faedeb3
Merge branch 'main' into feat/806-container-queries-storybook
mutebg Jul 17, 2023
8a1aff9
Merge branch 'main' into feat/806-container-queries-storybook
mutebg Aug 15, 2023
3afbf60
Merge branch 'main' into feat/806-container-queries-storybook
mutebg Oct 2, 2023
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
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@
"@docsearch/css": "^3.0.0",
"@docsearch/react": "^3.0.0",
"@emotion/jest": "^11.2.1",
"@emotion/react": "^11.1.5",
"@emotion/styled": "^11.10.4",
"@emotion/react": "^11.10.5",
"@emotion/styled": "^11.10.5",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will these changes no longer work with lower versions of emotion? Do note that we have emotion versions specified in the peerDependencies for consumers and this will likely need to be upped.

It's a bit grey to me if this would actually make this a breaking change or not. Given it's only a minor version bump on a dependency we could argue it isn't... but I guess there is still some level of risk for consumers who are upgrading.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It didn't work on the version we had, and required the bump. I'm not sure which specific minimum version it's in. I could narrow that down. But ultimately, it does require moving to a higher emotion version to use container queries

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

11.10.4 will be the lowest supported version can we please update the peer dependencies?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also I think it should be sub 12 not the thin range that it's currently specified to

"@hookform/resolvers": "^2.5.0",
"@mapbox/rehype-prism": "^0.3.1",
"@mdx-js/loader": "^1.5.8",
Expand Down
13 changes: 13 additions & 0 deletions src/block/__tests__/__snapshots__/block.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,19 @@ exports[`Block with no props renders an unstyled div 1`] = `
</DocumentFragment>
`;

exports[`Block with props renders GridLayout with container name and type 1`] = `
<DocumentFragment>
.emotion-0 {
container-type: inline-size;
container-name: test-container;
}

<div
class="emotion-0"
/>
</DocumentFragment>
`;

exports[`Block with props renders as span 1`] = `
<DocumentFragment>
<span
Expand Down
73 changes: 72 additions & 1 deletion src/block/__tests__/block.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {ThemeProvider, CreateThemeArgs} from '../../theme';
import {Visible} from '../../grid/visibility';
import {createCustomThemeWithBaseThemeSwitch} from '../../test/theme-select-object';
import {TextBlock} from '../../text-block';
import {MQ} from '../../utils';
import {MQ, styled, getColorCssFromTheme} from '../../utils';

// The style presets are added for easier visualization of the spacings around the Block component
const blockCustomThemeObject: CreateThemeArgs = {
Expand All @@ -29,6 +29,13 @@ const blockCustomThemeObject: CreateThemeArgs = {
borderColor: '{{colors.red060}}',
},
},
blockContainerQueryWrapper: {
base: {
borderStyle: 'dashed',
borderWidth: '{{borders.borderWidth010}}',
borderColor: '{{colors.blue060}}',
},
},
blockTransition: {
base: {
color: '{{colors.inkBase}}',
Expand Down Expand Up @@ -124,6 +131,70 @@ export const StoryBreakpoint = () => (
);
StoryBreakpoint.storyName = 'Breakpoint';

export const StoryContainerQueries = () => {
const QueryBlock = () => (
<Block
stylePreset="blockDefault"
paddingInline={{
rules: [
{
rule: '@container (width <= 300px)',
value: 'space010',
},
{
rule: '@container (width > 300px)',
value: 'space040',
},
],
}}
paddingBlock={{
rules: [
{
rule: '@container (width <= 300px)',
value: 'space010',
},
{
rule: '@container (width > 300px)',
value: 'space040',
},
],
}}
>
<Text>This block changes padding depending on its container size</Text>
</Block>
);

return (
<StorybookPage columns={blockGridCols}>
<StorybookCase title="Container < 300px">
<Block
overrides={{
containerName: 'container-small',
containerType: 'inline-size',
}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Block does not have overrides, these need to be props. I think the same apply to the other componnets

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally I had them as props, @jps suggested moving them into overrides - to be the same level as logical props
Ideally would keep them consistent across all components

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the thing is that Block does not have overrides, so the logical props on the Block are props, <Block marginInline />
GridLayout has overrides (which was wrong) and in that case they can be part of the overrides

stylePreset="blockContainerQueryWrapper"
style={{width: '200px'}}
>
<QueryBlock />
</Block>
</StorybookCase>
<StorybookCase title="Container > 300px">
<Block
overrides={{
containerName: 'container-large',
containerType: 'inline-size',
}}
stylePreset="blockContainerQueryWrapper"
style={{width: '500px'}}
>
<QueryBlock />
</Block>
</StorybookCase>
</StorybookPage>
);
};
StoryContainerQueries.storyName = 'Container Queries';

export const StoryTransitions = () => (
<StorybookPage columns={blockGridCols}>
<Block
Expand Down
12 changes: 12 additions & 0 deletions src/block/__tests__/block.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,5 +99,17 @@ describe('Block', () => {
const fragment = renderToFragmentWithTheme(Block, props);
expect(fragment).toMatchSnapshot();
});

test('renders GridLayout with container name and type', () => {
const props: BlockProps = {
overrides: {
containerName: 'test-container',
containerType: 'inline-size',
},
};

const fragment = renderToFragmentWithTheme(Block, props);
expect(fragment).toMatchSnapshot();
});
});
});
4 changes: 4 additions & 0 deletions src/block/block.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ import {
MQ,
getSpacingFromTheme,
getStylePresetFromTheme,
ContainerQueryProps,
} from '../utils/style';
import {getTransitionPresetFromTheme} from '../utils/style/transition-preset';
import {containerProps} from '../utils/container-properties';

export interface BlockProps
extends React.HTMLAttributes<HTMLDivElement>,
Expand All @@ -23,6 +25,7 @@ export interface BlockProps
* @deprecated This property is deprecated and will be removed in the next major release. Use `marginBlockEnd` instead.
*/
spaceStack?: MQ<string>;
overrides?: ContainerQueryProps;
}

const StyledDiv = styled.div<BlockProps>`
Expand All @@ -32,6 +35,7 @@ const StyledDiv = styled.div<BlockProps>`
${({spaceStack}) =>
spaceStack && getSpacingFromTheme(spaceStack, undefined, 'marginBottom')}
${logicalProps()}
${containerProps()}
${({transitionPreset}) =>
transitionPreset && getTransitionPresetFromTheme(transitionPreset)};
`;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-disable no-script-url */
import React from 'react';
import {Story as StoryType} from '@storybook/react';
import {styled} from '../../../utils';
import {Videocam as FilledVideocam} from '@emotion-icons/material/Videocam';
import {Share as FilledShare} from '@emotion-icons/material/Share';
import {
Expand Down Expand Up @@ -35,6 +36,15 @@ import {Tag} from '../../../tag';
import {VideoPlayer} from '../../../video-player';
import {DEFAULT_VIDEO_PLAYER_CONFIG} from '../../../video-player/__tests__/config';

const QueryContainerSmall = styled.div`
container-type: inline-size;
width: 300px;
`;
const QueryContainerLarge = styled.div`
container-type: inline-size;
width: 400px;
`;
LukeFinch marked this conversation as resolved.
Show resolved Hide resolved

const H = ({overrides, ...props}: Omit<HeadlineProps, 'children'>) => (
<Headline
overrides={{typographyPreset: 'editorialHeadline030', ...overrides}}
Expand Down Expand Up @@ -953,6 +963,91 @@ export const StoryLogicalProps = () => (
);
StoryLogicalProps.storyName = 'Logical props';

export const StoryContainerQueries = () => {
const CardWithQueries = () => (
<CardComposable
overrides={{
stylePreset: 'cardInset',
paddingBlock: {
rules: [
{
rule: '@container (width <= 300px)',
value: 'space020',
},
{
rule: '@container (width > 300px)',
value: 'space060',
},
],
},
paddingInline: {
rules: [
{
rule: '@container (width <= 300px)',
value: 'space020',
},
{
rule: '@container (width > 300px)',
value: 'space060',
},
],
},
}}
rowGap={areasGap}
containerType="inline-size"
>
<CardContent rowGap={contentGap}>
<Flag>Flag</Flag>
<Headline
overrides={{
typographyPreset: {
rules: [
{
rule: '@container (width <= 300px)',
value: 'editorialHeadline020',
},
{
rule: '@container (width > 300px)',
value: 'editorialHeadline040',
},
],
},
}}
>
This headline&apos;s typographyPreset changes depending on its
container size
</Headline>
<P />
</CardContent>
<CardMedia
media={{
src: 'https://storybook.newskit.co.uk/placeholder-3x2.png',
}}
/>
<CardActions>
<Tag href={href} size="medium">
Tag
</Tag>
</CardActions>
</CardComposable>
);
return (
<StorybookPage>
<StorybookCase title="Container Query < 300px">
<QueryContainerSmall>
<CardWithQueries />
</QueryContainerSmall>
</StorybookCase>
<StorybookCase title="Container Query > 300px">
<QueryContainerLarge>
<CardWithQueries />
</QueryContainerLarge>
</StorybookCase>
</StorybookPage>
);
};
StoryContainerQueries.storyName = 'Container Queries';

export const StoryOverrides = () => (
<StorybookPage>
<StorybookCase title="Style preset - card and flag colours">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,22 @@ exports[`GridLayout renders GridLayout with colum/row prop 1`] = `
</DocumentFragment>
`;

exports[`GridLayout renders GridLayout with container name and type 1`] = `
<DocumentFragment>
.emotion-0 {
margin: 0;
padding: 0;
display: grid;
container-type: inline-size;
container-name: test-container;
}

<div
class="emotion-0"
/>
</DocumentFragment>
`;

exports[`GridLayout renders GridLayout with different areas for different breakpoints 1`] = `
<DocumentFragment>
.emotion-0 {
Expand Down
14 changes: 13 additions & 1 deletion src/grid-layout/__tests__/grid-layout.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ describe('GridLayout', () => {
const props: GridLayoutProps = {
areas: `
"A A"
"B C"
"B C"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😬

"D E"`,
children: areasChildren,
};
Expand All @@ -64,6 +64,18 @@ describe('GridLayout', () => {
expect(fragment).toMatchSnapshot();
});

test('renders GridLayout with container name and type', () => {
const props: GridLayoutProps = {
overrides: {
containerName: 'test-container',
containerType: 'inline-size',
},
};

const fragment = renderToFragmentWithTheme(GridLayout, props);
expect(fragment).toMatchSnapshot();
});

test('renders GridLayout with different areas for different breakpoints', () => {
const props: GridLayoutProps = {
areas: {
Expand Down
Loading