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

feat(tailwind): Add font-size aliases for text utilities #5

Merged

Conversation

fkapsahili
Copy link
Contributor

@fkapsahili fkapsahili commented Oct 9, 2024

This PR introduces new font-size aliases (sm, base, lg, xl) to the existing text-<number> utilities to the Tailwind configuration. The current config generates classes like text-text-1, which can seem unintuitive and redundant. Additionally, the existing setup prevents the use of default Tailwind utilities such as text-sm and text-base.

Prerequisites

  • Use a meaningful title for the pull request
  • Test the changes you've made by adding or editing tests

Content

  • Added font-size aliases for text utilities in the Tailwind configuration
  • Preserved backward compatibility with existing text utilities

Tests

  • Added tests to ensure that the new font-size aliases are correctly generated and mapped to their respective text-<number> parts
  • Verified backward compatibility by testing the existence of text-<number> utilities

@GianlucaGuarini
Copy link
Contributor

GianlucaGuarini commented Oct 9, 2024

Thank you for your contribution. Wouldn't be easier to add directly these aliases as tokens here?

For example

"xl": { "value": "{typography.text-1}" }

@fkapsahili
Copy link
Contributor Author

Thank you for your contribution. Wouldn't be easier to add directly these aliases as tokens here?

For example

"xl": { "value": "{typography.text-1}" }

@GianlucaGuarini Thanks for looking into it!

Yes, that could make sense. I adjusted the config to generate e.g. text-1 instead of text-text-1 for the Tailwind class names, and it will produce additional utilities like text-base, text-sm, text-lg, and text-xl for overriding the built-in Tailwind class names.

Also, updating the token definitions in text.json will not only generate tokens for Tailwind classes, but also create tokens for other platforms, since it affects the overall token generation; but I guess that's fine? 🙂

@GianlucaGuarini
Copy link
Contributor

@domirs can you please have a look at it as well?

Yes, that could make sense. I adjusted the config to generate e.g. text-1 instead of text-text-1 for the Tailwind class names, and it will produce additional utilities like text-base, text-sm, text-lg, and text-xl for overriding the built-in Tailwind class names.

Isn't this technically a breaking change? Probably we need to bump a new major release for it

formats/tailwind/get-font-size.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@fkapsahili
Copy link
Contributor Author

fkapsahili commented Oct 10, 2024

@GianlucaGuarini I’ve updated the config to only remove the text- prefix for built-in Tailwind class names (text-sm, text-base, etc.). This preserves existing generated tokens like text-text-1 etc, but I think that's fine for now.

I think considering reducing redundancy in other text- tokens in the future would be useful, but for now, this keeps things stable and improves Tailwind usage.

@GianlucaGuarini GianlucaGuarini merged commit 88b7533 into axa-ch:main Oct 10, 2024
4 checks passed
@GianlucaGuarini
Copy link
Contributor

Thank you @fkapsahili for your contribution. We will inform our design team that these tokens might need to be renamed in future

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