patternfly / design-tokens

MIT License
1 stars 8 forks source link

Some font-weight tokens are not being exported #75

Closed mcoker closed 2 months ago

mcoker commented 2 months ago

@lboehling @evwilkin, these two tokens are defined in figma, but are not being parsed correctly to create tokens

And looks like it has something to do with the bold properties being nested in an object that also has a value property (because we have the tokens --pf-t--global--font--weight--body and --pf-t--global--font--weight--heading). @srambach said we've added --default to tokens in situations like this, but if that isn't preferred here, I wonder if we could update the parser to be able to generate tokens in an object structure like this?

@lboehling do you think we should update --pf-t--global--font--weight--[body/heading] to --pf-t--global--font--weight--[body/heading]--default? If so, that should be all that's needed to resolve this issue.

FWIW here's the important part of the current json export

    "font": {
      [...]
      "weight": {
        "body": {
          "description": "Use to define the default weight for body text",
          "type": "number",
          "value": "{global.font.weight.100}",
          "bold": {
            "description": "Use to define the bold weight for body text, often used to field labels or to add emphasis.",
            "type": "number",
            "value": "{global.font.weight.200}"
          }
        },
        "heading": {
          "description": "Use to define the default weight for heading text",
          "type": "number",
          "value": "{global.font.weight.300}",
          "bold": {
            "description": "Use to define the bold weight for heading text, often used to add emphasis.",
            "type": "number",
            "value": "{global.font.weight.400}"
          }
        }
      },
evwilkin commented 2 months ago

@mcoker without diving into the code, updating the naming structure to stay consistent seems like the easier path. We may be able to update the parser to be dynamic in how far it parses, but that feels a little fragile if there's an alternate naming option that would match what's used elsewhere.

That said, I'd like to see @lboehling 's feedback on your suggestion and we can go from there 👍