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

[Bug]: missed lowercasing of font style "alone" in font weight tokens #267

Open
nhoizey opened this issue Mar 12, 2024 · 1 comment
Open
Assignees
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@nhoizey
Copy link
Contributor

nhoizey commented Mar 12, 2024

What happened?

When I generate Sass variables based on font weight tokens coming from Figma, I get for example this:

$font-weight-400: 400;
$font-weight-500: 500;
$font-weight-700: 700;
$font-weight-400-italic: Italic;
$font-weight-500-italic: 500 italic;
$font-weight-700-italic: 700 italic;

The two last variables have italic as lowercase, but $font-weight-400-italic has a capital in Italic.

I think it's because there is no actual weight number in front of this font style.

The regex here allows it: https://github.com/tokens-studio/sd-transforms/blob/main/src/transformFontWeights.ts#L35-L38

But the conversion of font style here is "inside" the one for the font weight value, so if there is no font weight value, the font style is not converted: https://github.com/tokens-studio/sd-transforms/blob/main/src/transformFontWeights.ts#L55-L57

I guess the if (match.groups.style) should be outside the if (match?.groups?.weight).

I can make a PR if this is the correct fix.

Reproduction

No response

Expected output

No response

Version

0.14.4

@nhoizey nhoizey added bug Something isn't working triage To be reproduced as bug labels Mar 12, 2024
@jorenbroekema jorenbroekema removed the triage To be reproduced as bug label Mar 12, 2024
@jorenbroekema
Copy link
Member

jorenbroekema commented Mar 12, 2024

Yep that seems like a bug indeed. You could pull that second if statement out, but make sure to account for a "space" separator if both the weight and style match groups are present. Maybe put the match group results in an array and do a matches.map(part => part.toLowerCase()).join(" ") or something. PR welcome :)!

@jorenbroekema jorenbroekema added good first issue Good for newcomers help wanted Extra attention is needed labels May 10, 2024
@jorenbroekema jorenbroekema removed their assignment Aug 12, 2024
florin01hyma added a commit that referenced this issue Nov 4, 2024
jorenbroekema pushed a commit that referenced this issue Nov 4, 2024
jorenbroekema pushed a commit that referenced this issue Nov 5, 2024
* font weight bug fix (#267)

* fix: font weight bug fix (#267)

* PR review comments addressed

* New PR review comments addressed

* redundant file deleted
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants