search

busbud / tailwind-buddy

Tailwind Buddy: Your Friendly Helper for Composing Tailwind Classes 🎨
https://busbud.github.io/tailwind-buddy/
12 stars 1 forks source link

Some questions #19

Closed zAlweNy26 closed 2 months ago

zAlweNy26 commented 2 months ago

Hi, first of all good job for the package, it is really nice how you structured it. I just have some questions regarding the decisions made with it.

Thanks in advance for any further response.

flozero commented 2 months ago

Hey @zAlweNy26, hope you're doing great! Thank you for your message. I'm really happy to see that you're enjoying this package. I really tried hard specially for the performance aspect of it in my latest releases.

Regarding your first point: I'm using defaultVariants to inject base classes and use it quite a lot to check responsiveness, compound variants. Removing it would require some thought, as it made sense in my initial implementation.

When you say it "complains," could you provide more details? For example, you could have a default or empty variant with no classes as the default. I understand this might not be ideal, but I believe it would resolve your issue.

As for your second point: That's actually a fair observation. It should be "classes" or "class" to be more framework-agnostic in its naming.

This is probably something I simply didn't pay attention to. Could you tell me the context in which you're using it so I can better understand?

I'll spend some time soon thinking about your first point in particular. The second point seems to be a no-brainer, but it will come with significant breaking changes for us. So, I may just introduce both "class" and "className" as a temporary solution.

Also would you mind give me a bit more how you are using the library like the project, what features are you using the most. And if you find any annoying part

zAlweNy26 commented 2 months ago

Hi, thank you for the response.

When I said "it complains" I meant that typescript complains of course, because the variants are all required. I didn't think about that solution of making an empty variant to fix my problem, thanks! If you prefer to leave it as it is now, I think it's a fair solution for me to have an empty variant even if it's not ideal.

For the second point, I'm using the package in a Nuxt project (so it's Vue) and seeing that className in my component props seems a little weird, I could have bypassed it with an Omit utility type, but I thought it would have been better to point out it in an issue. If it's a breaking change that could cause you some very annoying issues, it's not a problem for me to bypass it.

Anyway at the moment I'm using the library for my project's components to be able to make them more generic and reusable as possible, and I'm using a lot the "slots" logic that I really like. The annoying part for me was to register the safelist for tailwind, as I had to pass a lot of components' variants, but I don't think there is another way to accomplish it, so I'm ok with it.

flozero commented 2 months ago

Actually for the generateSafeList if you check at https://github.com/busbud/tailwind-buddy/issues/20.

Adding the class should be totally possible, I believe I will look into it as soon as I have some times.

So you are using the responsive props ? If you don't you don't need to pass the components inside the safeList. Its only for that purpose we need to add them.

the tailwind main maintainer answer to my point for tailwind v4 and they seems to want to deprecate the options safeList. The way of doing this will need to change. I am not sure how to integrate a "build" step to generate a css file instead of this array but I will think about it and this will actually be back compatible with this if we implement it.

Thank you for your feedbacks thats really helpful !

zAlweNy26 commented 2 months ago

Oh wow I didn't know that. Thanks for the clarification!

flozero commented 2 months ago

thanks to you that means the doc is probably not clear enough :)

flozero commented 2 months ago

hey @zAlweNy26 I am closing this issue as I have created three issues that I think have covered what your questions were:

Don't hesitate to tell me if you think something is missing. Will work on them a bit later with the priority on the class property