-
Notifications
You must be signed in to change notification settings - Fork 0
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
fix(Theme): resolve alias tokens to ref token values #1013
Conversation
Storybook for this build: https://ds.equisoft.io/pr-1013/ |
Webapp for this build: https://ds.equisoft.io/pr-1013/webapp/ |
} | ||
|
||
if (isAliasToken(token)) { | ||
const aliasToken = customizedTheme.alias[token]; | ||
const aliasToken = resolvedTheme.alias[token]; |
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.
J'ai changé un peu l'algo pour pas toujours resolve les tokens et utiliser ceux qui ont déjà été resolved à la place. Ça va raccourcir un peu le nombre de boucles quand des alias réfèrent d'autres alias.
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!
En plus avec le snapshot ça permet d'être plus solide dans la validation de l'algo!
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.
Nice! Ça ne devrait pas causer de problème avec theme-directory, sauf que les alias vont être resolved maintenant dans les thèmes aussi. Je ne pense pas qu'ils soient utilisés tels quels mais faudra bientôt réfléchir à un système de version 😅
Les alias tokens n'étaient pas utilisables en dehors du DS, alors ça rendait l'utilisation des couleurs difficile dans les apps. En gros le snapshot montre les changements sur les alias tokens en output.