-
Notifications
You must be signed in to change notification settings - Fork 3
feat: Add new base size and shape tokens #60
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
Conversation
Adding new size base tokens, deprecating base font-size, line-height, type-scale, and level tokens in favor of size. System font-size, line-height tokens will alias to base size tokens. Type compound tokens will alias to system font-size, line-height tokens. These bundle system tokens into a style. Shape tokens updated to use t-shirt sizing and to alias base size tokens. Depth tokens blur updated to alias to base size tokens. Y offset of ambient shadow updated to be calculated off of the base shadow.
Font size and line height now alias base size tokens. Compound tokens alias semantic tokens instead of deprecated font-size and line-height tokens
Visual Comparisonsys/breakpoint.json
sys/opacity.json
sys/shape.json
sys/type.json
|
What should this file be named? type.json is in sys font.json? typography.json?
tokens/base/size.json
Outdated
| "unit": { | ||
| "value": "0.25rem", | ||
| "type": "dimension", | ||
| "description": "The base unit used for the typographic grid system.\n", | ||
| "deprecated": true, | ||
| "deprecatedComment": "Use {base.baseline} instead for consistent sizing calculations" | ||
| }, |
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.
Deprecated tokens should be listed in different json in deprecated folder
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.
ah okay, one sec
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.
should be done
Removed deprecated level configuration from shadow.json.
Removed deprecated base unit definition from size.json.
Removed deprecated font sizes and line heights from typography.json. Moved to deprecated folder
…tokens-studio into taka-size-updates
|
@takashimokobe shape still has |
tokens/sys/type.json
Outdated
| "fontWeight": "{font-weight.medium}", | ||
| "lineHeight": "{line-height.subtext.small}", | ||
| "fontSize": "{font-size.subtext.medium}", | ||
| "letterSpacing": "{letter-spacing.subtext.medium}" |
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.
should this be md or medium
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.
yes, I'll update this
In favor of short hand "sm", "lg"
In favor of t-shirt shorthand
Fix JSON formatting by adding a newline at the end of the file.
tokens/deprecated/sys/type.json
Outdated
| } | ||
| } | ||
| }, | ||
| }, |
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.
remove this comma
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.
nice work @takashimokobe ty!!
tokens/sys/shape.json
Outdated
| "half": { | ||
| "value": "{base.unit} * 0.5", | ||
| "xs": { | ||
| "value": "{size.100}", |
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.
in decision log it said 4px, so {size.50}, what is the correct value?
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 should be size.50
| "value": "{letter-spacing.50}", | ||
| "type": "number" | ||
| }, | ||
| "md": { | ||
| "value": "{letter-spacing.100}", | ||
| "type": "number" | ||
| }, | ||
| "lg": { | ||
| "value": "{letter-spacing.150}", | ||
| "type": "number" | ||
| } | ||
| }, | ||
| "body": { | ||
| "sm": { | ||
| "value": "{letter-spacing.200}", | ||
| "type": "number" |
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.
Should it use size token too?
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.
i think letter spacing uses size token?
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.
no, letter-spacing spacing is a decimal value (for our use case) - 0.4 through 0.16
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.
i see now
Was mapped to incorrect value
Co-authored-by: Raisa Primerova <[email protected]>
| "value": 0.24, | ||
| "type": "number" | ||
| }, | ||
| "200": { |
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.
ah i see, this doesn't use size?
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.
no, letter-spacing is a decimal or percentage
we could use math but feel unnecessary
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 @takashimokobe for quick fixes
| "value": "{letter-spacing.50}", | ||
| "type": "number" | ||
| }, | ||
| "md": { | ||
| "value": "{letter-spacing.100}", | ||
| "type": "number" | ||
| }, | ||
| "lg": { | ||
| "value": "{letter-spacing.150}", | ||
| "type": "number" | ||
| } | ||
| }, | ||
| "body": { | ||
| "sm": { | ||
| "value": "{letter-spacing.200}", | ||
| "type": "number" |
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.
i see now
Base size tokens
This pr adds new base
sizetokens that represent dimensions in the 8pt grid. Naming is based off a scale number (0, 25, 100) and the baseline unit of 8pt.size.[scaleNumber]=scaleNumber/100*8This provides a scalable foundation for any dimension related system tokens to alias, including:
System token updates
blurupdated to reference basesizetokens.sizetokenssizetokenssizetokensDeprecated