mikaelvesavuori / figmagic

Figmagic is the missing piece between DevOps and design: Generate design tokens, export graphics, and extract design token-driven React components from your Figma documents.
https://docs.figmagic.com
MIT License
808 stars 71 forks source link

Handle "auto" line-height values from Figma? #82

Closed flo-sch closed 3 years ago

flo-sch commented 3 years ago

Describe the bug

I just faced a bug with line-height tokens in Figma, I have some ideas and will be happy to contribute with a PR, but just wanted your view with it first (We can also discuss it live if you prefer)

To resume, our designer defines text styles that we re-use in the Tokens Figmagic board to extract the tokens. (So we should not adjust those values manually). One of those values is "Auto" line-height, which Figma API treats a bit specifically (see payload below) and results in Figmagic throwing an error during the line height tokens processing, causing the entire Line-Height tokens file to not be generated at all.

(The error is catched internally and I did not get a feedback from the CLI, I just noticed thanks to Git that a file was to be deleted after syncing the tokens)

The error happens in both v3 and v4, since the resulting payload has no lineHeightPercentFontSize property:

{ 
  "fontFamily": "Poppins",
  "fontPostScriptName": "Poppins-SemiBold", 
  "fontWeight": 600, 
  "paragraphSpacing": 20, 
  "textCase": "UPPER", 
  "textAutoResize": "WIDTH_AND_HEIGHT", 
  "fontSize": 12, 
  "textAlignHorizontal": "LEFT", 
  "textAlignVertical": "TOP", 
  "letterSpacing": 0.72, 
  "lineHeightPx": 14.0625, 
  "lineHeightPercent": 100, 
  "lineHeightUnit": "INTRINSIC_%"
}

To Reproduce Steps to reproduce the behavior:

  1. Create a Figmagic document with a line-height defined as "Auto"
  2. Run figmagic CLI to sync tokens from that document
  3. No lineHeight tokens file is generated

Expected behavior

I could think of 2 possible behaviours:

  1. If it is possible, support "Auto" as a value (... potentially converting it to some units? Or use the CSS normal keyword? Haven't thought that through yet: unclear what Figma means by "Auto")
  2. If not possible, then show a warning about that specific token, but still generate the lineHeight file without it with the other tokens that are valid
flo-sch commented 3 years ago

Seems like Figma uses "Auto" as a default value for "100%": https://help.figma.com/hc/en-us/articles/360040449893-Line-height-behavior

We will convert the Line height value in any existing Styles to the new format. For example: Any Styles that used a Font's default Line height (i.e. 100% on the old rendering) will be set to "Auto".

We could potentially use the lineHeightPercent property instead? (That would mean resulting to 1.00 as a value in such case)

mikaelvesavuori commented 3 years ago

Hi @flo-sch, I'm happy to take a solution proposal/pull request. The only real input I have at the moment is that I want to ensure that behavior is not altered; something I think will happen if using lineHeightPercent instead.

Go ahead and give it a stab if you can!

mikaelvesavuori commented 3 years ago

I see that the reason I stopped using lineHeightPercent is because it's deprecated.

Refer to https://www.figma.com/developers/api

flo-sch commented 3 years ago

Ah yup, good one, might as well not use that one then! Then I'll have a look at a conversion instead, will submit a PR sometimes this week hopefully!

Can't get proper specifications from Figma regarding that non-standard "Auto" value, seems akward :/