terrazzoapp / terrazzo

Use DTCG tokens JSON to generate code for web, mobile, native apps, and more
https://terrazzo.app
MIT License
152 stars 28 forks source link

Possible bug in typography-mixins output in plugin-sass #198

Open torstendaeges opened 8 months ago

torstendaeges commented 8 months ago

Thank you again for the quick fixes!

I think I found another one: When using plugin-sass in conjunction with plugin-css (CSS Variable Mode), the "default" mode of mixins will use CSS variables, while the following modes are still using the actual values. As I unserstand it, they too should be using the variables to be fully dynamic.

Here's a sample output as generated by the input below:

  "fontalias": (
    default: (
      "font-family": (var(--fontalias-font-family)),
      "font-size": (var(--fontalias-font-size)),
    ),
    "l": (
      "font-family": (Helvetica, -system-ui, sans-serif),
      "font-size": (2rem),
    ),
    "m": (
      "font-family": (Helvetica, -system-ui, sans-serif),
      "font-size": (1.5rem),
    ),
    "s": (
      "font-family": (Helvetica, -system-ui, sans-serif),
      "font-size": (1rem),
    ),
  ),

Here's the tokens.config.mjs ...

import pluginSass from '@cobalt-ui/plugin-sass';
/** @type {import('@cobalt-ui/core').Config} */

export default {
  tokens: ['./tokens.json'],
  plugins: [
    pluginSass({
      filename: 'test.scss',
      pluginCSS: {
        filename: 'test.css',
        modeSelectors: [
          {mode: 'l', selectors: ['[data-viewport="l"]']},
          {mode: 'm', selectors: ['[data-viewport="m"]']},
          {mode: 's', selectors: ['[data-viewport="s"]']},
          ],
      },
    }),
  ],
};

...and the tokens.json to reproduce this:

{
    "bodyTextSmall": {
      "$type": "typography",
      "$value": {
        "fontFamily": ["Helvetica", "-system-ui", "sans-serif"],
        "fontSize": "1rem"
      }
    },
    "bodyTextMedium": {
        "$type": "typography",
        "$value": {
          "fontFamily": ["Helvetica", "-system-ui", "sans-serif"],
          "fontSize": "1.5rem"
        }
      },
      "bodyTextLarge": {
        "$type": "typography",
        "$value": {
          "fontFamily": ["Helvetica", "-system-ui", "sans-serif"],
          "fontSize": "2rem"
        }
      },
    "fontalias": {
        "$type": "typography",
        "$value": "{bodyTextLarge}",
        "$extensions": {
            "mode": {
                "l": "{bodyTextLarge}",
                "m": "{bodyTextMedium}",
                "s": "{bodyTextSmall}"
            }
        }    
    }
}

Hope this isn't the way it is supposed to be. :) Thank you!

drwpow commented 8 months ago

Thank you, keep the bug reports coming! I probably won’t be able to get to this till next week if that’s OK.

Normally I’d encourage you (or others) to make a PR but this specifically is a nasty bug that’s probably caused by some of the architecture decisions. Don’t let me stop you from trying if you’re interested! But though the fix is relatively quick, it’s not straightforward at all and slightly-annoying to wade through

drwpow commented 8 months ago

Ah so actually, this is generating correctly in plugin-sass as-intended. But it may not be as you expected.

The current behavior of plugin-sass with modes is to always return the raw value rather than the CSS variable. The reason is: with the modeSelectors set up in CSS, those should actually handle modes for you. When using token("fontAlias") in Sass, it will pull the var(--font-alias-*) variables with the understanding that the modeSelectors will dynamically change the value as-needed.

However, if we used variables for token("fontAlias", "m"), then it would return the same value, and thus there wouldn’t really be “mode” selecting, in a sense. The only way to override this would be if we had, say --font-alias-m in :root, we could return that. But since that would be ignoring the modeSelectors, it would essentially work the same way as the raw values would (unless you were applying other overrides, in which case Cobalt won’t know what you’re doing and can’t anticipate it).

So all that to say, perhaps the usage of token("…") in Sass when specifying a mode isn’t intuitive. Or perhaps there’s a usecase I’m missing. In any case, would love feedback if there’s anything that could be improved here.

torstendaeges commented 8 months ago

