chore(design): Added descriptions to semantic tokens JSON files

[edison-cy-yang]

Motivations

This is an enabler so we can use semantic descriptions in docs

Changes

Added

Changed

Deprecated

Removed

Fixed

Security

Testing

Changes can be tested via Pre-release

---

In Atlantis we use Github's built in pull request reviews.

[edison-cy-yang]

@chris-at-jobber A couple of questions that I came across while adding these descriptions:

• should some of the tokens json structures be changed, eg dont make subtle on the same level as hover in interactive- of semanticColor.tokens.json?

• semantic colors for workflows in light mode are set in workflow.tokens.json but the ones for dark mode are set in dark.tokens.json. Should we make the locations of these tokens consistent?

• Should the same description be applied to the hover, surface, and onSurface states of a semantic color?

[chris-at-jobber]

@edison-cy-yang hopefully this answers the questions!

Token structure

If that would change the output values then I would say no, as that would be a massive breaking change, but if it's not, then maybe? I'm not sure what exactly that change would look like.

Workflow colors

I would be OK with moving the token definitions from workflow.tokens into semanticColor.tokens, I'm not sure if that would have any knock-on effects (@scotttjob might know!)

Description for hover/etc

Couldn't hurt! I think as a general rule for

• hover tokens
  • "Hover state for {the token it's referencing}"
• surface/onSurface tokens
  • "Use for the surface of an element that has {status it's referencing} status"
  • "Use for text or iconography that sits overtop a {same status surface value} background"

[scotttjob]

If that would change the output values then I would say no, as that would be a massive breaking change, but if it's not, then maybe? I'm not sure what exactly that change would look like.

Changing the structure of the JSON would definitely change the final token key. (each 'level' traversed in the JSON file is translated to a dash in the final token)

I would be OK with moving the token definitions from workflow.tokens into semanticColor.tokens, I'm not sure if that would have any knock-on effects (@scotttjob might know!)

The only concern I have is for the token aliases, I believe they'll still be processed properly but would want to verify that.

Ideally the only thing that would change from this would be potentially the tokens appearing in a different order in the generated output, but the contents would be identical. So I would be comfortable with this update if I could see that Diff as part of the PR.

[cloudflare-workers-and-pages[bot]]

Deploying atlantis with    Cloudflare Pages

Latest commit:	304c86a
Status:	✅  Deploy successful!
Preview URL:	https://4bd215d7.atlantis.pages.dev
Branch Preview URL:	https://job-105692-add-descriptions.atlantis.pages.dev

View logs

[edison-cy-yang]

@chris-at-jobber @scotttjob Thanks! I will leave those structural JSON changes out for now. I want to take a look at parsing token descriptions next, hoping I will have more understanding and feel more comfortable making these by then.

Descriptions for hover, surface, and onSurface states added.

[edison-cy-yang]

I ran through these quickly, did not run the code directly.

Can we run a quick diff on the output of the generated files before and after this change? There should be no change.

I normally wouldn't ask for a diff, but there's enough changes in here that a diff is a good sanity check.

@scotttjob those generated css and js files are gitignored, by seeing the diff are you talking about temporarily adding those files to the index and running diff on them (then removing them from index)?

In this case would it make sense to include this step (in a shell script) as part of running npm run bootstrap just to confirm the diffs are what we expected?

[scotttjob]

I ran through these quickly, did not run the code directly. Can we run a quick diff on the output of the generated files before and after this change? There should be no change. I normally wouldn't ask for a diff, but there's enough changes in here that a diff is a good sanity check.

@scotttjob those generated css and js files are gitignored, by seeing the diff are you talking about temporarily adding those files to the index and running diff on them (then removing them from index)?

In this case would it make sense to include this step (in a shell script) as part of running npm run bootstrap just to confirm the diffs are what we expected?

I don't think we have to go that far, I was thinking just a manual diff. So if you swapped branches back to master, ran the build command, copied the files out, swapped branches, ran the command again, then manually compared.

So a one-off screenshot. :)

[edison-cy-yang]

I ran through these quickly, did not run the code directly. Can we run a quick diff on the output of the generated files before and after this change? There should be no change. I normally wouldn't ask for a diff, but there's enough changes in here that a diff is a good sanity check.

@scotttjob those generated css and js files are gitignored, by seeing the diff are you talking about temporarily adding those files to the index and running diff on them (then removing them from index)? In this case would it make sense to include this step (in a shell script) as part of running npm run bootstrap just to confirm the diffs are what we expected?

I don't think we have to go that far, I was thinking just a manual diff. So if you swapped branches back to master, ran the build command, copied the files out, swapped branches, ran the command again, then manually compared.

So a one-off screenshot. :)

Picked a few files

[scotttjob]

I ran through these quickly, did not run the code directly. Can we run a quick diff on the output of the generated files before and after this change? There should be no change. I normally wouldn't ask for a diff, but there's enough changes in here that a diff is a good sanity check.

@scotttjob those generated css and js files are gitignored, by seeing the diff are you talking about temporarily adding those files to the index and running diff on them (then removing them from index)? In this case would it make sense to include this step (in a shell script) as part of running npm run bootstrap just to confirm the diffs are what we expected?

I don't think we have to go that far, I was thinking just a manual diff. So if you swapped branches back to master, ran the build command, copied the files out, swapped branches, ran the command again, then manually compared. So a one-off screenshot. :)

Picked a few files

Amaaaaaaazing! That's exactly the kind the reassurance I was looking for. Great work Edi!