Skip to content

feat(@clayui/css): LPD-52388 Add *px spacer utilities to match Lexicon spacers #5995

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pat270
Copy link
Member

@pat270 pat270 commented Apr 3, 2025

https://liferay.atlassian.net/browse/LPD-52388

I debated whether we should try and namespace the spacers like:

    0: 0,
    0-5: $spacer * 0.125,
    1: $spacer * 0.25,
    2: $spacer * 0.5,
    2-5: $spacer * 0.75,
    3: $spacer,
    3-5: $spacer * 1.25,
    4: $spacer * 1.5,
    4-5: $spacer * 2.5,
    5: $spacer * 3,
    5-5: $spacer * 3.5,
    5-75: $spacer * 4,
    6: $spacer * 4.5,
    7: $spacer * 6,
    8: $spacer * 7.5,

but it didn't make sense to me. It's difficult to remember and inserting more sizes increases the confusion.

@matuzalemsteles
Copy link
Member

@pat270 I think having the hardcoded pixel in the prefix in this case now can get a bit confusing since we use a different standard, I think we can implement 0.5, 2.5 this is similar to what exists in taillwindcss for example to avoid breaking change. I think I prefer it this way because if you change the standard from a spacing-1 to 10px for example I don't need to change the entire codebase but if you have a spacing-2px you would need to change it to spacing-10px for example.

@pat270
Copy link
Member Author

pat270 commented Apr 8, 2025

@matuzalemsteles my opinion is that we should move everything to px in the future if we don't want to introduce breaking changes. If we want to follow the format we have now 0, 0.25, and 0.5, we will need to shift the scale which will break current implementation. There is also . in the class name which is invalid unless we escape it with a \.

Tailwind isn't limited to 0-###. They have px classes too, https://tailwindcss.com/docs/padding.

@matuzalemsteles
Copy link
Member

@pat270 Well, in theory, we would have to adjust the spacing values ​​since there is a standard and maybe we didn't follow it before, but since we added this a long time ago, it might break the look in some places that don't expect this change.

Yes, Tailwind doesn't limit it, but this is for a custom value and doesn't scale from p-1 to p-96. The p-px is just px1 or the developer customizes its value with p-[10px] but we can't do this in Clay because we don't have an engine to handle this at build time to read the files and generate the utils.

@pat270
Copy link
Member Author

pat270 commented Apr 8, 2025

Well, in theory, we would have to adjust the spacing values ​​since there is a standard and maybe we didn't follow it before, but since we added this a long time ago, it might break the look in some places that don't expect this change.

The prefix/suffix doesn't matter to me. I don't want to break layouts. We can use s0-5, 0-25s or whatever. The idea is the same, this pattern leaves room to scale up and in between.

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.

2 participants