-
Notifications
You must be signed in to change notification settings - Fork 14
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
Changes from 24 commits
a162a43
1608c53
98504da
53d1307
1478cea
2341ccc
4d78efe
8f1d77e
d16da5a
754c35c
6afb330
c1ede9e
e2f5585
99a49f3
2a3befd
d69a9f1
71aeceb
1507631
3088f7c
44b882f
667e4dd
4a1fcd6
db0ddfe
a3becab
f316a46
80110a6
b2425fb
43900b8
ba253fc
5c0e7b7
04ad044
48cb5c5
49d001b
3f5ca77
faedeb3
8a1aff9
3afbf60
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 = { | ||
|
@@ -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}}', | ||
|
@@ -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', | ||
}} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, |
||
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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,7 +55,7 @@ describe('GridLayout', () => { | |
const props: GridLayoutProps = { | ||
areas: ` | ||
"A A" | ||
"B C" | ||
"B C" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😬 |
||
"D E"`, | ||
children: areasChildren, | ||
}; | ||
|
@@ -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: { | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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