onesine / react-tailwindcss-datepicker

Modern date range picker component for React using Tailwind 3 and dayjs. Alternative to Litepie Datepicker.
https://react-tailwindcss-datepicker.vercel.app/
MIT License
550 stars 169 forks source link

Default inputClassName not applying, and overrding inputClassName is not working #64

Closed nate-dash closed 1 year ago

nate-dash commented 1 year ago

I'm using the latest version of the tool, with react 18.2, I'm not sure what would cause this.

My config file looks correct to me.

/** @type {import('tailwindcss').Config} */
module.exports = {
  content: [
    './src/**/*.{js,jsx,ts,tsx}',
    "./node_modules/react-tailwindcss-datepicker/dist/index.esm.js",
  ],
  theme: {
    extend: {
      backgroundColor: '#f1f5f9'
    },
  },
  plugins: [
    'tailwindcss',
    '@tailwindcss/forms'
  ],
}

Happy to provide more info, just not sure what else to give right now.

giero commented 1 year ago

I have the same problem and I kind of know where it comes from.

In my project I make a lot of customizable components, where I almost always pass className from the outside. The problem shows when a component has default tailwind classes and they need to be changed/modified. Many (next to each other) classes don't want to work as intended. Like here: https://github.com/onesine/react-tailwindcss-datepicker/blob/master/src/components/Input.tsx#L63

However, I found this article: https://dev.to/diogoko/overriding-tailwind-classes-in-react-4po3 and started using twMerge - great library for merging tailwind classes :)

The above code could look like this:

return twMerge("relative transition-all duration-300 py-2.5 pl-4 pr-14 w-full border-gray-300 dark:bg-slate-800 dark:text-white/80 dark:border-slate-600 rounded -lg tracking-wide font-light text-sm placeholder-gray-400 bg-white focus:ring disabled:opacity-40 disabled:cursor-not-allowed", border, ring, classNameOverload);

This should fix the problem (here and everywhere where classes would be overwritten).

giero commented 1 year ago

This may help: https://github.com/onesine/react-tailwindcss-datepicker/pull/68 :)

nate-dash commented 1 year ago

This may help: #68 :)

Thank you, how do I install from your repo instead of the default? I can't seem to get it to work with

npm i https://github.com/giero/react-tailwindcss-datepicker.git
giero commented 1 year ago

I only created this repo for pull request - you have to wait for merge to @onesine's project :)

onesine commented 1 year ago

Hi @nate-dash & @giero 👋 Thank you for using our package.

Sorry for the delay. Thank you for your comments. This is indeed an issue that needs to be resolved. As it is currently handled, it is not interesting. I saw your MR @giero, the problem is that we will have to install tailwind-merge and I think that may increase the size of the library. If we can find a solution that does not affect the size of the package, that would be great. It would be nice not to have our users downloading a library that weighs a lot. A small size should be a strong point for react-tailwindcss-datepicker.

I could be wrong @giero 😅. Let me know if I'm confused about anything.

PRs are always welcome 🎉.

giero commented 1 year ago

Well, I think the tradeoff is worth it. twMerge size is 21.1kB (gzipped 6.7kB), not too big in my opinion (bundlephobia), but I'm still looking for another solution just in case :)

JefteCaro commented 1 year ago

PR #79 This should fix the issue without additional dependencies into the package.

using string

<Datepicker
  inputClassName="dark:bg-white"
/>

using functions twMerge

import { twMerge } from "tailwind-merge";
...
<Datepicker
  inputClassName={twMerge('dark:bg-white', 'text-2xl')}
/>

assuming you have custom classNames function

<Datepicker
  inputClassName={classNames('dark:bg-white', 'dark:text-gray-900')}
/>
giero commented 1 year ago

I don't think this it will resolve anything. For example

<Datepicker
  inputClassName={twMerge('dark:bg-white', 'text-2xl')}
/>

this will just generate string and it will be still concatenated to existing className like here https://github.com/onesine/react-tailwindcss-datepicker/blob/master/src/components/Input.tsx#L63

This should work like this:

<Datepicker
  inputClassName={(predefinedInputClassName) => twMerge(predefinedInputClassName, 'dark:bg-white', 'dark:text-gray-900')}
/>

I closed https://github.com/onesine/react-tailwindcss-datepicker/pull/68 because you gave me an idea how to do this different way. I will provide another PR :)

btw @JefteCaro you left twMerge in https://github.com/onesine/react-tailwindcss-datepicker/blob/master/pages/index.js :)

karimhossenbux commented 1 year ago

What about if we pass classes into inputClassName, to not use the default ones?

Makes more sense to customize the input as we want. No extra lib, just what the user wants.

giero commented 1 year ago

That makes sense - DatePicker would have its own default classes but they could be overwritten entirely by dedicated class names - not individually or partially. This will solve the problem with merging classes.

davecarlson commented 1 year ago

This has now broken things on our side. We were using the "old behavior" to add existing classes to the pre-defined ones.

How does one do this now ?

inputClassName="font-medium text-gray-600"
giero commented 1 year ago

You can still do this, but using inputClassName as a function:

inputClassName={(className) => `${className} font-medium text-gray-600`)}

But this may not work as intended - that's why I use https://www.npmjs.com/package/tailwind-merge for this, and then

inputClassName={(className) => twMerge(className, "font-medium text-gray-600")}
davecarlson commented 1 year ago

ah perfecto! I missed the part where the existing classNames were exposed.