Adds selector ‘strategy’ plugin option

[chasegiunta]

Addresses #11 , #20 , #22

This intentionally keeps the code pretty dumb. Though, rather than using a ternary for every single selector, I opted to pass the selector string options to a simple function to determine whether to use .form- classes or not. This keeps it somewhat cleaner.

To Test

If you'd like to do a simple test, paste in the kitchen sink html code here and toggle the useFormClasses boolean option (and remove altogether).

  plugins: [
    require("@tailwindcss/forms")({
      useFormClasses: false,
    }),
  ],

The kitchen sink demo HTML is essentially duplicated. The first top set, being the original untouched kitchen sink, should work as intended without the option present, or set to false. (The bottom set will be broken except for the form controls that will rely on the .form- class & the element or selector — you can ignore)

If useFormClasses is set to true, the entire top original kitchen sink set should be entirely broken and the bottom set, which are all styled with .form- classes, should be styled correctly.

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/tailwindlabs/tailwindcss-forms/Bbhe61MTGiJPBTXGL3uQys4bzTVm
✅ Preview: https://tailwindcss-forms-git-fork-chasegiunta-master-tailwindlabs.vercel.app

[spencerpauly]

Can we get this merged in? It's actually super critical or else tailwindforms will break react-select and other UI kits.

[RobbieTheWagner]

@spencerpauly unfortunately, there has been no response from anyone who has permissions to this repo. It seems this is unmaintained by the Tailwind team. I've tried reaching out via their Discord as well, and received no response.

[adamwathan]

It's not unmaintained it's just that we can only prioritize so many things at a time. Still planning to merge something like this but it's not as simple as clicking a button, need to make sure we are happy with the actual API. I'm literally about to have a baby today, spent the last 5 weeks working 15 hours a day to build a new engine for the framework, and the rest of the team are trying to fulfill on promises to Tailwind UI customers for adding React + Vue support within the next two weeks.

We'll get to this soon but we are a finite number of people and there's only so much we can do at once.

[RobbieTheWagner]

Sure thing, totally understand @adamwathan. Congrats on the baby and thanks for all the work you do on Tailwind ❤️ . If you're open to adding some maintainers to help out, I think a few of us here would be glad to lend a helping hand.

[adamwathan]

Went to clone this down locally to make some changes but looks like there are conflicts for some reason and not sure what the real source of them is. Any chance you can rebase this on master and force push so I can clone again @chasegiunta ?

I have about 30 min to work on this right now but I don't want to just implement it over again and have you lose the commit credit 😕 but I really can't make sense of the diff and we still don't have tests for this project so it's a bit risky, I might be a moron but it looks like some things like border colors are different between this and master in certain situations for example, or maybe I am too dumb to make out the real diff, haha.

[adamwathan]

If you're open to adding some maintainers to help out, I think a few of us here would be glad to lend a helping hand.

Thanks @rwwagner90 — the biggest issue is honestly just trusting other people to make the same API decisions as me, it takes a lot of time to build that, so if I still have to be the one to hit the merge button on public API stuff, I'm still the bottleneck.

For example in this PR (which is awesome btw, thanks @chasegiunta) there are a handful of things I already want to change in regards to naming, so I have to review it and either make the changes myself or make suggestions. This PR has been open since January and today is the first time I have been able to make the time to go over the whole thing in detail and even form an opinion on it, so I just dunno how adding maintainers fixes that bottleneck unfortunately 😕

[chasegiunta]

@adamwathan if you're restrained for time, go have your baby and we can pick this up at a later date. I'm also in the middle of meetings right now and can't help at the current moment 😅

I think the main API decision is the naming of useFormClasses. It does look like there's new conflicts of today so understandable. Also very understandable that you can't just press a button and trust it's done how you're comfortable. I'll try to elaborate on my changes here possibly later today.

[adamwathan]

Turns out I had some local commits that actually made it into a real release that hadn't even been pushed to GitHub, hah. I have started reworking things a bit but didn't get to finish. If you want to make these changes to the existing PR I am good to merge:

• Configure using a strategy option rather than useFormClasses, where possible values are base and class

  plugins: [
   require("@tailwindcss/forms")({
     strategy: 'class',
   }),
  ],

• Use these for the class names:

  Base	Class
  [type='text']	form-input
  [type='email']	form-input
  [type='url']	form-input
  [type='password']	form-input
  [type='number']	form-input
  [type='date']	form-input
  [type='datetime-local']	form-input
  [type='month']	form-input
  [type='search']	form-input
  [type='tel']	form-input
  [type='time']	form-input
  [type='week']	form-input
  [multiple]	form-multiselect
  textarea	form-textarea
  select	form-select
  [type='checkbox']	form-checkbox
  [type='radio']	form-radio
  [type='file']	form-file

Open to suggestions if anyone thinks there are problems with this but this is where I was leaning 👍🏻

[chasegiunta]

@adamwathan Good deal. On it.

[chasegiunta]

@adamwathan

• Updated to use strategy as the option name, with class & base options (still defaults to base).
• Updated to use .form-multiselect & .form-file classes (I believe those were the only two that were missing from your list)
• I refactored this to be a bit more maintainable. The whole swap() method I was doing felt a bit dirty and wasn't sitting right with me. Instead, I'm throwing all the rules into a large object, looping over the object and doing addBase() that way. Much cleaner (IMO) and didn't seem to have any detrimental effect on style ordering or performance.

for (const rule of rules) {
  addBase({
    [rule[strategy]]: rule.styles,
  })
}

• Attempted to update the README, but writing is hard 😅 . Feel free to trash those changes.

Easiest way to test is to copy this modified kitchen code which applies the correct form- classes and then toggle the strategy options in your config.

@spencerpauly @rwwagner90 It would be great if you both could swing a test.

[adamwathan]

Merged and released in v0.3.0, thanks @chasegiunta!

[ghost]

This broke the JIT for me 😟
Works fine with plugin disabled.

Repo on my page.

[Ratko-Solaja]

This broke the JIT for me 😟
Works fine with plugin disabled.

Repo on my page.

It is broken in Laravel Mix as well. In order to fix it, I had to declare strategy: 'base'.