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

Mode name are duplicated in referenced values #308

Closed
f-Keith opened this issue Jul 2, 2024 · 24 comments · Fixed by #309
Closed

Mode name are duplicated in referenced values #308

f-Keith opened this issue Jul 2, 2024 · 24 comments · Fixed by #309

Comments

@f-Keith
Copy link

f-Keith commented Jul 2, 2024

Hello,

Since the fix #300 , the name of the mode is now included in the value of the design token when exporting the Figma Variables. Is this a bug because the reference is now broken?

Example:
image (1)

The previous JSON export for background/Primary Red/Default would look like:

{
    "theme": {
        "light": {
            "background": {
                "primary red": {
                    "default": {
                      "type": "color",
                      "value": "{color.lifeblood red.500}"
                     }
                }
            }
        }
    }
}

However, it now exports with the additional light mode name in value

{
    "theme": {
        "light": {
            "background": {
                "primary red": {
                    "default": {
                      "type": "color",
                      "value": "{color.light.lifeblood red.500}"
                     }
                }
            }
        }
    }
}

We have a design token for a color (color/lifeblood red/500) that does not play a part of theming, but our library is now unable to reference it because it is now looking for the reference color/light/lifeblood red/500).

@MildTomato
Copy link

MildTomato commented Jul 2, 2024

I've got the same, I think I have the same issue.
Been comparing to old json outputs and it looks like new json exports have the mode name repeated in the value.

I cloned this repo, and ran a few older versions though, and it seems like something has maybe changed on Figma's side, since even the older versions seem broken.

@f-Keith
Copy link
Author

f-Keith commented Jul 2, 2024

@MildTomato , I took a quick look at the MR tagged in my earlier message, and the changes there seem to be the cause of the issue. I haven't dug deeper yet but I don't think the issue lies with Figma.

@lukasoppermann
Copy link
Owner

Hey, did you try to disable this setting?

Image

@MildTomato
Copy link

MildTomato commented Jul 2, 2024

re the screenshot above @lukasoppermann

It does work, but it removes the Figma Variable mode references, which we use 😅.

@f-Keith
Copy link
Author

f-Keith commented Jul 2, 2024

@lukasoppermann ,

Thanks for a speedy reply! I'll give it a try later today, if its all good on my side, I'll close this issue :)

@f-Keith f-Keith closed this as completed Jul 2, 2024
@f-Keith f-Keith reopened this Jul 2, 2024
@lukasoppermann
Copy link
Owner

Ann darn, okay so you want it only in the json, but not in the variable name?

I am not sure how the usage is, but we could add a checkbox to include it in the name.

Would anyone be up to sending a PR for this?

@MildTomato
Copy link

MildTomato commented Jul 2, 2024

Ann darn, okay so you want it only in the json, but not in the variable name?

Yea, I think that's right. I'm going to test just editing the JSON first to make sure that actually does fix things.

update - yep, that seems to be the fix.

@MildTomato
Copy link

MildTomato commented Jul 2, 2024

@lukasoppermann If I run v6.10.4 I'm guessing it should work as before right?

edit - I can have a look and see if I can post a PR, if I can get my head around how all this works.

@lukasoppermann
Copy link
Owner

edit - I can have a look and see if I can post a PR, if I can get my head around how all this works.

@MildTomato that would be great.

The settings file you need to add a checkbox to is GeneralSettings (after the highlighted code).

You also need to add the option, I'd name it modeInTokenName defaultSettings

I think (but I am not sure), you'd afterwards just change this line to set addMode to false if modeInTokenName is false.

@MildTomato
Copy link

@lukasoppermann nice one. well, that makes it easy 😄

@0m4r
Copy link
Collaborator

0m4r commented Jul 5, 2024

I have made a fix here - I (almost) blindly followed the instruction from @lukasoppermann two answers above
See #309

I hope this is good enough or at least a good base to solve the problem.

@nedzen
Copy link

nedzen commented Jul 8, 2024

I hope this gets fixed soon. It's quite annoying.

@jacekpuzio-kk
Copy link

jacekpuzio-kk commented Jul 19, 2024

This bug has corrupted my JSON file too, but the problem is more complex.

I have '01 Primitives' collection, where I have 2 columns: 'light-mode' and 'dark-mode', sample variable: brand/500.
Then I have '02 Aliases' collection, with the "Default" column, where I create tokens such as "color.bg.brand.default" that is linked to "brand/500" from '01 Primitives'.

Now, there are two issues. When creating a variable in '02 Aliases' e.g. "color.bg.brand.default",

  1. The plugin now adds "Default" column name from the '02 Aliases' collection to the string name e.g.
    value : {01 primitives.Default.color.brand.500}
    Since "Default" it's referring to the column in '02 Aliases', not '01 Primitives' I get errors that the reference/theme/collection cannot be found.

  2. Previously, from what I noticed, in the JSON file, it was referring incorrectly to 'dark-mode' instead. Not sure why. Shouldn't that be a raw token without any reference to the collection?
    value : {01 primitives.dark-mode.color.brand.500}.

Now the issue is, that even if my default collection (column) in '01 Primitives' is set in Figma to 'light-mode', in the JSON file it's referring by default to 'dark-mode' in both scenarios. If I turn the setting 'Reference mode in variables' off, which is also not a solution if you have more than 1 theme, it's still using the wrong, 'dark-mode' column for the source of HEX codes instead of 'light-mode'. I missed that issue previously, as my 'dark-mode' is WIP, it's just a duplicate of the 'light-mode' where I only edited grays to be more bluish.