We can certainly see where you're coming from regarding the mode-handling in plugin-sass' mixin output. If I understand correctly, we'd normally use the standard "." value of the mixin, which respects css mode switching. So the modes of the mixin are only used if we need to override css mode switching - but unlike how it's done with the "$__token-values" (where, thanks to the bugfix, the mode targets are used), the overrides use raw values.

Using raw values here makes sense to me and seems to work for us, but I keep wondering if that prevents some usecases:

Say there are typography tokens with modes (e.g. l, m, s) and the values of their properties are aliases. Say these aliases point to tokens that again have modes. In such a case, using raw-values instead of the mode targets for the overrides would possibly circumvent additional alias-levels and therefore break those other mode switches. So I guess my solution would be to use raw values if the targets are tokens with raw values and the mode targets as css variables if these targets have aliases as their values.

Does that make sense? Hope I could describe this in an understandable way.

drwpow commented 7 months ago

Yes that makes sense! I’ve actually been investigating a related problem with modes and theming in the DTCG format discussions: mode scoping. You won’t find the term “mode scoping” explicitly referred to by any single comment, and it’s just a term I came up with to describe this problem. But if you look carefully, you’ll find this problem exists in nearly every comment, from every team implementing it. I’ve been trying to outline the problem in some notes I’ve been taking on the modes problem:

Another problem is “mode scoping,” an issue that’s been talked about frequently in the DTCG GitHub and elsewhere, but something no proposal addresses. Many design systems implement some variation of base → semantic → component token layers (example). But because there are no restrictions over whether modes cross these “layers” or not, it can lead to confusion and churn.

As a practical example, pretend you have a “base” grayscale from 100 – 1300 (values don’t matter), and you decide your semantic.text-color token is gray.700. This ends up being “light mode,” and when you add “dark mode,” you use gray.600 to get roughly the same contrast. Shortly on, you realize that your dark mode gray.600 needs to be adjusted to improve contrast—you need a color that doesn’t exist in gray.100 – 1300 at all. So the question becomes: does semantic.text-color alias gray.700 for light and dark modes, and the modes are thrust onto gray.700 so it has multiple values? Or does that go against the idea of “stateless” core tokens, so we create a separate scale of gray-light.XXX and gray-dark.XXX, so light and dark modes can exist on the semantic layer)? The former answer is more “clever” but blurs the line between the core and semantic layers as the core takes on more “awareness.” The latter answer preserves the layer boundaries better, but introduces the idea that there are “rules“ over when core tokens can/can’t be used in certain contexts (arguably also blurring the lines, but in a different manner). Either answer will “fork” the design system in 1 of 2 ways that is difficult/impossible to reverse down the line.

A hand-wavy answer to this is “it’s up to every team to decide.” But when we consider every design system implementing modes faces this same confusion, and it stems directly from how teams define modes/theming (i.e. the purpose of this document), this may not be a difference of opinion so much as it a symptom that modes/theming are ill-defined in any token system (and could probably be remedied with a more opinionated solution).


So back to your suggestion:

So I guess my solution would be to use raw values if the targets are tokens with raw values and the mode targets as css variables if these targets have aliases as their values.

That’s how you and your team are using aliases, but other teams may have other assumptions about how aliasing works. And you also have strong opinions in that thread like “core tokens should never have aliases; all aliases should only be at the semantic layer and semantic tokens should only alias core tokens.” Which still doesn’t answer the question of how modes do/don’t apply to aliases, but is just another example.

If you’re confused by now, so am I 😄. My whole point is “this problem has layers to it, and many people have their own assumptions about how it should/shouldn’t work,” and so even some of Cobalt’s current behavior has been tiptoeing around the modes ↔ aliasing problem and shouldn’t be considered intentional decisions (not all).

But I’m currently talking with others involved in DTCG to have more of an opinionated take on this, because I think we are all starting to realize that any attempt at resolving this can happen in the DTCG format itself. And it would save all of us from having to solve these same problems in different ways.

torstendaeges commented 7 months ago

Hey Drew,

writing this, I got the feeling that it got away too much from the topic of the issue itself, so I thought it better to write an E-Mail. :) Sorry that it got so long.

You're right, design tokens are kind of the wild west right now. Other than syntax and type, you could use millions of naming conventions, layers, alias structures... It took our team way too much trial and error to find a structure that works for us, especially when you consider that one has to make sure it will also fit the mental models behind the tools used, like Cobalt, Figma and so on - and future standards for good measure. :) I'm pretty sure some teams will just give up along the way and will end up using something half-baked.

I also think that it would help design tokens to gain acceptance and find traction much faster if there were some basic rules in the DTCG spec - so I'm glad there are discussions on that. After all, some freedoms only make it way harder to maintain your tokens down the line. But I digress.

In our team, we were lucky to have decided on a few rules from the onset:

So i guess we're using the Matrix setup for modes to bridge the different tools.

Some observations:

BTW: I'll still get back to you regarding your questions on those boolean and string types. Also, I think I spotted a typo in your "Token Orchestration" document - shouldn't it say "with" instead of "without" in the first line?

Have a nice sunday and thanks again for your great work! Torsten

Am Mi., 13. März 2024 um 20:19 Uhr schrieb Drew Powers < @.***>:

Yes that makes sense! I’ve actually been investigating a related problem with modes and theming in the DTCG format discussions https://github.com/design-tokens/community-group/issues/210: mode scoping. You won’t find the term “mode scoping” explicitly referred to by any single comment, and it’s just a term I came up with to describe this problem. But if you look carefully, you’ll find this problem exists in nearly every comment, from every team implementing it. I’ve been trying to outline the problem in some notes I’ve been taking on the modes problem https://docs.google.com/document/d/1GdvLiO57xQKGKtVvQ_8I8Cbi0wLJTV-ChNaNpzZtoh8/edit?usp=sharing :

Another problem is “mode scoping,” an issue that’s been talked about frequently in the DTCG GitHub and elsewhere, but something no proposal addresses. Many design systems implement some variation of base → semantic → component token layers (example https://uxdesign.cc/naming-design-tokens-9454818ed7cb). But because there are no restrictions over whether modes cross these “layers” or not, it can lead to confusion and churn.

As a practical example, pretend you have a “base” grayscale from 100 – 1300 (values don’t matter), and you decide your semantic.text-color token is gray.700. This ends up being “light mode,” and when you add “dark mode,” you use gray.600 to get roughly the same contrast. Shortly on, you realize that your dark mode gray.600 needs to be adjusted to improve contrast—you need a color that doesn’t exist in gray 100 – 1300 at all. So the question becomes: does semantic.text-color end up being gray.700 for light and dark modes, and instead gray.700 gets light and dark modes? Or does that go against the idea of “stateless” core tokens, so we create a separate scale of gray-light.XXX and gray-dark.XXX, so light and dark modes can exist on the semantic layer)? The former answer is more “clever” but blurs the line between the core and semantic layers as the core takes on more “awareness.” The latter answer preserves the layer boundaries better, but introduces the idea that there are “rules“ over when core tokens can/can’t be used in certain contexts (arguably also blurring the lines, but in a different manner). Either answer will “fork” the design system in 1 of 2 ways that is difficult/impossible to reverse down the line.

A hand-wavy answer to this is “it’s up to every team to decide.” But when we consider every design system implementing modes faces this same confusion, and it stems directly from how teams define modes/theming (i.e. the purpose of this document), this may not be a difference of opinion so much as it a symptom that modes/theming are ill-defined in any token system (and could probably be remedied with a more opinionated solution).


So back to your suggestion:

So I guess my solution would be to use raw values if the targets are tokens with raw values and the mode targets as css variables if these targets have aliases as their values.

That’s how you and your team are using aliases, but other teams may have other assumptions about how aliasing works. And you also have strong opinions in that thread https://github.com/design-tokens/community-group/issues/210#issuecomment-1633721916 like “core tokens should never have aliases; all aliases should only be at the semantic layer and semantic tokens should only alias core tokens.” Which still doesn’t answer the question of how modes do/don’t apply to aliases, but is just another example.

If you’re confused by now, so am I 😄. My whole point is “this problem has layers to it, and many people have their own assumptions about how it should/shouldn’t work,” and so even some of Cobalt’s current behavior has been tiptoeing around the modes ↔ aliasing problem and shouldn’t be considered intentional decisions (not all).

— Reply to this email directly, view it on GitHub https://github.com/drwpow/cobalt-ui/issues/198#issuecomment-1995465757, or unsubscribe https://github.com/notifications/unsubscribe-auth/BAJK5DHOOEWGQGZGH24E7MTYYCRDHAVCNFSM6AAAAABDNMMBU2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOJVGQ3DKNZVG4 . You are receiving this because you authored the thread.Message ID: @.***>