[BD-46] feat: ensure design tokens have a valid type attribute

[PKulkoRaccoonGang]

Description

Ensure design tokens have a valid type attribute
Issue #1947
W3C report

Merge Checklist

• If your update includes visual changes, have they been reviewed by a designer? Send them a link to the Netlify deploy preview, if applicable.
• Does your change adhere to the documented style conventions?
• Do any prop types have missing descriptions in the Props API tables in the documentation site (check deploy preview)?
• Were your changes tested using all available themes (see theme switcher in the header of the deploy preview, under the "Settings" icon)?
• Were your changes tested in the example app?
• Is there adequate test coverage for your changes?
• Consider whether this change needs to reviewed/QA'ed for accessibility (a11y). If so, please add wittjeff and adamstankiewicz as reviewers on this PR.

Post-merge Checklist

• Verify your changes were released to NPM at the expected version.
• If you'd like, share your contribution in #show-and-tell.
• 🎉 🙌 Celebrate! Thanks for your contribution.

[openedx-webhooks]

Thanks for the pull request, @PKulkoRaccoonGang!

When this pull request is ready, tag your edX technical lead.

[codecov]

Codecov Report

Base: 90.41% // Head: 90.41% // No change to project coverage 👍

Coverage data is based on head (2998785) compared to base (5b7ecf2).
Patch has no changes to coverable lines.

Additional details and impacted files

@@           Coverage Diff           @@
##            alpha    #1992   +/-   ##
=======================================
  Coverage   90.41%   90.41%           
=======================================
  Files         192      192           
  Lines        3120     3120           
  Branches      746      746           
=======================================
  Hits         2821     2821           
  Misses        283      283           
  Partials       16       16           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[viktorrusakov]

Hmm, I don't think we should use file here, we're not loading any files with the url, it's just an svg. Maybe we can introduce svg type for such cases, even though it's not covered by W3C specs?

@adamstankiewicz curious what you think

[adamstankiewicz]

@viktorrusakov Good question... I'm a bit hesitant to introduce a svg type that isn't officially covered by the W3C spec. The file type per the W3C spec seems to indicate the file type might still be in flux as well. Given SVGs are technically files, even though its not defined as a separate file here, I'm inclined to leave these tokens as "type": "file" for the time being.

[adamstankiewicz]

Based on the documentation for the shadow token type, it is a composite token made up of multiple properties (docs):

{
  "shadow-token": {
    "$type": "shadow",
    "$value": {
      "color": "#00000080",
      "offsetX": "0.5rem",
      "offsetY": "0.5rem",
      "blur": "1.5rem",
      "spread": "0rem"
    }
  }
}

As is, we're defining the shadow using a CSS-specific value. Because shadow is a composite token, each aspect of the shadow is defined as its own property. style-dictionary seems to have support for composite tokens but they may need custom transforms/formatters to handle them properly for each platform.

Conceptually, this makes sense; the tokens are intended to be platform agnostic such that they can be transformed into platform-specific implementations. In the current form, we're essentially assuming the shadow token will always be used as a CSS value, which wouldn't be compatible if/when we transform tokens for iOS/Android apps eventually.

There seems to be consensus in the W3C issues regarding the shadow composite type that it should support an array for its value to define multiple layers of shadow, as is the use case for this token. While it's not officially documented, I think it's safe to assume moving in that direction makes sense.

In refactoring, we'd likely need to implement a custom transformer to deal with the composite shadow type, perhaps similar to this example.

[adamstankiewicz]

[curious] I believe strokeStyle refers more to things like border stroke, not so much the text-decoration. Since the underline here is related to text-decoration, it may be an additional property on composite tokens of type typography (docs).

Since this is still up in the air in the W3C spec, perhaps we can opt for defining a textDecoration type here to represent the enum of choices available for text-decoration similar to what W3C is proposing for the fontFamily and fontWeight token types.

{
  "typography": {
    "link": {
      "decoration": {
        "base": { "value": "none", "type": "textDecoration", "source": "$link-decoration" },
      }
    }
}

[PKulkoRaccoonGang]

Yep, it makes sense 👍

[adamstankiewicz]

[curious] Would love to hear your thoughts on whether we should start defining typography-related tokens as composite tokens based on the proposed spec. For example, these tokens defining the text styles for the badge component might be written as:

{
  "typography": {
    "badge": {
      "type": "typography",
      "value": {
        "fontSize": "75%",
        "fontWeight": "{typography.font.weight.bold}"
      }
    }
  }
}

This would likely require implementation of a custom transform to deal with the composite typography type, perhaps similar to this example.

For example, Figma's Tokens Studio plugin allows composite typography tokens.

My biggest concern is that if we keep the distinct token approach we have now vs. refactoring to composite tokens is that if we want to switch down the road after design tokens are out in the wild, it'd be a breaking change.

Curious to hear how you feel about composite typography tokens vs. having distinct token definitions for each typography property as we currently have.

[adamstankiewicz]

[curious] The transition token type per the spec is a composite token. Similar question here as other comments around whether we should adopt the composite tokens vs. single token per property approach.

[adamstankiewicz]

[curious] what is the ratio type representing here?

[PKulkoRaccoonGang]

@viktorrusakov proposed to use the type: "position". Its a good type for the CSS property align-self (start, center, end).

[viktorrusakov]

yeah, I don't think W3C specs have a type for such properties, position sounds good to me.

@adamstankiewicz what do you think?

[adamstankiewicz]

position sounds good here, too +1

[adamstankiewicz]

[consideration] Curious for your thoughts on whether we should define transitions as a composite token? Primary rationale being that having the separate parts helps define the style properties in a more platform agnostic way than this CSS implementation. It would likely require a custom transform, though.

[openedx-webhooks]

@PKulkoRaccoonGang 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

[edx-semantic-release]

🎉 This PR is included in version 21.0.0-alpha.10 🎉

The release is available on:

• npm package (@alpha dist-tag)
• GitHub release

Your semantic-release bot 📦🚀