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

refactor: tailwindcss, remove all unnecessary arbitrary values, update TailwindCSS to v3.4 #269

Merged

Conversation

hrynevychroman
Copy link
Collaborator

1. Remove some arbitrary values: For example we had translate-x-[-50%] previously, but there is -translate-x-1/2 predefined class 🙂
2. In TailwindCSS v3.4 we received extended min/max-width/height predefined rules: updated all values to be more native [min-w-[8rem] -> min-w-32]
3. In TailwindCSS v3.3 we received CSS variabled without var(): update all custom variables [h-[var(--radix-select-trigger-height)] -> h-[--radix-select-trigger-height]]
4. Grid Updates. There is some custom grid-templates defined, for example grid-cols-[25px_1fr], but use fr values without wrap it inside minmax(0,*fr) not safe, so updated all values to be wrapped up. [grid-cols-[25px_1fr] -> grid-cols-[25px_minmax(0,1fr)]]. As a reference used default grid-templates rules, where minmax used. Link to docs

If you have some suggestions or wishes please add comments, I will update my PR ❤️

@sadeghbarati
Copy link
Collaborator

Nice changes @romanhrynevych 💯

We should pay more attention to the TailwindCSS blog and changelogs

All Good! can you resolve this conflicts?

@hrynevychroman
Copy link
Collaborator Author

@sadeghbarati thanks 🙌

Yeah, I will resolve conflict and notify you for review ❤️

@hrynevychroman hrynevychroman marked this pull request as draft January 13, 2024 08:51
…nd-classes

# Conflicts:
#	apps/www/src/public/registry/styles/default/calendar.json
#	apps/www/src/public/registry/styles/default/select.json
#	apps/www/src/public/registry/styles/new-york/calendar.json
#	apps/www/src/public/registry/styles/new-york/select.json
@hrynevychroman hrynevychroman marked this pull request as ready for review January 13, 2024 08:59
@hrynevychroman
Copy link
Collaborator Author

hrynevychroman commented Jan 13, 2024

@sadeghbarati all conficts was in /registry .json files, so just rebuild it 🙂
But there is apiToken error in Cloudflare, maybe you can help ❤️

@sadeghbarati
Copy link
Collaborator

But there is apiToken error in Cloudflare

Don't worry about it Zernonia will fixed it later

@valh1996
Copy link
Contributor

valh1996 commented Jan 15, 2024

Well done @romanhrynevych for this update !
As a small note, what do you think of using the has-[] variant that appeared in version 3.4 aswell?

I know that has() is widely used in the calendar component, for example. I don't know if that would complicate things more than it would simplify them, what do you think?

@hrynevychroman
Copy link
Collaborator Author

Well done @romanhrynevych for this update ! As a small note, what do you think of using the has-[] variant that appeared in version 3.4 aswell?

I know that has() is widely used in the calendar component, for example. I don't know if that would complicate things more than it would simplify them, what do you think?

I think there is no need for making that changes, in Calendar.vue used <style> tag with @apply styles, if styles was defined like classNames maybe, but now I think we can just leave this)

@hrynevychroman
Copy link
Collaborator Author

hrynevychroman commented Jan 15, 2024

Also, I'm not a very big fan of @apply styles, because than tailwind.css file becomes bigger. For example if you have w-full class in 100 places of code, it will only add once, but if you assign it by @apply, tailwindcss must write new class selector and add this styles, so all magic of tailwindcss is just disappeared 🥲

Copy link

Deploying with Cloudflare Pages

Name Result
Last commit: dfecd06a
Status: ⚡️ Deployment in progress...
Preview URL: https://6563461e.shadcn-vue.pages.dev
Branch Preview URL: https://6563461e.shadcn-vue.pages.dev

@sadeghbarati
Copy link
Collaborator

Dam, I'll resolve the conflict

@sadeghbarati sadeghbarati merged commit fc71814 into unovue:dev Feb 2, 2024
2 checks passed
@sadeghbarati sadeghbarati deleted the refactor/update-tailwind-classes branch February 2, 2024 19:19
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.

4 participants