Skip to content

Make it possible to override ring and border color on plugin level #44

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

Closed
wants to merge 1 commit into from

Conversation

mdix
Copy link

@mdix mdix commented Feb 17, 2021

Hey there,

first of all: Thanks for creating this plugin (and tailwindcss in the first place). It saved us a lot of time!

We needed to customize the ringColor and borderColor of the form elements and so I thought I give it a try. I'm pretty sure that it's not mergeable right away (and that there might be a discussion whether you want to allow customization that way or not), but I'd be very happy if you would drop me some lines on what I'd need to change or how we can deal with this. We really like to use this plugin, but we also don't want to override the styles everywhere. :-)

And I just noticed that it might make sense to add some tests. Let me know if you're interested in merging this at all, then I'll add some tests.

Thanks and best
Marc

@vercel
Copy link

vercel bot commented Feb 17, 2021

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/oftg0hfnz
✅ Preview: https://tailwindcss-forms-git-fork-mdix-master.tailwindlabs.vercel.app

@adamwathan
Copy link
Member

Hey thanks for the PR! Ultimately we decided for this project to encourage people to customize the defaults by simply writing CSS, rather than try to come up with some complex configuration API.

We did that on the old custom-forms plugin and it was just a nightmare to deal with and to teach :/ So instead I would recommend just writing straight up CSS to deal with this:

@layer base {
  [type="text"],
  [type="email"],
  /* ... */ {
    border-color: pink;
  }

  [type="text"]:focus,
  [type="email"]:focus,
  /* ... */ {
    --tw-ring-color: purple;
  }
}

Definitely need to document this some time, but here's a video that covers it that we shared recently on our YouTube channel:

https://youtu.be/pONeWAzDsQg?t=868

Thanks again though I really do appreciate the PR! ❤️

@adamwathan adamwathan closed this Mar 24, 2021
@mdix
Copy link
Author

mdix commented Mar 24, 2021

Hello adam. Thanks! That was the most polite way of saying NO to a merge request I ever experienced. Thanks for the helpful links and the very good explanation. 👍

@adamwathan
Copy link
Member

I am from Canada 🇨🇦 😅 Thanks for understanding and thanks again, hope to see more contributions in the future!

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.

2 participants