Skip to content

feat: tailwind prefix + latest shadcn component #146

Closed
ArnavK-09 wants to merge 6 commits intotscircuit:mainfrom
ArnavK-09:main
Closed

feat: tailwind prefix + latest shadcn component #146
ArnavK-09 wants to merge 6 commits intotscircuit:mainfrom
ArnavK-09:main

Conversation

@ArnavK-09
Copy link
Copy Markdown
Contributor

/claim #145

@vercel
Copy link
Copy Markdown

vercel Bot commented Jan 17, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
runframe ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 17, 2025 11:22pm

@ArnavK-09 ArnavK-09 changed the title feat: tailwind prefix feat: tailwind prefix + latest shadcn component Jan 17, 2025
@seveibar
Copy link
Copy Markdown
Contributor

seveibar commented Jan 17, 2025

@ArnavK-09 really nice work, will be pulling and testing this shortly, when i tried to do this i found a lot of little tiny errors on the page, but your work here looks correct

@ArnavK-09
Copy link
Copy Markdown
Contributor Author

I'll wait for the response, btw any way to fix conflict?

@seveibar
Copy link
Copy Markdown
Contributor

@ArnavK-09 yes to resolve conflict you just do bun i

Actually could you merge the latest main? It should fix your preview deployment.

git fetch
git merge origin/main

@ArnavK-09
Copy link
Copy Markdown
Contributor Author

ArnavK-09 commented Jan 17, 2025

Screenshot_2025-01-18-03-23-16-86_40deb401b9ffe8e1df2f1cc5ba480b12

Ohh an too bad at git stuff, any fix or should create new pr?

Maybe because I updated PKG json for latest shadn

@ArnavK-09
Copy link
Copy Markdown
Contributor Author

Hey @seveibar sorry for issues, I tried to resolve commit, can you review and proceed to merge it valid? Thanks

@seveibar
Copy link
Copy Markdown
Contributor

i found this issue with border radiuses. Did you regenerate the shadcn components using the cli?

image

CC @imrishabh18 if this prefix method works we could theoretically use this instead of twind for libraries, although twind is a bit cleaner imo

@ArnavK-09
Copy link
Copy Markdown
Contributor Author

i found this issue with border radiuses. Did you regenerate the shadcn components using the cli?

image

CC @imrishabh18 if this prefix method works we could theoretically use this instead of twind for libraries, although twind is a bit cleaner imo

Oh yes, i regenerated with shadcn to get latest version, it also generated some radius css variables, i deleted them, my bad
Should I bring variables back or any other plan?

@seveibar
Copy link
Copy Markdown
Contributor

yes let's bring back the variables so that the radiuses are correct, regenerating was the right way to do it fwiw 👍

@ArnavK-09
Copy link
Copy Markdown
Contributor Author

yes let's bring back the variables so that the radiuses are correct, regenerating was the right way to do it fwiw 👍

I just noticed that updating shadcn components to latest version also updated some styling of shadcn components included heights and radius, I tried to revert radius style of tabs as before! If any other styling you want as before lemme know, I'll wait for approval
Thanks

@seveibar
Copy link
Copy Markdown
Contributor

seveibar commented Jan 18, 2025

err, looks like it's not right yet

Your Update

image

Last Released Version

image

@seveibar
Copy link
Copy Markdown
Contributor

seveibar commented Jan 18, 2025

@ArnavK-09 if you look at the screenshots you can see the padding is wrong. I don't care whether or not this changes- it's fine to use the latest shadcn version OR to not change from the existing style, but the padding around the tab being off hints that we may possibly have an issue with padding in other places (just want to make sure we load everything in properly)

@seveibar
Copy link
Copy Markdown
Contributor

I'm not sure if it's clear but the X padding does not match the Y padding. We'd like to use the latest version of shadcn without modifications, but it seems like it might not be loading properly

@seveibar
Copy link
Copy Markdown
Contributor

From the shadcn homepage, also uniform padding around the active tab

image

@seveibar
Copy link
Copy Markdown
Contributor

I know it's a million comments at this point but for emphasis: please don't try to fix the padding or change ANY shadcn styles or adapt them to match the old ones etc. We want to use shadcn without modifications, but the tab issue is just a hint that it's not loaded properly

@seveibar
Copy link
Copy Markdown
Contributor

fwiw i updated the components on your branch and for whatever reason it didn't really work, a bunch of styles were lost (!)

image

So i don't completely know what's going on here...

@seveibar
Copy link
Copy Markdown
Contributor

been working on this for an hour or so now, closing but will allow you to get a partial claim, TBA shortly

@ArnavK-09
Copy link
Copy Markdown
Contributor Author

been working on this for an hour or so now, closing but will allow you to get a partial claim, TBA shortly

so should i continue to resolve these issues or not?

@ArnavK-09
Copy link
Copy Markdown
Contributor Author

err, looks like it's not right yet

Your Update

image

Last Released Version

image

it maybe because i manually changed height after installation latest components, if you want i try to fix these issues, but better to let me know if you plan to close this pr or not so that i can continue to resolve these issues, thnks

@seveibar
Copy link
Copy Markdown
Contributor

closing in favor #149 , thanks @ArnavK-09 !!!

@seveibar seveibar closed this Jan 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants