Repository navigation
Add composite type tokens - #74980
Closed
jameskoster wants to merge 3 commits into
Closed
jameskoster wants to merge 3 commits into
jameskoster wants to merge 3 commits into
Conversation
jameskoster
added
[Type] Enhancement
|
Size Change: +4.24 kB (+0.14%) Total Size: 2.99 MB
ℹ️ View Unchanged
|
Member
|
The source tokens make sense and match what I'd expect with #74391. The CSS, JavaScript, and Markdown outputs are a little less clear. The individual CSS properties are probably fine, if a bit verbose, as long as we'd have the The JavaScript and Makrdown outputs look wrong, though. These should match the actual CSS properties (and are used for the ESLint/Stylelint "known" tokens linting), but they're only including the prefix and not the property (e.g. |
Member
Contributor
Author
|
I'm not 100% certain I interpreted the feedback correctly, but I pushed some changes 🙂 |
Member
|
I pushed some minor changes in 871bea8 to generalize the behavior, since looking at how it's implemented, this seems to be a common token value type internal to Terrazzo. It should also make it easier for us to maintain. Couple other thoughts on my mind:
|
Contributor
Author
|
Yeah 'font' might not be the best category name, and now you mention it I'm curious if we want font-family, font-size, font-weight in this instance... 🤔
|
Member
|
I think we'd end up fighting against Terrazzo and the DTCG spec to try to make that happen, because the name is generated from standardized properties). The alternative with changing (or adding) a top-level grouping would have it look like, e.g.:
|
Contributor
Author
|
Got it. I suppose either suggestion works. |
Member
|
Also looping @ciampo here since he raised the question initially in #73931 (comment) . Thinking about it some more, I agree with (a) @ciampo's original observation that some primitives don't really fit the definition of "font" (e.g. line-height, letter-spacing), (b) it definitely doesn't fit the composite tokens like these, and (c) it might be nicer if we could still maintain them together. With that in mind, perhaps So we'd have
I'm also fine with |
Contributor
Apart from that, I have a more general reflection re. composite tokens. Adding composite tokens adds complexity to our setup, and creates quite a few more DS tokens (of which a few are actually aliasing the same values). Do we think they are necessary? In other words, do we think they bring much value, given that we're going to expose these presets anyway through the cc @mirka since we had discussed tokens and styles reusable utilities in the past. |
Member
I generally lean towards this. Presets of multiple values would probably be more sustainable to abstract as components or CSS utility hooks or whatever. Similar things I was planning to do this way are focus ring styles and animations. This is based upon the notion that an infinitely growing list of tokens is bad (which may be an incorrect notion, I don't know). It may be useful to agree on some criteria for what justifies a new design token, versus some other way to encapsulate variables.
I have a relatively strong preference for |
Contributor
Author
|
Yes good points, consumers should be primarily interacting with |
Member
|
With what's being discussed, is the concern about the tokens themselves, or the proliferation of CSS properties? Conceptually we could still tokenize these as part of the design system. Typography is a built-in concept within the DTCG spec. The CSS we generate is an artifact of those tokens, but it's an incidental side effect independent of the tokens themselves. We could exclude these specific tokens from the CSS, still maintain them in the token JSON, and optionally generate some artifact that's more tightly consumed by the I was partly considering whether we'd want these for Figma integration, though I don't know that with how Figma imports DTCG tokens as variables that it would help in how Figma manages text styles. |
Member
Interesting, I hadn't thought about it that way — managing values in a central data store, that doesn't necessarily have to be emitted in the design-tokens.css. I honestly don't know what specific benefits there are to having that central data store, but if there are, the downsides are definitely mitigated by not emitting everything to the design-tokens.css. |
Contributor
|
@aduth what would this alternative solution look like, in practice? Defining tokens in JSON, and then having some custom terrazzo code to generate "presets" in CSS so that they can be consumed by |
Member
|
To be honest, I'm not sure there's much value in following the approach, other than "correctness" in codifying this within our token set. At least without some use-case like being able to import those into Figma, for example. But yeah, the idea would be that we filter them out of the CSS output, and create a custom Terrazzo plugin that emits some JavaScript (or CSS, or JSON) that's not broadly loaded or shared, but instead only used by the Text components. |
Contributor
Author
|
In case it influences anything; to my knowledge Figma doesn't support importing text styles (composite tokens) currently. |
Contributor
|
Should we start with the simplest approach (ie. use |
Member
Yep, let's. |
This was referenced Mar 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@aduth is this what you had in mind for #74391?