onesine / react-tailwindcss-select

Tailwind Select Component for React.js
https://demo-react-tailwindcss-select.vercel.app/
MIT License
183 stars 38 forks source link

tailwind-merge support #43

Open alcidesbsilvaneto opened 1 year ago

alcidesbsilvaneto commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.