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

Fix: fonts.css template doesn't correctly parse the style or weight options #1399

Merged
merged 2 commits into from
Dec 8, 2024

Conversation

LewisDaleUK
Copy link
Contributor

Modify the fonts.css.template logic, so that it correctly coerces the null values when font.style and font.weight are undefined.

The current logic is using null coersion (the ?? operator), which means that the value is only written when font.style is null - which of course then throws an error because it tries to read value from an undefined object.

Then, if font.style and font.weight are defined, the output is [object Object][object Object], because the actual statement is never reached.

Issue #, if available:

Description of changes:

Update the logic to use a ternary statement with empty strings as the fallbacks.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@LewisDaleUK LewisDaleUK requested a review from a team as a code owner November 28, 2024 14:04
Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-1399.d16eby4ekpss5y.amplifyapp.com

@jorenbroekema
Copy link
Collaborator

Hey, this looks fine by me, could you maybe add a test and run npx changeset locally to create a human-readable changeset (patch bump is fine)?

@LewisDaleUK
Copy link
Contributor Author

Hey, this looks fine by me, could you maybe add a test and run npx changeset locally to create a human-readable changeset (patch bump is fine)?

Added both now, let me know if they're okay/you need any more detail in them

jorenbroekema
jorenbroekema previously approved these changes Dec 3, 2024
Copy link
Collaborator

@jorenbroekema jorenbroekema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved test file (we're a bit inconsistent about folder paths in lib versus tests) and used snapshots so it's easier to update in the future e.g. when we migrate to using tabs in v5 in all our formats, approved!

@dbanksdesign dbanksdesign merged commit 7a661bb into amzn:main Dec 8, 2024
5 checks passed
tkgroot pushed a commit to tkgroot/style-dictionary that referenced this pull request Dec 9, 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 this pull request may close these issues.

3 participants