Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace custom REST API types with @figma/rest-api-spec types #19

Merged
merged 3 commits into from
Feb 22, 2024

Conversation

james04321
Copy link
Collaborator

@james04321 james04321 commented Feb 16, 2024

Now that we have official Typescript types for the REST API at @figma/rest-api-spec, use those types instead of the custom types we previously wrote.

@james04321 james04321 marked this pull request as ready for review February 21, 2024 23:44
@james04321 james04321 requested a review from a team February 21, 2024 23:44
Copy link

@jefflee-figma jefflee-figma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing

@james04321 james04321 merged commit e881303 into main Feb 22, 2024
1 check passed
@james04321 james04321 deleted the james-yang/rest-api-spec-types branch February 22, 2024 02:07
pwolfert added a commit to CMSgov/design-system that referenced this pull request May 29, 2024
pwolfert added a commit to CMSgov/design-system that referenced this pull request May 30, 2024
…rce (#2952)

* This script can just be about exporting CSS/Sass now

After the Figma changes, we have a different way of storing tokens, and they're all in one folder. We haven't needed the multiple input directories for a while, and we don't really have a big reason to run the command multiple times for different outputs either. This command has become specific to outputting what our other packages will consume, while we have a separate set of scripts for syncing with Figma.

* Move these into the same folder

* Move the new token types and token-reading utils because they are more broadly useful

We will now need them in the CSS/Sass exporters

* Move some testing files and add some new tests

* In the flattened token files, keep the notation internally consistent

Let the Figma scripts do the conversion from `.` to `/`, but let our other exporters expect the alias notation to match the flattened keys

* Starting to write the new exporter for CSS vars

* Make it easier to test

by breaking it into functions that are easier to test and by only converting the files it finds rather than requiring all theme files to be present

* Now have the basic structure of the new exporter

Still need to resolve values more intelligently and generate variable names in a way that matches the old ones and conveys the correct hierarchy

* Structure of this script starting to take shape

* Add variable naming logic

* Tests for var naming

* New alias resolving logic and a bunch of comments

* Update other tests

* We can put both CSS and SCSS in the same file (and finish it)

* More snapshot tests, but now I see that layout is empty

The test data needs some layout tokens, but it gets complicated, because spacers are currently defined as plain numbers in Figma, whereas we'll probably want the JSON `$type` to be `dimension`. We need to intelligently convert the values between our JSON storage and Figma

* Add fiddly logic for converting from and to Figma's generic number type

Once we do our first down-sync from Figma, we could possibly start storing this translation information as meta-data inside the JSON tokens. It wouldn't get uploaded to Figma, but it could be manually maintained in our repository. Being able to store information that doesn't go to Figma, however, would require that we merge incoming data with our local JSON files instead of the current overwriting method.

Actually, if we're not storing that info in Figma, it's already in our tokens...why would I have to tell a dimension token that it's a dimension? If we don't need to be able to save the JSON based on only the information stored in Figma, then this isn't a problem at all. Maybe after the first down-sync we just need to update the down-sync operation to be a merge, and we look at the local (repository) token's `$type` and `$value` properties before converting from our Figma `NUMBER` variable. If `$type` is `dimension` then look at the unit of `$value` to determine how to translate from Figma.

* Turn that long commit message into comments so we can address later

* Get rid of extra quotes in CSS output and add support for percentages

This is so fiddly. I hate it.

* Update tokens package README

* Move test mock files into a less conspicuous and distracting place

* This list hierarchy got lost

* Fix minor TS errors

* Replace our old tokens with some test ones synced from Figma

We don't have the full set of tokens in Figma yet because we're in the process of trying to transfer the library file from one account to another, but we've got all the `System` variables, and I manually created a few variables in the `Theme` collection.

* Don't need these theme TS files anymore either

* Fix `"{spacer.4}px"` as a `$value` in the JSON

If a variable is an alias, we don't need to send it through our logic for converting pixel values to other units

* Add checks in the unit tests for that functionality and fix it again

My previous solution wasn't perfect because it didn't account for inheriting the token type from the aliased variable

* Round `ex` units

* More type refinement

* Create an example .env file

* Sync full set of variables from Figma

* Add duration conversion support. Pushing to Figma works!!

* WIP: Prompt and store decisions about ambiguous number types from Figma

* WIP: Add guessing back in. It works!

I now that I think about it again, I don't really think it's necessary to store the number type information in the `$extensions` when we can later infer from the `$type` and the converted `$value` string that it wrote, which should contain the unit. The only weird one would be `0` values that are also `dimensions`, but I say we always pretend those are pixel values.

* Use context clues from value and type instead of storing metadata

* Update unit tests and make sure we can get types for Figma from aliases

Now that we don't duplicate the type information on alias tokens, we have to resolve the aliases until we get to a real token with a type in order to send that information to Figma.

* Don't need these types anymore

* Add support for font weight tokens and update Figma

* Temporarily revert var printing and add some snapshot tests that will fail

They will fail because I've manually created the snapshots by copying the old CSS files so we can see the changes from this branch

* Whoops, I had used a different sort method to create the original snapshots

* Lowercase some variables in Figma

* Remove duplicate pagination variable in Figma

* Delete some more unused Figma variables

* Add some comments

* Use new official Figma API types package

Following the example of figma/variables-github-action-example#19

* Some more doc comments

* Add a comment about the `ex` scale factor
pwolfert added a commit to CMSgov/design-system that referenced this pull request Jun 27, 2024
* [WNMGDS-2746][WNMGDS-2776] Update token build scripts to use JSON source (#2952)

* This script can just be about exporting CSS/Sass now

After the Figma changes, we have a different way of storing tokens, and they're all in one folder. We haven't needed the multiple input directories for a while, and we don't really have a big reason to run the command multiple times for different outputs either. This command has become specific to outputting what our other packages will consume, while we have a separate set of scripts for syncing with Figma.

* Move these into the same folder

* Move the new token types and token-reading utils because they are more broadly useful

We will now need them in the CSS/Sass exporters

* Move some testing files and add some new tests

* In the flattened token files, keep the notation internally consistent

Let the Figma scripts do the conversion from `.` to `/`, but let our other exporters expect the alias notation to match the flattened keys

* Starting to write the new exporter for CSS vars

* Make it easier to test

by breaking it into functions that are easier to test and by only converting the files it finds rather than requiring all theme files to be present

* Now have the basic structure of the new exporter

Still need to resolve values more intelligently and generate variable names in a way that matches the old ones and conveys the correct hierarchy

* Structure of this script starting to take shape

* Add variable naming logic

* Tests for var naming

* New alias resolving logic and a bunch of comments

* Update other tests

* We can put both CSS and SCSS in the same file (and finish it)

* More snapshot tests, but now I see that layout is empty

The test data needs some layout tokens, but it gets complicated, because spacers are currently defined as plain numbers in Figma, whereas we'll probably want the JSON `$type` to be `dimension`. We need to intelligently convert the values between our JSON storage and Figma

* Add fiddly logic for converting from and to Figma's generic number type

Once we do our first down-sync from Figma, we could possibly start storing this translation information as meta-data inside the JSON tokens. It wouldn't get uploaded to Figma, but it could be manually maintained in our repository. Being able to store information that doesn't go to Figma, however, would require that we merge incoming data with our local JSON files instead of the current overwriting method.

Actually, if we're not storing that info in Figma, it's already in our tokens...why would I have to tell a dimension token that it's a dimension? If we don't need to be able to save the JSON based on only the information stored in Figma, then this isn't a problem at all. Maybe after the first down-sync we just need to update the down-sync operation to be a merge, and we look at the local (repository) token's `$type` and `$value` properties before converting from our Figma `NUMBER` variable. If `$type` is `dimension` then look at the unit of `$value` to determine how to translate from Figma.

* Turn that long commit message into comments so we can address later

* Get rid of extra quotes in CSS output and add support for percentages

This is so fiddly. I hate it.

* Update tokens package README

* Move test mock files into a less conspicuous and distracting place

* This list hierarchy got lost

* Fix minor TS errors

* Replace our old tokens with some test ones synced from Figma

We don't have the full set of tokens in Figma yet because we're in the process of trying to transfer the library file from one account to another, but we've got all the `System` variables, and I manually created a few variables in the `Theme` collection.

* Don't need these theme TS files anymore either

* Fix `"{spacer.4}px"` as a `$value` in the JSON

If a variable is an alias, we don't need to send it through our logic for converting pixel values to other units

* Add checks in the unit tests for that functionality and fix it again

My previous solution wasn't perfect because it didn't account for inheriting the token type from the aliased variable

* Round `ex` units

* More type refinement

* Create an example .env file

* Sync full set of variables from Figma

* Add duration conversion support. Pushing to Figma works!!

* WIP: Prompt and store decisions about ambiguous number types from Figma

* WIP: Add guessing back in. It works!

I now that I think about it again, I don't really think it's necessary to store the number type information in the `$extensions` when we can later infer from the `$type` and the converted `$value` string that it wrote, which should contain the unit. The only weird one would be `0` values that are also `dimensions`, but I say we always pretend those are pixel values.

* Use context clues from value and type instead of storing metadata

* Update unit tests and make sure we can get types for Figma from aliases

Now that we don't duplicate the type information on alias tokens, we have to resolve the aliases until we get to a real token with a type in order to send that information to Figma.

* Don't need these types anymore

* Add support for font weight tokens and update Figma

* Temporarily revert var printing and add some snapshot tests that will fail

They will fail because I've manually created the snapshots by copying the old CSS files so we can see the changes from this branch

* Whoops, I had used a different sort method to create the original snapshots

* Lowercase some variables in Figma

* Remove duplicate pagination variable in Figma

* Delete some more unused Figma variables

* Add some comments

* Use new official Figma API types package

Following the example of figma/variables-github-action-example#19

* Some more doc comments

* Add a comment about the `ex` scale factor

* [WNMGDS-2786] Update doc site code to use new JSON tokens (#3098)

* Resolve the TypeScript errors by refactoring to use new token system

However, I still need to deal with Gatsby build errors. There's one that says the `tokens.ts` file imports `fs`, which is a node package and is a no-no, so I might need to shuffle some functions around to different files

* Move the function with fs calls to another module

* It's working!

Restored functionality to the following pages:
- `/foundation/system-color-tokens/`
- `/foundation/theme-colors/`
- `/migration-guides/v7-standardizing-theme-colors/`
- Component page CSS variable lists
- Color utility pages

* Fix minor regression causing there to only be two columns of color ramps

* [WNMGDS-2785] Fix SCSS token generation and consumption (#3100)

* Create folders if they don't exist

* Updated references to layout files and made sure we had a default one

* Update scss filenames in sass-to-css migration examples

* Fix TS error in docs

* [WNMGDS-2785] Add full support for font-family tokens (#3101)

* Add full support for font family tokens/variables

I'm including some failed type defs for making Token a union

* Get rid of commented typedef code

* Manually add font fallbacks to token files

* [NO-TICKET] Stop ignoring token files for Prettier cleanup (#3102)

Stop ignoring token files for Prettier cleanup

* [WNMGDS-2785] Fix first batch of Figma/CSS differences (#3107)

* Put the system tokens first in the css/scss output

In Sass, I'm pretty sure order of declaration matters, and we now reference system variables in theme variables. It also just feels more natural to have them up front if they're to be referenced later

* BREAKING: Use new button font-weight tokens instead of `--button__font-weight`

* Update refs to updated font variables and add other global inherited font variables

* Remove the need for `--inset__border-width` in healthcare theme

* Trying to get Choice to look right again

Filled in the missing gap token and fixed the medicare color

There's more to do on Choice, but I'm checking what I've got in for now

* Add a `choice/label/top-margin` variable to replace `choice__translateY`

* Fix choice border widths in medicare

* Remove `yarn prettier` package script because it conflicts with the base `yarn prettier` command

and causes an infinite loop when you try to run it. I've been using it to preview the token JSON changes before the pre-commit hook runs.

* Stop using the `--shadow-base` variable

* Add `label` vars to Figma and add support for `text-transform`

* Delete unused system variables from Figma

* Fix syncing `textTransform` to Figma

* Remove this from the diff

* BREAKING: Get rid of the `--hint__font-size` variable only used in medicare

This will mean a visual change for medicare where the hint text size now matches the error text size and defaults to 1rem

* Rename new `--choice-label__top-margin` variable to `--choice-label__margin-top`

* Rename the label offset variable

* Fix bad typography variable references and disuse of font weight vars

* Fix incorrect token values for medicare heading sizes

* Fix 2xl headings not getting their font weight from tokens

* BREAKING: Add missing theme-level "normal" and "bold" font weight

Removing `--font-weight-semibold` and `--font-weight-light` in the process. We don't use them in our other tokens. The semibold one is used in a utility class only. I've hard-coded the value into the utility class until it's deemed time to deprecate that utility class.

* User error inputting the updated medicare 5xl value in Figma

* More fixes to medicare font sizes. Now all the size-related VRTs pass

The only remaining VRT failures for typography are Link changes

* Forgot to rename this in the CSS

* BREAKING: Deprecate semibold utility class and remove semibold Open Sans from includes

* Update VRTs for the medicare hint text, whose size we changed

* Add deprecation styles to removed `ds-u-font-family--semibold`

* [WNMGDS-2785] Implement max-width for form fields (#3110)

* Get the new field-max-width system variables

* Update references in CSS to point to new `field-max-width*` variables

* Update the css files in docs, too, so there are fewer changes in the diff later

* Added currency-icon variables to Figma to support masked fields

* Update VRTs that are related to the medicare hint size adjustment

* Other nominal VRT changes for medicare text fields

* Update Autocomplete VRTs affected by medicare hint size change

* Update Dropdown VRTs affected by medicare changes

* Fix SingleInputDateField padding in medicare

* Fix MultiInputDateField showing separators in medicare

* Update date field VRT refs affected by medicare hint size change

* Restore unique disabled field width for medicare

* [WNMGDS-2785] Updated Accordion with Figma changes (#3111)

Update accordion CSS and fix visual regressions

Removed the need to have a healthcare-only `--accordion-icon__size` variable.

Started using the new border radius variables

* [WNMGDS-2785] Update USA Banner with Figma changes (#3112)

* Use new gutter variable

I'm already thinking I'm going to reverse this and just delete it from Figma. You'll see

* Revert "Use new gutter variable"

This reverts commit ea0c2a9.

* WIP: Well, I thought this was a good idea

Turns out that medicare does use a different value for the max-width, and that's `unset`. Having pixel values and `unset` doesn't work in Figma, so I think a better option there is to make it a boolean. However, that boolean still has to play nice with the variable we told people in the docs to use to set the max width (`--usa-banner__max-width`).

* Use a boolean value for the theme data but allow apps to override the max width

* Fix medicare link color and get rid of two more variables

Updated the VRT ref images too

* [WNMGDS-2785] Fix border radii for cmsgov badges (#3115)

Revert border radii for cmsgov badges so they match production

* [WNMGDS-2785] Update more medicare hint VRTs (#3116)

* Update more VRT refs related to medicare hint changes

* Update dropdown interaction test refs

* Update other interaction-test refs that changed by medicare hint updates

* Update more medicare month picker VRT refs

Don't know how these were missed before

* [NO-TICKET] Fix broken doc-site build (#3120)

Don't print any of the boolean values on the doc site

Those boolean tokens are mostly internal and are not robust enough to set to different values. They shouldn't be shown in the docs as configuration options. This fixes build because I made the CSS value function to throw an error if we try to get the value of a boolean token.

* [WNMGDS-2785] Fix reference to system-only token that doesn't exist in theme (#3117)

Fix reference to system-only token that doesn't exist in theme

* [WNMGDS-2785] Remove refs to z-index variables (#3118)

Remove references to the underused and now removed z-index variables

* [WNMGDS-2785] Fix filter chips by removing variable with bad value (#3119)

Fix filter chips by removing variable with bad value

This didn't actually have the right values in Figma, but we don't need it anyway. It's the same value for each theme, so just hard-code it into the CSS. It wasn't being used in Figma

...in fact, the production component doesn't even match the Figma designs.

* [NO-TICKET] Delete Sketch plugin (#3097)

Delete the Sketch plugin because we won't be using it anymore

* [WNMGDS-2797] Update links with Figma changes (#3123)

* Set values for link underline offset and thickness that will pass the existing VRTs

* Change ghost button styles to match the underline offset and thickness of links

* Update VRTs for other places where ghost buttons are used

* [WNMGDS-2797] Remove `ds-u-sans-serif` and `ds-u-serif` utility classes (#3131)

Remove the `ds-u-sans-serif` and `ds-u-serif` classes and related variables

We do not have theme-level fonts that universally fill those categories, and we don't find that these are useful as utility classes. Font families should be inherited from a theme's components or base style and not fiddled with directly.

* [NO-TICKET] Fix regression in max-width of USA banner on doc site (#3132)

Fix regression in max-width of USA banner on doc site

For some reason a value of `initial` seems to trigger the `var()` fallback mechanism instead of just removing the max width. I guess that's how custom properties work?

* [WNMGDS-2797] Remove the unused `--autocomplete*` and `--dropdown*` variables (#3133)

Remove the unused `--autocomplete*` and `--dropdown*` variables

* [WNMGDS-2797] Update CSS var snapshots (#3134)

* Update CSS var snapshots from all our previous changes

* Update doc site static theme files

* Fix medicare token value regression for button border

* Update VRT ref images for the astro example

I can't see any difference in the diffs. No pixels are highlighted in the diff image, and I can't see any differences between the expected and actual with my naked eye. I suspect it has to do with link underlines and potential sub-pixel differences.

* [WNMGDS-2797] Use variable references in theme files (#3135)

Define theme variables with references to other theme variables

This allows for another layer of customizability by product teams and shows our intention for different component variable values, though the cost is that these theme files take up a bit more space.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants