Skip to content

tailwind-merge support #43

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

alcidesbsilvaneto
Copy link

@alcidesbsilvaneto alcidesbsilvaneto commented May 31, 2023

Now when the user provides classNames properties the lib will merge the classes provided by the user with the default ones, so we can customize some parts of the component without having to re-write all the default classes to keep other styles like effects, borders, spacing etc...

@thornyweb
Copy link

I think the twMerge arguments should be the other way around so that the users classNames override the ones set in the component. E.g.

            className={twMerge("px-2 py-2 cursor-not-allowed truncate text-gray-400 select-none", classNames?.listDisabledItem)}

Using the args as you have them now, if I specify px-4 in my props the px-2 in component will replace it.

Thanks for doing this though, it's a great addition to a super component.

@alcidesbsilvaneto
Copy link
Author

I think the twMerge arguments should be the other way around so that the users classNames override the ones set in the component. E.g.

            className={twMerge("px-2 py-2 cursor-not-allowed truncate text-gray-400 select-none", classNames?.listDisabledItem)}

Using the args as you have them now, if I specify px-4 in my props the px-2 in component will replace it.

Thanks for doing this though, it's a great addition to a super component.

Thanks for the feedback, it was a really stupid mistake, it's fixed now. Thanks again.

@onesine
Copy link
Owner

onesine commented Jun 2, 2023

Hi @alcidesbsilvaneto 👋

So sorry for the delay.

Tailwind merge is a great tool and it would be very useful to use it on the project. But, it will make another package to install and it will of course increase the size of react-tailwindcss-select. We try to install as few packages as possible.

However, I think we can expose the element classes so that people can use tailwind merge outside the library.

<Select
  value={animal}
  onChange={handleChange}
  options={options}
  classNames={{
     menuButton: ({ className }) => twMerge(className, 'bg-blue-500 text-white'),
     menu: ({ className }) => twMerge(className, 'py-2'),
     listItem: ({ className }) => twMerge(className, 'text-blue-500 hover:bg-blue-500 hover:text-white')
  }}
/>

With this, tailwind-merge won't be installed on the project and people will be able to merge classes.

If you can help us expose the classes of the elements that would be great 😀.

Thanks for this PR 👍

@anthonyespirat
Copy link

tw-merge could be very useful for clean code, especially with a lib call react-taiwindcss-**. And only 300ko (bundle) if I'm not wrong

However, exposing className could be a good alternative idea

@alcidesbsilvaneto
Copy link
Author

Hi there, @onesine , now I am the one who apologizes for the delay. Just opened the #47 and I hope it solves the merging problem.

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.

4 participants