-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Theme: Update Figma plugin to convert additional properties #73407
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
Conversation
jameskoster
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
b7c4da6 to
79291f9
Compare
Primitives are no longer included in the output, as their values are instead inlined
1a77e22 to
9dc5ed8
Compare
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
We no longer export primitive tokens, so no point in creating this grouping
|
@jameskoster I pushed a few more improvements:
Example before / after:
|
Nice! |
| 'warning', | ||
| 'caution', | ||
| 'error', | ||
| ] ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In other places we call these "tones" as "color ramps". Could we use the same name here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In other places we call these "tones" as "color ramps". Could we use the same name here?
Hm, I think there might be value in differentiating. With how we're using these in color theming, I'd say that we use the seed value for a given tone to produce a color ramp, but not that tone and color ramp are the same (tone being a singular term describing a mood or meaning, and color ramps being the set of color values within that domain). Additionally, we produce color ramps from seeds which aren't associated with tones, like "bg" and "primary".
| "description": "Background color for interactive elements with neutral tone and strong emphasis." | ||
| }, | ||
| "Color/Semantic/Background/Neutral/bgInt-neutral-strong-active": { | ||
| "Color/Background/Neutral/bgInteractive-strong-active": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The token names are now a combination of camel case bgInteractive and kebab case -strong-active. Could we unify that on the better-looking bg-interactive-strong-active? That's also the convention that the associated CSS variables use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think this could be a good opportunity to revisit these variable renamings. I'm not positive what the original motivation was (perhaps Figma truncation?), but it creates some complexities to try to manage them. It'd be a lot easier to just keep them uniform.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originally we shortened things like "bgInteractive" to "bgInt" because the variable selection UI is quite narrow, but this doesn't look too bad:
I think there's more value in uniformity.
Speaking of which, I noticed that how we've structured the tokens in Figma means the code snippets in "Dev mode" are pretty wonky:
Maybe we should try restructuring to ensure alignment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, that's an interesting point on the Dev Mode snippets. I'm not sure we can get those totally correct without completely upending how we've structured the colors into folders.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean in Figma? Is there any reason not to do that, if we want to? I think proper alignment would be quite helpful. Probably fine to tackle separately though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah we could create a folder for those targets like surface / interactive 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated to simplify / align how the exported tokens retain details of the original tokens in e748ace. This feels a lot simpler, still creating folders for "type" ("color", "dimension"), "property" ("bg", "fg", "padding", etc.), and "target" ("surface", "interactive", etc.).
This required a few corresponding changes in Automattic/figma-ds-token-manager#6 which are now pushed there (af26c93).
Net result is that Dev Mode snippets are correct:
The variables panel and corresponding display in Figma fields aren't quite as ergonomic, but I think it's acceptable. This is aligned with what you shared above @jameskoster .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the color picking experience in Figma is fine, I really appreciate the alignment.
The only question I have now is whether it's worth using Collections. The duplicated collection name + group seems a bit strange, and the fact that each collection will only ever include a single root-level group may also be an indicator that collections aren't necessary...
With that said, a single collection would limit what we can do with modes which might come back to bite us later. Maybe it's fine to go with separate collections if we change the collection name to differentiate from the group (e.g. WPDS Color rather than wpds-color). WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the main thing to keep in mind with collections is ensuring we have room to support distinct modes. Aligning this to the token file groupings is an imperfect but reasonably good fit to be safe. I think this also works well for future compatibility as far as how Figma "Import" mechanic works (i.e. import a single DTCG JSON file to a single collection).
I think capitalizing the collection could be fine enough to implement if it'd help differentiate the collection and top-level group. I might explore if this is something we could add using extensions so we don't have to maintain a manual mapping or rely on some programmatic capitalization (can be error-prone, as mentioned elsewhere)
|
After having spent more time with Figma's recently launched DTCG import functionality, I feel like we can move away entirely from maintaining this Figma-specific plugin and use DTCG tokens directly, with some revisions on our end to fix up and restructure the tokens: |
|
Closing in favor of #73860 |



What?
This is meant to pair with accompanying changes in Automattic/figma-ds-token-manager#6 . This custom Figma plugin is intended to provide designers a way to easily consume design tokens in Figma documents. Word is that Figma will be supporting this natively in the near future, so a custom plugin may not be needed for very long.
Why?
Testing Instructions
Verify accuracy of changes to README.md and
figma.jsonoutputs.Ensure no local changes to commit after running
npm run --workspace @wordpress/theme build.