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

added better support for variable alias when transforming tokens using style dictionary #261

Conversation

ryanzec
Copy link
Contributor

@ryanzec ryanzec commented Aug 6, 2023

This makes changes so that the outputted design token can properly be transformed with style dictionary when using aliases for a variable value.

This PR adds in functionality so that after all variables are processed, it does a second pass on the variables and if the variable is an alias, it will basically create a copy of the variable and update both the name and the values of the variable to account for the modes of the alias.

Previous to these changes, this is an example of a alias variables design token output:

Screen Shot 2023-08-06 at 11 32 23 AM
Screen Shot 2023-08-06 at 11 32 13 AM

With the changes, the output is now:

Screen Shot 2023-08-06 at 11 34 19 AM
Screen Shot 2023-08-06 at 11 34 11 AM
Screen Shot 2023-08-06 at 11 34 05 AM

Adding for the mode in the values is needed to make sure when transforming with style dictionary, the reference can be found. Adding the mode in the name is needed to make sure there are not duplicate variables names.

This seems like the cleanest way to add this functionality but let me know if there is any feedback.

This addresses #260

@coveralls
Copy link

coveralls commented Aug 14, 2023

Pull Request Test Coverage Report for Build 5956451766

  • 3 of 17 (17.65%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.9%) to 66.461%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/utilities/getVariables.ts 3 17 17.65%
Files with Coverage Reduction New Missed Lines %
src/utilities/getVariables.ts 1 20.29%
Totals Coverage Status
Change from base Build 5858043726: -0.9%
Covered Lines: 495
Relevant Lines: 731

💛 - Coveralls

@lukasoppermann
Copy link
Owner

Hey, I think this is certainly one option, as I am also not sure exporting multiple files works. (Haven't had time to look into it).

I would like this feature to be togglable via the UI. I guess we could tie this the Reference mode in variables option?

If it is one, than it would reference modes in both the file structure on references, if it is off, it will be removed from both. WDYT?

@ryanzec
Copy link
Contributor Author

ryanzec commented Aug 17, 2023

I think that makes sense since when mode reference is on and aliasing a variable with multiple modes does not work right now when transforming anyways.

@lukasoppermann I believe I have updated the PR based on your feedback but if I did not understand it correctly, let me know.

@ryanzec ryanzec changed the title added proper support for variable alias when transforming tokens using style dictionary added better support for variable alias when transforming tokens using style dictionary Aug 17, 2023
@ryanzec
Copy link
Contributor Author

ryanzec commented Aug 23, 2023

@lukasoppermann Is there anything else this PR needs?

@lukasoppermann lukasoppermann merged commit ea25341 into lukasoppermann:main Aug 23, 2023
1 check passed
@lukasoppermann
Copy link
Owner

Hey @ryanzec there are some problems with this. It works fine when disabled, but when enabled it has a wrong setup in the file. I will investigate this further. Sorry for the long wait.

@lukasoppermann
Copy link
Owner

Ahh, @ryanzec so the issue is, that you are replacing the reference and the variable name with the same mode. however the referenced variable is different and may come from a different mode and collection.

lukasoppermann added a commit that referenced this pull request Sep 2, 2023
@lukasoppermann
Copy link
Owner

Hey @ryanzec I had to rollback this PR. I didn't find the time to fix it and its blocking the merge of other changes.

I tried to fix it in here: https://github.com/lukasoppermann/design-tokens/tree/fixForModesExport but did not succeed yet.

Can you send a new PR with this and try to fix it.

Use cases it needs to cover:

  • reference without modes
  • normal tokens
  • reference with multiple modes
  • modeReference set to false

@hayawata3626
Copy link
Contributor

@lukasoppermann
Which items are currently not being met?
If I have a time, I will debug.

reference without modes
normal tokens
reference with multiple modes
modeReference set to false

@hayawata3626
Copy link
Contributor

hayawata3626 commented Sep 9, 2023

@lukasoppermann
I tried to fix the problem and pushed the codes.
If you have a time, please check.(If it is correct, I will create a PR.)
https://github.com/hayawata3626/design-tokens/tree/fix-get-variables

Details I added the codes.
262f990

lukasoppermann added a commit that referenced this pull request Sep 12, 2023
* added better support for variable alias when transforming tokens using style dictionary (#261)

* added proper support for variable alias when transforming tokens using style dictionary

* process alias modes when modeReference is enabled

* Update src/utilities/getVariables.ts

* Update src/utilities/getVariables.ts

---------

Co-authored-by: Lukas Oppermann <lukasoppermann@github.com>

* add a button solely for saving the settings.

* add functions auto-save of export settings

* remove unnecessary code

* change naming of button

* reBuild the code

---------

Co-authored-by: Ryan Zec <basire@gmail.com>
Co-authored-by: Lukas Oppermann <lukasoppermann@github.com>
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.

4 participants