lukasoppermann / design-tokens

🎨 Figma plugin to export design tokens to json in an amazon style dictionary compatible format.
https://www.figma.com/community/plugin/888356646278934516/Design-Tokens
MIT License
972 stars 135 forks source link

fix aliased variables being emmited twice #289

Closed MonsieurMan closed 5 months ago

MonsieurMan commented 1 year ago

Hi there, this is just a quick fix for a bug I had.

Aliased tokens with different modes were emitted twice (with two modes) as there were two place in the code where modes were taken into account.

This removes the loop in the processAliasModes fn, just keeping L98 which is already looping over modes.

I didn't tested it with more than two modes for now.


Thanks for the plugin, I'm doing research for implementing a multi-brand design system and I'll maybe use it, could contribute more if it is the case and you're open to it.

lukasoppermann commented 1 year ago

Hey @MonsieurMan this makes sense.

Could you explain a way to test this to me?

How to I reproduce the issue with double variables? What setup do I need to create this? Afterward I can test this.

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 6722612248


Changes Missing Coverage Covered Lines Changed/Added Lines %
src/utilities/getVariables.ts 1 6 16.67%
<!-- Total: 1 6 16.67% -->
Totals Coverage Status
Change from base Build 6708682924: 0.2%
Covered Lines: 496
Relevant Lines: 728

💛 - Coveralls
JackHowa commented 1 year ago

I'm having a similar issue. And I'm not sure if it's related to #293 where the last theme seems to be being the default mode for aliased tokens. I think I can write a new test file under tests/ folder

lukasoppermann commented 1 year ago

@JackHowa this would be great. All we need is a way to test this and than we can merge it.

JackHowa commented 10 months ago

@JackHowa this would be great. All we need is a way to test this and than we can merge it.

Cool, working on it now with src/utilities/getVariables.ts. Investigating what figma gives you in terms of data to generate something like this mode:


  "colors": {
    "themeA": {
      "palette": {
        "brand": {
          "primary": {
            "type": "color",
            "value": "#001944ff",
            "blendMode": "normal"
          }
        }
      },
      "surfaces": {
        "primary": {
          "default": {
            "type": "color",
            "value": "{colors.themeC.palette.brand.secondary}"
          }
        }
      }
    },
    "themeB": {
      "palette": {
        "brand": {
          "primary": {
            "type": "color",
            "value": "#001944ff",
            "blendMode": "normal"
          }
        }
      },
      "surfaces": {
        "primary": {
          "default": {
            "type": "color",
            "value": "{colors.themeC.palette.brand.secondary}"
          }
        }
      }
    },
    "themeC": {
      "palette": {
        "brand": {
          "primary": {
            "type": "color",
            "value": "#001944ff",
            "blendMode": "normal"
          }
        }
      },
      "surfaces": {
        "primary": {
          "default": {
            "type": "color",
            "value": "{colors.themeC.palette.brand.secondary}"
          }
        }
      }
    }
  }
JackHowa commented 10 months ago

This works as expected for me locally. I will work on trying to unit test extractTokens. But will involve mocking figma's api

JackHowa commented 10 months ago

@lukasoppermann @MonsieurMan thanks for your work on this and patience. I have added some tests in a separate pr off the default that shows the alias modes emitting two values to show https://github.com/lukasoppermann/design-tokens/pull/299/files#diff-18b59c301a30dbea8cef574b8e83d81e812fa126a021df0860ff518f3dd21035R17

I'm happy to make a pr off of this branch if that's helpful. Like I said before, this is working for our usecase! Thanks!

lukasoppermann commented 5 months ago

@JackHowa can you please resolve conflicts? I think you know better what is correct.

JackHowa commented 5 months ago

@JackHowa can you please resolve conflicts? I think you know better what is correct.

sorry, this isn't my pr. looks like my pr was just merged! https://github.com/lukasoppermann/design-tokens/pull/300 🥳 Thanks so much @lukasoppermann !!

lukasoppermann commented 5 months ago

@JackHowa sorry, I completely missed this the whole time. 🤦

JackHowa commented 5 months ago

@JackHowa sorry, I completely missed this the whole time. 🤦

hahah no worries at all. thanks for the quick replies. your plugin is a lifesaver for my org