Hope it gets fixed soon, as my whole setup is broken now :-( It would be also nice to have a setting in the Plugin section which is the default collection e.g. 'light mode' or 'dark mode' from '01 Primitives'.

@jacekpuzio-kk
Copy link

Any chances for a quick fix? 🥲

@0m4r
Copy link
Collaborator

0m4r commented Jul 23, 2024

@jacekpuzio-kk there is a PR open here with a possible fix and some discussion.
Feel free to try it out and provide your feedback!

@jacekpuzio-kk
Copy link

jacekpuzio-kk commented Jul 30, 2024

@0m4r yeah, but i'm looking for a updated plugin as a designer so that I could use it in Figma 😅

@TylerCousins
Copy link

TylerCousins commented Aug 9, 2024

I think it's a bigger issue in the JSON structure, as when it exports for example:

Figma Variables

1-Primitives

Name Value
Reds/20 FF9999
Reds/80 660000

2-Semantics

Name Light Dark
Error/Primary Reds/20 Reds/80

3-Components

Name Value
Alert/Text Error/Primary

It becomes:

  "1-primitives": {
    "reds": {
      "20": {
        "type": "color",
        "value": "#ff9999ff",
        "blendMode": "normal"
      },
      "80": {
        "type": "color",
        "value": "#660000ff",
        "blendMode": "normal"
      }
    }
  },
  "2-semantics": {
    "light": {
      "error": {
        "primary": {
          "type": "color",
          "value": "{1-primitives.light.reds.20}"
        }
      }
    },
    "dark": {
      "error": {
        "primary": {
          "type": "color",
          "value": "{1-primitives.dark.reds.80}"
        }
      }
    }
  },
  "3-components": {
    "alert": {
      "text": {
        "type": "color",
        "value": "{2-semantics.mode 1.error.primary}"
      }
    }
  }
}

Issues

  1. The mode from the current collection is put into the reference path for its value, so {2-semantics.light.error.primary} references {1-primitives.light.reds.20} when it should be referencing {1-primitives.reds.20}. The fix for this is easy, the mode from the current collection should just be removed. This also leads to collections that have no modes inserting "mode 1" into their variables' references, e.g. {3-components.alert.text} references {2-semantics.mode 1.error.primary}.
  2. The bigger issue is that {3-components.alert.text} should be referencing {2-semantics.error.primary}, but that will lead to a "Reference doesn't exist" error, as {2-semantics.error.primary} does not exist. This is as the current JSON structure treats the Variable Modes as Nested Object Properties, so from my understanding, referencing a variable without specifying a mode, e.g. {2-semantics.error.primary}, wouldn't work with this structure.

I'm not very familiar with JSON, and only started using Figma a month ago, so my understanding of Design Tokens comes from https://www.contentful.com/blog/design-token-system/ , which suggests a schema, where instead of the Variable Mode split taking place above the Variable Categories, i.e. (Collection.Mode.Category.Variable), Variables should have a Nested Object Property "ValuesByMode", with one reference for each Variable Mode, so perhaps:

"2-semantics": {
  "error": {
    "primary": {
      "type": "color",
        "valueByMode": {
          "light": "{1-primitives.reds.20}",
          "dark": "{1-primitives.reds.80}"
          }
        }
      }
    }

Would JSON be able to refer to just {2-semantics.error.primary}? I'm not sure how this would build the stylesheet and how then that is used in a page that wants to toggle between modes and have the components react accordingly. Sorry for the long comment and naivety.

@0m4r
Copy link
Collaborator

0m4r commented Aug 9, 2024

the "mode" is an issue also when setting up a figma variable that references another one from the same mode, but this must be worked around by setting up a different figma variable structure as it is merely impossible to detect if a reference belongs to the same mode as the variable being parsed.

@TylerCousins in your specific case, I would not set any mode in the primitives, as they are primitives, they should be intended to be used anywhere.

{
  "light": {
    "error": {
      "primary": {
        "type": "color",
        "value": "{1-primitives.reds.20}"
      }
    }
  },
  "dark": {
    "error": {
      "primary": {
        "type": "color",
        "value": "{1-primitives.reds.80}"
      }
    }
  }
}

@TylerCousins
Copy link

TylerCousins commented Aug 12, 2024

@0m4r thanks for the reply, Typo on my part, edited to fix that. I currently avoid referencing from the same mode, but Figma enabling that means some might use it and yes, that would conflict.

@0m4r
Copy link
Collaborator

0m4r commented Aug 12, 2024

@0m4r thanks for the reply, Typo on my part, edited to fix that. I currently avoid referencing from the same mode, but Figma enabling that means some might use it and yes, that would conflict.

yeah, I can see that. I am not sure if there is any information on the same mode reference that Figma can provide.
It would be worth reaching out to them, to understand more.

(I have got the same issue on the project I am working on, so, I feel you! - If you find a better solution do let me know :P)

@okaverin
Copy link

Could the whole "mode" feature be reverted and then reintroduced making sure that there's access to the old behavior? Our current flow has been broken for a few months because none of the options produces the previous result. This is quite unfortunate.

@jacekpuzio-kk
Copy link

jacekpuzio-kk commented Aug 22, 2024

@okaverin even older versions before the unfortunate update were incorrectly providing random HEX codes if you had more than 1 theme... i.e. your main theme is Light Mode, but in the JSON files tokens are referring to Dark Mode and using those HEX codes instead...

@0m4r
Copy link
Collaborator

0m4r commented Sep 30, 2024

So, I have just updated #309 with the possibility to configure to add a mode name to the token name, the token value, both or none of them.
The code also now resolves the alias within the same mode or collection of Figma variables.

I'd be really happy if someone would try it out and give feedback.

@0m4r
Copy link
Collaborator

0m4r commented Nov 18, 2024

🥳

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 a pull request may close this issue.

8 participants