Replace Theo with Style Dictionary #967

spaceninja · 2020-09-30T00:04:31Z

Overview

This PR changes the tool we use to manage our design tokens from Theo to Style Dictionary. We're making this change because we've used it on several other projects and been impressed with the flexibility it gives us.

Testing

Review the preview environment, and confirm there are no visual changes other than the color values changing from RGB to hex values.

Check out the branch locally, run npm ci and npm run storybook
Try editing a design token and confirm that it updates in Storybook.

Code Review

Apologies for the size of this changeset. You'll probably want to break your code review up by file type:

SCSS files should only contain variable name updates
YML files have all been deleted (those are the old token files)
JSON files are mostly new design token files
JS files are all either design token files or tooling to help build them
MDX files are either new (replacing some previously auto-generated design token stories) or updates to how they consume tokens.

Fixes Display hex color values #888

changeset-bot · 2020-09-30T00:04:34Z

⚠️ No Changeset found

Latest commit: 2d6649d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

spaceninja · 2020-09-30T00:27:00Z

Note there is one bit of code that I'd like to DRY up — at the top of most of the design token story files (see src/tokens/*.stories.mdx) there's a repeated bit of code to generate a table of properties. This should ideally be pulled out into a separate file. Unfortunately, when I did so, I got errors about JSX that I wasn't able to quickly resolve. I talked to @calebeby for help, and while we couldn't fix it in the time we had, he mentioned another possibility was converting it to a React component (like ColorPalette)

Since I'm being pulled back to client work tomorrow and this PR is already pretty big, I decided to leave the code as-is for now. Assuming @tylersticka is okay with it, I'll file a followup ticket to come back and DRY that up.

(Note if someone else is interested, the "definitive" version of that code can be found in font.stories.mdx.)

spaceninja · 2020-09-30T00:29:39Z

@calebeby I tagged you in this as an FYI since we were talking about the JSX code not working. Don't feel obligated to review, though.

tylersticka

It's nice that Style Dictionary doesn't require as much up-front logic as Theo did. But I'll be honest, this doesn't feel like an upgrade in its current state:

As mentioned in the PR description, we're now requiring repetitive code across the story documentation that we weren't before. This will make it more difficult for contributors to make new token groups.
Previously, you would @import a category once and then refer to it by that name. Now, every token is output to a single monolithic tokens object, increasing the number of characters you need to type to refer to them. This will also make looping over tokens more difficult than it would be otherwise, something we've had to hack around in a different project.
Although we already have a dependency for defining JS build scripts (Gulp) and triggering it on file changes (also Gulp), this PR uses a bare Node script and a new Chokidar dependency. This feels like an artifact of a different project and confuses the organization of this specific project.

While I support this change long-term, my feelings haven't changed since Monday: If this will take more than a couple of hours to resolve, it's probably best to create an issue and return to it after more of our user-facing tasks have been addressed.

build-tokens.js

package.json

src/base/_defaults.scss

* v-next: (156 commits) Update dependency js-yaml to v4 (#1119) Update dependency @whitespace/storybook-addon-html to v4 (#1105) Update Storybook and Storybook Addons Update dependency @cypress/webpack-preprocessor to v5.6.0 Update babel monorepo to v7.12.17 Update dependency css-loader to v5.0.2 Update dependency @changesets/cli to v2.14.1 Update babel monorepo to v7.12.16 Object: Bunch (#1130) Update dependency sass to v1.32.5 Update dependency postcss-loader to v4.2.0 Update dependency mini-css-extract-plugin to v1.3.5 Update dependency @changesets/cli to v2.14.0 Update dependency cypress to v6.3.0 Update dependency @wordpress/block-library to v2.27.2 Update dependency @rollup/plugin-node-resolve to v11.1.0 Update dependency @changesets/cli to v2.13.1 Update dependency @changesets/changelog-github to v0.2.8 Update dependency storybook-mobile to v0.1.29 Update dependency sass to v1.32.2 ...

spaceninja · 2021-03-04T00:44:26Z

The danger of walking away from a PR of this scope for six months is that the merge conflicts when you come back will take a full day to reconcile. I've now got this PR updated and I'll tackle the changes @tylersticka requested shortly.

* v-next: Add Sky Nav Behavior (#748)

spaceninja · 2021-03-10T02:47:45Z

@tylersticka I've addressed your feedback, and I think you'll be much happier with the current state of this PR, especially the new token groupings, which allows for using the Sass tokens in almost exactly the same way as before.

The Sass @use groups are manually configured, but adding new ones is pretty easy if needed. Also, the previous tokens file is still generated, so if a designer ever feels slowed by the process, they can still access new tokens that way, and file a ticket for a dev to create new groups if needed.

The one thing I'm unhappy with in this PR is the Story files for the tokens. Previously, these were automatically generated by Theo. Now they're manually created, and still have the duplicate code issue.

However, I think the rest of the changes are beneficial enough to warrant merging this as-is and filing a new issue to address that. It should be possible to generate the Story files with Style Dictionary, but this PR is already quite large. Unless you see any reason we shouldn't move forward with Style Dictionary, I'd like to merge this and start working on that ticket as a follow-up.

Let me know if you'd like to talk about this PR with voices.

CC'ing @Paul-Hebert as a reviewer since he's had hands on Style Dictionary recently for a client project and may have insight or feedback.

tylersticka

This is awesome. I love the changes to naming conventions, the config for categories seems intuitive to me, I dig the use of the modular functions instead of custom transform types, and overall the outputs seem like the best of both worlds now.

The only thing stopping me from marking "Approve" is that I have not had time to do a pass for regressions in the patterns. If someone else is able to do so, that would be swell!

Concerns have been addressed

Paul-Hebert · 2021-03-10T17:56:56Z

I should be able to do a regression pass this afternoon 👍

spaceninja · 2021-03-10T18:11:25Z

Added a new issue for DRYing the design token story files up, or auto-generating them with Style Dictionary: #1143

Paul-Hebert · 2021-03-10T20:53:43Z

Doing a regression pass now (not planning to review the code since it sounds like Tyler has already but if you'd like code review as well lemme know)

Paul-Hebert

@spaceninja I did a regression test and I spotted no differences on components, utilities, etc.

I did notice that for some reason on the preview deploy the Prism docs page is missing. I also see an error in the console:

Unexpected error while loading ./vendor/prism/prism.stories.mdx: ReferenceError: Prism is not defined

I'm not sure what the deal is... I don't see any obvious changes that would have broken this but I also didn't do a deep dive on the code. Any idea what's going on there?

spaceninja · 2021-03-10T22:48:01Z

Okay, I fixed the Prism error. No idea why that broke on my branch, but the fix was simple enough.

Paul-Hebert

🚢

spaceninja added 7 commits September 25, 2020 17:06

wip

e034839

add missing sizing tokens

d2b126a

begin replacing token variables

3213575

update sass variables

28e62bc

update build steps

0e0362a

replace token stories, remove theo

845d49c

add watch:tokens script

467eca2

remove old generated tokens from ignore files

656c9ed

spaceninja changed the title ~~Feature/style dictionary~~ Replace Theo with Style Dictionary Sep 30, 2020

fix github actions

22471b2

spaceninja requested review from tylersticka and calebeby September 30, 2020 00:28

spaceninja marked this pull request as ready for review September 30, 2020 00:28

tylersticka previously requested changes Sep 30, 2020

View reviewed changes

build-tokens.js Outdated Show resolved Hide resolved

build-tokens.js Outdated Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

package.json Show resolved Hide resolved

src/base/_defaults.scss Outdated Show resolved Hide resolved

spaceninja removed the request for review from calebeby September 30, 2020 16:44

spaceninja added 11 commits March 3, 2021 11:36

lint fix

a2f4171

fix build

9d5e461

update ci and storybook config

44c49f7

fix storybook config

59d88d4

fix aspect ratio boxes

009feb6

removing old token file

11deeda

this was removed intentionally, do not restore it

7deb164

update storybook config

492f456

standardize sass @use sort order

462c225

remove deleted file

59cd335

spaceninja added 14 commits March 8, 2021 15:49

move style dictionary config and build script

8676261

move token watch to gulp

63abb33

Merge branch 'v-next' into feature/style-dictionary

0bd077f

* v-next: Add Sky Nav Behavior (#748)

update sky-nav to fix build

7d38fb2

generate category scss files, update stories

c07213d

use color tokens

749fe83

fix typo

ecf8dc4

use breakpoint tokens

c19870d

use font tokens

0abeaad

update layout, scale, and opacity tokens

bd7eb55

use motion tokens

375f657

use size tokens

94c23e3

remove generic tokens file

e6af0b2

lint fixes

755f0c7

spaceninja requested review from tylersticka and Paul-Hebert March 10, 2021 02:47

tylersticka reviewed Mar 10, 2021

View reviewed changes

Paul-Hebert reviewed Mar 10, 2021

View reviewed changes

spaceninja added 2 commits March 10, 2021 14:40

fix Prism

5a4f4ef

fix key error

8707eb9

typo

2d6649d

Paul-Hebert approved these changes Mar 10, 2021

View reviewed changes

spaceninja merged commit 78334b8 into v-next Mar 10, 2021

spaceninja deleted the feature/style-dictionary branch March 10, 2021 23:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace Theo with Style Dictionary #967

Replace Theo with Style Dictionary #967

spaceninja commented Sep 30, 2020 •

edited

Loading

changeset-bot bot commented Sep 30, 2020 •

edited

Loading

spaceninja commented Sep 30, 2020 •

edited

Loading

spaceninja commented Sep 30, 2020

tylersticka left a comment •

edited

Loading

spaceninja commented Mar 4, 2021

spaceninja commented Mar 10, 2021

tylersticka left a comment •

edited

Loading

Paul-Hebert commented Mar 10, 2021

spaceninja commented Mar 10, 2021

Paul-Hebert commented Mar 10, 2021

Paul-Hebert left a comment

spaceninja commented Mar 10, 2021

Paul-Hebert left a comment

Replace Theo with Style Dictionary #967

Replace Theo with Style Dictionary #967

Conversation

spaceninja commented Sep 30, 2020 • edited Loading

Overview

Testing

Code Review

changeset-bot bot commented Sep 30, 2020 • edited Loading

⚠️ No Changeset found

spaceninja commented Sep 30, 2020 • edited Loading

spaceninja commented Sep 30, 2020

tylersticka left a comment • edited Loading

Choose a reason for hiding this comment

spaceninja commented Mar 4, 2021

spaceninja commented Mar 10, 2021

tylersticka left a comment • edited Loading

Choose a reason for hiding this comment

Paul-Hebert commented Mar 10, 2021

spaceninja commented Mar 10, 2021

Paul-Hebert commented Mar 10, 2021

Paul-Hebert left a comment

Choose a reason for hiding this comment

spaceninja commented Mar 10, 2021

Paul-Hebert left a comment

Choose a reason for hiding this comment

spaceninja commented Sep 30, 2020 •

edited

Loading

changeset-bot bot commented Sep 30, 2020 •

edited

Loading

spaceninja commented Sep 30, 2020 •

edited

Loading

tylersticka left a comment •

edited

Loading

tylersticka left a comment •

edited

Loading