Repository navigation
Theme: Refine typography tokens #73931
There are no files selected for viewing
|
Contributor
There was a problem hiding this comment. I feel like
Member
Author
There was a problem hiding this comment. Hm, it's a good question. I could have sworn I took inspiration from this somewhere in how the file and top-level grouping was named, but the examples I could find in either the spec or examples in Terrazzo documentation either don't mention it, or do group them as "typography".
Member
Author
There was a problem hiding this comment. I think how I may have been considering it was reserving "typography" as a grouping for the compositions of individual font token values, similar to how it's documented in the specification with
Contributor
There was a problem hiding this comment. I see. In my mind, I still think that "type" or "typography" is a better name for this group, and it seems to match how terrazzo and others' approach too (colors, typography, spacing...). That doesn't mean that we can not have In fact, it poses the question of how we should structure those "presets" — are they semantic tokens, or an even higer-level entity?
Contributor
There was a problem hiding this comment. @jameskoster out of curiosity, do you foresee the need for more tokens in the future (targeting things like letter spacing or other metrics)?
Contributor
There was a problem hiding this comment. Maybe 😄 Difficult to say at this stage. I don't think letter-spacing is used in the existing componentry. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,57 +37,68 @@ | |
| }, | ||
| "size": { | ||
| "$type": "dimension", | ||
| "xs": { | ||
| "$value": { "value": 11, "unit": "px" }, | ||
| "$description": "Extra small font size" | ||
| }, | ||
| "sm": { | ||
| "$value": { "value": 12, "unit": "px" }, | ||
| "$description": "Small font size" | ||
| }, | ||
| "md": { | ||
| "$value": { "value": 13, "unit": "px" }, | ||
| "$description": "Medium font size" | ||
| }, | ||
| "lg": { | ||
| "$value": { "value": 15, "unit": "px" }, | ||
| "$description": "Large font size" | ||
| }, | ||
| "xl": { | ||
| "$value": { "value": 20, "unit": "px" }, | ||
| "$description": "Extra large font size" | ||
| }, | ||
| "2xl": { | ||
| "$value": { "value": 32, "unit": "px" }, | ||
| "$description": "2X large font size" | ||
| } | ||
| }, | ||
| "line-height": { | ||
|
Contributor
There was a problem hiding this comment. Not a blocker for now, but have we considered which unit to use to express these tokens? Using
Member
Author
There was a problem hiding this comment. I'm not personally aware of any consideration to that extent, but I'd be curious as well. I think it's a change we could choose to make down the line also.
Member
There was a problem hiding this comment. I have mentally put I'm interested in the px/unitless decision for line-height. That has a lot of implications, not only on new things, but for migrations of existing styles (almost all unitless?).
Contributor
There was a problem hiding this comment. Re |
||
| "$type": "dimension", | ||
| "xs": { | ||
| "$value": { "value": 16, "unit": "px" }, | ||
| "$description": "Extra small line height" | ||
| }, | ||
| "sm": { | ||
| "$value": { "value": 20, "unit": "px" }, | ||
| "$description": "Small line height" | ||
| }, | ||
| "md": { | ||
| "$value": { "value": 24, "unit": "px" }, | ||
| "$description": "Medium line height" | ||
| }, | ||
| "lg": { | ||
| "$value": { "value": 28, "unit": "px" }, | ||
| "$description": "Large line height" | ||
| }, | ||
| "xl": { | ||
| "$value": { "value": 32, "unit": "px" }, | ||
| "$description": "Extra large line height" | ||
| }, | ||
| "2xl": { | ||
| "$value": { "value": 40, "unit": "px" }, | ||
| "$description": "2X large line height" | ||
| } | ||
| }, | ||
| "weight": { | ||
| "$type": "fontWeight", | ||
| "regular": { | ||
| "$value": 400, | ||
| "$description": "Regular font weight for body text" | ||
| }, | ||
| "medium": { | ||
| "$value": 499, | ||
| "$description": "Medium font weight for emphasis and headings" | ||
| } | ||
|
Comment on lines
+94
to
+101
Member
Author
There was a problem hiding this comment. I'm not 100% on these tokens, and would welcome feedback. These are being carried forward from existing variables: gutenberg/packages/base-styles/_variables.scss Lines 38 to 40 in ac35a71 Note that I did not use the
Member
There was a problem hiding this comment. Being a browser app, isn't it plausible that the same quirk would manifest in Figma though?
Member
Author
There was a problem hiding this comment.
That's a good question. I'm not really sure. Doing some brief inspecting, it looks like the main canvas area in Figma is a I think the root of the decision here is codifying the decision that we want
Member
There was a problem hiding this comment. I think we need to ship with
Member
Author
There was a problem hiding this comment. Ok, let's go with that. Updated in 5d57c17 . This also means it'll be 499 in Figma, which may be what we want or expect, but may also be slightly misaligned.
Contributor
There was a problem hiding this comment. I tested it on the web version of Figma on Windows OS, but it seems that Segoe UI cannot be selected in the first place. 
On the other hand, in a local app, I can select Segoe UI, but I cannot enter a font weight numerically. 
It seems like 500 is probably fine for design tokens.
Member
Author
|
||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| export * from './badge'; | ||
| export * from './box'; | ||
| export * from './form/primitives'; | ||
| export * from './stack'; | ||
| export * from './visually-hidden'; |
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 occurs to me that we call this "medium", where in other sizing tokens we use "md" abbreviated t-shirt size naming. Maybe we're okay with this seeming inconsistency? There's not an obvious abbreviated form to represent all the options here (i.e. "regular").
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 I'm ok with font weight tokens not to be abbreviated