-
Notifications
You must be signed in to change notification settings - Fork 6
feat: Add property for preserving css variables in token resolution #128
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
base: main
Are you sure you want to change the base?
Conversation
8768a62 to
d1b1682
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #128 +/- ##
==========================================
+ Coverage 97.43% 98.49% +1.05%
==========================================
Files 45 46 +1
Lines 2186 2326 +140
Branches 417 451 +34
==========================================
+ Hits 2130 2291 +161
+ Misses 56 35 -21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
d1b1682 to
7890c73
Compare
src/shared/theme/utils.ts
Outdated
| return defaults; | ||
| } | ||
|
|
||
| export function generateReferenceTokenName(colorName: string, step: string): string { |
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.
Perhaps generateReferenceColorTokenName? Or have an argument specifically for "type"? Your tech doc seems to allow for other kinds of reference tokens like border radiuses or spacing, but this PR assumes only color in this file.
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're right on both parts. It will eventually support more than color, but this PR only focuses on adding color. I made them more generic though since we'd have to do that eventually anyway.
feat: Add property for preserving css variables in token resolution chore: Enable useCssVars for testing chore: Add useCssVars support to runtime theming refactor: Make reference helpers more generic than just colors for future
Description of changes:
Adds a property to the internal theme builder that uses CSS variables instead of resolved values for better runtime theme swapping. This is part 2 of a series for theming improvements.
In an attempt to not generate duplicated CSS, the current theming setup only overrides tokens that have a different resolved value. That works ok for now, where you can only theme a handful of tokens. But with the introduction of reference tokens, essentially every token can be themed (300+), and would have a different resolved value. Rather than regenerating all 300+ tokens for the wholistic theming that reference tokens provide, this flag makes it so that only the reference tokens (~30) need to be sent, and the rest can be cascade via native CSS variable inheritance.
Next up:
Previous improvements:
[ac9YAbqsd7e6]
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.