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

Figma variables integration #41

Merged
merged 16 commits into from
Aug 9, 2024
Merged

Conversation

juliocastrodev
Copy link
Collaborator

@juliocastrodev juliocastrodev commented Jul 12, 2024

INFO: There is currently another PR in preparation to have a PR without font change, so that the impact on the components is better visible in chromatic.

This PR includes the minimum necessary code to migrate to the new Figma variables export and also theming and viewport support.

The new export (design-tokens.tokens.json) includes the theming variables inside the semantic-tokens field (right now only dark and light) and the view ports variables inside the constraints field (right now only desktop and mobile). The rest are considered "independent" variables.

A custom style dictionary formatter (cssFigmaVariables.cjs) has been implemented to parse this information and organize it in a convenient way. And also to rename some of the output css variable names.

The returning string of the formatter is what is going to be written to the resulting design-tokens.css file. If you test it using this branch, you would get 5 "blocks" of css variables:

  • :root ---> css variables that are independent of a theme or viewport
  • .light ---> css variables for the light theme
  • .dark ---> css variables for the dark theme
  • @media (max-width: 480px) ---> css variables for the mobile viewport
  • @media (min-width: 481px) ---> css variables for the desktop viewport

The way the script works is by iterating through the information of design-tokens.tokens.json, but the version parsed by style dictionary (see docs). And we store each variable to the corresponding category.

In the end we just do some string manipulation with this data structure to generate the 5 blocks of css variables mentioned before.

The rest of the changes are the minimum necessary to not to break the existing components. The support to change the theme in story book is also added but components are not migrated to support the new dark theme. This will be implemented in a different PR.

Copy link
Member

@r3dDoX r3dDoX left a comment

Choose a reason for hiding this comment

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

Nice work! Would be nice, if all the style changes (like single vs. double quotes) would not be in the PR. Could you make sure that the whole reformatting is not in this PR so we can focus on what's actually changing?

src: url(https://cdn.jsdelivr.net/fontsource/fonts/open-sans:vf@latest/latin-wght-normal.woff2) format('woff2-variations');
unicode-range: U+0000-00FF,U+0131,U+0152-0153,U+02BB-02BC,U+02C6,U+02DA,U+02DC,U+0300-0301,U+0303-0304,U+0308-0309,U+0323,U+0329,U+2000-206F,U+2074,U+20AC,U+2122,U+2191,U+2193,U+2212,U+2215,U+FEFF,U+FFFD;
font-weight: 100 900;
src: url(https://cdn.jsdelivr.net/fontsource/fonts/inter:vf@latest/latin-wght-normal.woff2)
Copy link
Member

Choose a reason for hiding this comment

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

Interesting that the name is latin-wght-normal. this is the variable font though, right?

Copy link
Member

@culas culas Jul 15, 2024

Choose a reason for hiding this comment

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

yes, the vf stands for "variable font" afaict, I have that same change locally on my machine from when I showed it off at a presentation :D

Copy link
Member

Choose a reason for hiding this comment

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

Weird :)

viewPorts: "constraints",
};

// TODO: extract the break points from figma
Copy link
Member

Choose a reason for hiding this comment

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

could you do this TODO before we merge this PR? Otherwise I might open a new issue in the project tool and link it in this TODO that it does not get forgotten

Copy link
Collaborator

Choose a reason for hiding this comment

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

since julio is not working anymore, I will quickly open a new ticket for this, so it is not forgotten

@culas
Copy link
Member

culas commented Jul 15, 2024

Not sure whether I'm a fan of the new additional nesting in the font styles with "body" and "heading". Makes the variables longer and doesn't add any helpful information. I'll take it up with UX, we can keep it like this just so we get this PR done.

@r3dDoX
Copy link
Member

r3dDoX commented Aug 7, 2024

Where do we stand with this PR atm? I see that there is a lot of visual font changes, but that seems to be alright since we change the font face. Is there anything blocking still?

@r3dDoX
Copy link
Member

r3dDoX commented Aug 7, 2024

I just saw the way we include the font-faces. I solved this for my customer a little different and I could change this here too. The way we did it is reference the font as src: url('the-font.woff2').
This leads to vite not resolving this url but leaving it to the bundler that uses the library to bundle this font as it sees fit. The only additional change that this requires is in the storybook vite config we would have to make sure, that those fonts are loaded from /the-font.woff2. But no CSS changes required. That would make this a little easier. If you agree I'd implement this solution here too.

…les-integration

# Conflicts:
#	package-lock.json
@lukecore lukecore force-pushed the features/figma-variables-integration branch from 23fa44a to 26e4635 Compare August 9, 2024 12:34
@culas
Copy link
Member

culas commented Aug 9, 2024

I just saw the way we include the font-faces. I solved this for my customer a little different and I could change this here too. The way we did it is reference the font as src: url('the-font.woff2'). This leads to vite not resolving this url but leaving it to the bundler that uses the library to bundle this font as it sees fit. The only additional change that this requires is in the storybook vite config we would have to make sure, that those fonts are loaded from /the-font.woff2. But no CSS changes required. That would make this a little easier. If you agree I'd implement this solution here too.

Yeah go ahead with it, sounds good to me.

@lukecore lukecore merged commit 65db2f5 into main Aug 9, 2024
3 checks passed
@lukecore lukecore deleted the features/figma-variables-integration branch August 9, 2024 13:36
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.

6 participants