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

resolve issue 222 (Suisse Int'l font family transform issue / special character transform) #223

Merged
merged 6 commits into from
Nov 16, 2023

Conversation

niborium
Copy link
Contributor

@niborium niborium commented Nov 15, 2023

fixes #222

Escape any occurences of ' automatically in the quoteWrapWhitespacedFont to resolve the issue with special character `.

escapeApostrophes is a new function that takes a string and replaces every apostrophe with an escaped apostrophe. It uses the replace method with a regular expression /'/g that matches all occurrences of an apostrophe and replaces them with ' (the double backslash is needed because backslash is an escape character in JavaScript strings).

In the quoteWrapWhitespacedFont (exisiting function), the input string is first passed to escapeApostrophes to ensure all apostrophes are properly escaped. Then, the rest of the function proceeds as before, but uses the escaped string for the checks and final return value.

This should resolve the issue with breaking CSS when the string contains apostrophes.

Copy link

changeset-bot bot commented Nov 15, 2023

🦋 Changeset detected

Latest commit: b15daba

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@tokens-studio/sd-transforms Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@jorenbroekema
Copy link
Member

jorenbroekema commented Nov 16, 2023

Hey, looks good in general but could you add a test case for this here https://github.com/tokens-studio/sd-transforms/blob/main/test/spec/css/transformFontFamilies.spec.ts ?

And you'll probably also need to run npm run format to ensure the code you added is formatted properly according to this repository's standards, or else the pipeline will fail on the linting step ;).

Oh and finally, npx changeset to add a changeset.

https://github.com/tokens-studio/sd-transforms/blob/main/CONTRIBUTING.md might be useful to give this a quick read, let me know if you have any questions! I'm happy to help out if there's anything that you struggle with

@niborium
Copy link
Contributor Author

niborium commented Nov 16, 2023

Hey, looks good in general but could you add a test case for this here https://github.com/tokens-studio/sd-transforms/blob/main/test/spec/css/transformFontFamilies.spec.ts ?

And you'll probably also need to run npm run format to ensure the code you added is formatted properly according to this repository's standards, or else the pipeline will fail on the linting step ;).

Oh and finally, npx changeset to add a changeset.

https://github.com/tokens-studio/sd-transforms/blob/main/CONTRIBUTING.md might be useful to give this a quick read, let me know if you have any questions! I'm happy to help out if there's anything that you struggle with

Updated the quoteWrapWhitespacedFont function, found an issue there. Explained:

In the updated quoteWrapWhitespacedFont function, the key change made was to ensure that the escapeApostrophes function is applied more judiciously. Previously, escapeApostrophes was used on the entire fontString, which could inadvertently escape apostrophes in already quoted font names. This led to incorrect output like \'Arial Black\' instead of the expected 'Arial Black'.

Trim and Check for Quotes: The function now trims the input fontString and checks if it is already quoted using the isAlreadyQuoted function.

Conditional Escape Application: The escapeApostrophes function is only applied if the font name is not already quoted. This prevents double escaping of apostrophes in font names that are already within quotes.

Intelligent Quoting: The function returns the font name wrapped in quotes only if it contains whitespace and is not already quoted. This ensures that multi-word font names are appropriately quoted, while avoiding redundant quotes around already quoted names.

In summary, the update improves the handling of font names by correctly escaping apostrophes only when necessary and smartly applying quotes to multi-word font names, ensuring accurate and clean CSS syntax.

+ Added test and runned test - both passed for transformFontFamilies.spec.ts (process font family + escape apostrophes)
+ Added changeset

See commits, hopefull i made everything as expected else let me know :)

@jorenbroekema jorenbroekema merged commit 2307b9d into tokens-studio:main Nov 16, 2023
2 checks passed
@niborium niborium deleted the issue-222 branch November 17, 2023 11:30
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.

[Bug]: Suisse Int'l font family transform issue / special character transform issue
2 participants