-
Notifications
You must be signed in to change notification settings - Fork 45
Add @container query support to the createTextStyle function
#147
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
base: master
Are you sure you want to change the base?
Conversation
michaeltaranto
left a comment
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.
Thanks for making this PR. Added a few comments below
| [`${containerName} (min-width: 600px)`]: textValues.desktop, | ||
| }, | ||
| }, | ||
| 'responsiveText', |
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.
Let's align the debugId here:
| 'responsiveText', | |
| 'containerText', |
| queryType, | ||
| queries, | ||
| }: { | ||
| queryType: '@media' | '@container'; |
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.
Thoughts on extracting this from the type?
| queryType: '@media' | '@container'; | |
| queryType: keyof ConditionalQueries; |
| [350, 550, 700] | ||
| .map( | ||
| (width) => ` | ||
| <div style="width: ${width}px;" class="${styles.container}"> |
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.
These should probably use a max-width:
| <div style="width: ${width}px;" class="${styles.container}"> | |
| <div style="width: 100%; max-width: ${width}px;" class="${styles.container}"> |
At the moment they seem to just grow and never conditionally adapt.
| const queryVars: StyleRule[typeof queryType] = {}; | ||
|
|
||
| Object.entries(queries).forEach(([query, value]) => { | ||
| const builtValues = | ||
| 'capHeightTrim' in value | ||
| ? (value as ComputedValues) | ||
| : precomputeValues(value); | ||
|
|
||
| queryVars[query] = { vars: assignVars(capsizeVars, builtValues) }; | ||
| }); | ||
|
|
||
| return queryVars; |
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.
Can i suggest renaming queryVars to queryRule given it is generating the ruleset:
| const queryVars: StyleRule[typeof queryType] = {}; | |
| Object.entries(queries).forEach(([query, value]) => { | |
| const builtValues = | |
| 'capHeightTrim' in value | |
| ? (value as ComputedValues) | |
| : precomputeValues(value); | |
| queryVars[query] = { vars: assignVars(capsizeVars, builtValues) }; | |
| }); | |
| return queryVars; | |
| const queryRule: StyleRule[typeof queryType] = {}; | |
| Object.entries(queries).forEach(([query, value]) => { | |
| const builtValues = | |
| 'capHeightTrim' in value | |
| ? (value as ComputedValues) | |
| : precomputeValues(value); | |
| queryRule[query] = { vars: assignVars(capsizeVars, builtValues) }; | |
| }); | |
| return queryRule; |
That would go for the function too, generateQueryVars to generateQueryRule.
(I noticed this is likely extending something that was my fault, in createVanillaStyle we build up a vars variable, which is actually a "styleRule". You can update that for bonus points 😄)
| }); | ||
| ``` | ||
|
|
||
| As an alternative to passing a media query object, can also provide a vanilla-extract [container query object](https://vanilla-extract.style/documentation/styling/#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.
Small typo, but you're probably under selling it here too, i'd add an example!
| As an alternative to passing a media query object, can also provide a vanilla-extract [container query object](https://vanilla-extract.style/documentation/styling/#container-queries). | |
| #### Container queries | |
| Queries can be provided following the vanilla-extract [container query object syntax](https://vanilla-extract.style/documentation/styling/#container-queries). For example: | |
| ```ts | |
| // Text.css.ts | |
| import { createTheme, createContainer } from '@vanilla-extract/css'; | |
| import { createTextStyle, precomputeValues } from '@capsizecss/vanilla-extract'; | |
| const containerName = createContainer(); | |
| const [themeClass, vars] = createTheme({ | |
| bodyText: { | |
| small: precomputeValues({ fontSize: 14, leading: 18, fontMetrics }), | |
| medium: precomputeValues({ fontSize: 16, leading: 22, fontMetrics }), | |
| large: precomputeValues({ fontSize: 18, leading: 24, fontMetrics }), | |
| }, | |
| }); | |
| export const containerText = createTextStyle(vars.bodyText.small, { | |
| '@container': { | |
| [`${containerName} (min-width: 400px)`]: vars.bodyText.medium, | |
| [`${containerName} (min-width: 600px)`]: vars.bodyText.large, | |
| }, | |
| }); | |
| ``` |
Would you mind re-wording the intro to this section too?
Responsive typography
As a convenience for responsive styles,
createTextStyleaccepts a second argument in the form of conditional vanillia-extract styling queries, returning a responsive class list.Media queries
Queries can be provided following the vanilla-extract media query object syntax. For example:
|
Thanks for providing the
@capsizecss/vanilla-extractpackage - it makes generating and consuming capsized text styles in a vanilla-extract project very easy.I was recently trying to size text based on the container it's in with a
@containerquery and noticed this isn't supported like@mediaqueries are. Rather than opening an issue I decided to make the change myself, but I'm happy to make an issue instead if you'd prefer.This is a PR to add @container query support to the
createTextStylefunction from the@capsizecss/vanilla-extractpackage.createTextStylealready supports @media queries, accepting a vanilla-extract media query object as a second argument like so:This PR extends
createTextStyleso that you can also provide a vanilla-extract container query object as the second argument like so:I've added a basic story to
vanilla-extractStorybook namedContainerthat provides a visual demonstration of thecontainerbased behaviour:The blue outlines around each text row delimits the width of the containers wrapping each text row.
This is to indicate that the text within each container responds to the width of the container.
As mentioned, I'd be happy to open an issue instead, but thought I'd have a crack at adding
@containersupport first.