Skip to content
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

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

Closed
nate-dash opened this issue Feb 6, 2023 · 13 comments

Comments

@nate-dash
Copy link

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
Copy link
Contributor

giero commented Feb 8, 2023

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
Copy link
Contributor

giero commented Feb 9, 2023

This may help: #68 :)

@nate-dash
Copy link
Author

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
Copy link
Contributor

giero commented Feb 13, 2023

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

@onesine
Copy link
Owner

onesine commented Feb 18, 2023

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
Copy link
Contributor

giero commented Mar 2, 2023

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 added a commit to JefteCaro/react-tailwindcss-datepicker that referenced this issue Mar 2, 2023
@JefteCaro
Copy link
Collaborator

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
Copy link
Contributor

giero commented Mar 2, 2023

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 #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 :)

giero pushed a commit to giero/react-tailwindcss-datepicker that referenced this issue Mar 2, 2023
@karimhossenbux
Copy link

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
Copy link
Contributor

giero commented Mar 27, 2023

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.

giero pushed a commit to giero/react-tailwindcss-datepicker that referenced this issue Mar 27, 2023
@onesine onesine closed this as completed Apr 3, 2023
@davecarlson
Copy link

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
Copy link
Contributor

giero commented Jun 16, 2023

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
Copy link

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

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

No branches or pull requests

6 participants