-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Do not force hsl and use tailwindcss theme colors whenever possible #185
Conversation
@emrosenf is attempting to deploy a commit to the shadcn-pro Team on Vercel. A member of the Team first needs to authorize it. |
exactly what i was trying to figure out. the numbers were too magical, but I assumed they all eventually come from slate. hope this merges in EDIT: @emrosenf looks like |
@jondot good catch. Looks like the Tailwind team is aware of the problem. That's a real shame. |
As a temporary solution, perhaps we could just add comments to
Benefits:
With comments like this, if someone wanted to switch to @shadcn would you be supportive of this? |
Make sense. It may not be immediately clear what colors are represented by these semantic values for those who are used to the built-in Tailwind palette, which could cause confusion. A great solution would be just to add comments in the docs for light theme, this would be great to understand what this color looks like in the Tailwind pallete. I think it would be enough. If a user wants to change colors they can replace them with their own HSL values. Probably there should be palette generators for dark colors, would be cool if anyone can provide a link for this.
That's true. But the name of the color should speak for itself, so I don't see any use case for it. --background: 0 0% 100%; /* white */
--foreground: 222.2 47.4% 11.2%; /* slate-900 */
--muted: 210 40% 96.1%; /* slate-100 */
--muted-foreground: 215.4 16.3% 46.9%; /* slate-500 */
--popover: 0 0% 100%; /* white */
--popover-foreground: 222.2 47.4% 11.2%; /* slate-900 */
--border: 214.3 31.8% 91.4%; /* slate-200 */
--input: 214.3 31.8% 91.4%; /* slate-200 */
--card: 0 0% 100%; /* white */
--card-foreground: 222.2 47.4% 11.2%; /* slate-900 */
--primary: 222.2 47.4% 11.2%; /* slate-900 */
--primary-foreground: 210 40% 98%; /* slate-50 */
--secondary: 210 40% 96.1%; /* slate-100 */
--secondary-foreground: 222.2 47.4% 11.2%; /* slate-900 */
--accent: 210 40% 96.1%; /* slate-100 */
--accent-foreground: 222.2 47.4% 11.2%; /* slate-900 */
--destructive: 0 100% 50%; /* red */
--destructive-foreground: 210 40% 98%; /* slate-50 */
--ring: 215 20.2% 65.1%; /* slate-400 */ |
@emrosenf The color values follow the Tailwind CSS convention. https://tailwindcss.com/docs/customizing-colors#using-css-variables (as @jondot said this is for resolving alphas). I like @its-monotype's inline comment suggestion to help improve documentation. Happy to merge a PR with this improvement. Thank you |
Closing in favor of #200 |
Incredible new update!
I re-organized your theming in a way that is more flexible and clear, especially for folks like myself who prefer to lean on the built-in tailwind.css palette.
Benefits
hsl()
slate
themeslate
->stone
)Even if you don't push this, I think it could be added to the documentation as an alternative theme setup