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

Add CSS variable transformer for sd #58

Merged
merged 7 commits into from
Apr 4, 2024

Conversation

douglaszaltron
Copy link
Contributor

#56

@lukasoppermann
Copy link
Owner

Hey @douglaszaltron,

I somehow figured this would be part of a formatter?
Why do you favor a transformer?

It has the benefit of flexibility but would need to be applied in correct order. Also did you test it with nested tokens. We must make sure to avoid issues where the referenced token is already transformed to a css value (this could be done using the original values I think).

Somehow it feels more fitting for me to have this done in the formatter, I guess in this function. But I am open to being convinced that transformers are better.

@douglaszaltron
Copy link
Contributor Author

douglaszaltron commented Mar 21, 2024

Hey @douglaszaltron,

I somehow figured this would be part of a formatter? Why do you favor a transformer?

It has the benefit of flexibility but would need to be applied in correct order. Also did you test it with nested tokens. We must make sure to avoid issues where the referenced token is already transformed to a css value (this could be done using the original values I think).

Somehow it feels more fitting for me to have this done in the formatter, I guess in this function. But I am open to being convinced that transformers are better.

At first, I thought about creating it as a formater, but as a transformer, I can apply the value modification for both JavaScript/* and JSON/*. It would extend more easily. I did realize about the correct order, that the value mutations need to be done beforehand.

transforms: [
    "name/cti/kebab",
    "shadow/css",
    "fontFamily/css",
    "font/css",
    "variables/css",
],

transformer: (token: StyleDictionary.TransformedToken, options: StyleDictionary.Options) => {
const cssVarPrefix = getCssVarPrefix(options);
const varPrefix = cssVarPrefix ? `${cssVarPrefix}-` : ''
return `var(--${varPrefix}${token.name})`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe it's better to add the value as fallback.

var(--${varPrefix}${token.name}, token.value)

Copy link
Owner

Choose a reason for hiding this comment

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

Maybe this can be an option? e.g. valueFallback ?

@douglaszaltron
Copy link
Contributor Author

@lukasoppermann I made the adjustments, take a look.

@lukasoppermann
Copy link
Owner

Looks great, I'll have to test it and will merge it afterwards.

@lukasoppermann lukasoppermann merged commit 8b4ec69 into lukasoppermann:main Apr 4, 2024
1 check failed
@lukasoppermann
Copy link
Owner

lukasoppermann commented Apr 4, 2024

Hey @douglaszaltron, sorry, I just noted that I completely misread this PR.

So what this does is it replaces the value of a token with it's own name as a css variable.
To me that makes no sense. I thought you were targeting the references.

E.g. danger.$value: {base.red} would be turned into danger: var(--base-red).

So do you want to target the reference or are you really looking for replacing a variables value with it's own name.

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.

None yet

2 participants