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

fix --radius CSS variable not being taken from the themes.ts definition #7

Closed
wants to merge 1 commit into from

Conversation

ubill
Copy link

@ubill ubill commented Jun 27, 2024

Description

You can reproduce the problem with a theme switcher, the border-radius for a button stays at --radius which is always 0.5rem before this fix. It stayed at 0.5rem no matter which theme was switched to.

Linked Issues

None found.

Additional context

None.

…on (it stayed at 0.5rem no matter which theme was switched to.)
Copy link

vercel bot commented Jun 27, 2024

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

Name Status Preview Comments Updated (UTC)
unocss-preset-shadcn-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 27, 2024 6:29am

@hyoban
Copy link
Owner

hyoban commented Jun 27, 2024

I can not reproduce it on https://unocss-preset-shadcn.vercel.app, it seems to work fine.

@ubill
Copy link
Author

ubill commented Jun 27, 2024

It looks like you will not see this problem on your vercel app, reason is because,
in these lines,
https://github.com/hyoban/unocss-preset-shadcn/blob/main/playground/src/components/theme-switch.tsx#L54-L55
you directly set the --radius CSS variable on the document.body. And you get the value to set [0, ... , ... , 1] from the page variables itself.

document.body.style.setProperty('--radius',${radius}rem)
setCurrentRadius(radius)

If instead, the user is relying solely on the CSS variable --radius being changed by a theme switcher cycling between ['zinc', 'stone', ...etc] to change the CSS variable,
e.g. expecting 0.95rem as specified in the themes definitions pasted into themes.ts,
https://github.com/hyoban/unocss-preset-shadcn/blob/main/src/themes.ts#L170

Then, this 'hardcoded' value 0.5rem is used for all themes regardless, since the theme-specific value e.g. 0.95 was not extracted out from the 'light' definition.

Hope this info helps.

@ubill
Copy link
Author

ubill commented Jun 27, 2024

Forgot to paste in the previous comment, the value of 0.5 is coming from this line.

https://github.com/hyoban/unocss-preset-shadcn/blob/main/src/generate.ts#L92

@hyoban
Copy link
Owner

hyoban commented Jun 27, 2024

In this project, color and radius are individual. I think you should always use a number for the radius.

Radius in themes do nothing here, I just copy them from shadcn-ui. You can see how shadcn-ui handles theme switch here.

@hyoban
Copy link
Owner

hyoban commented Jul 5, 2024

Thanks for your contribution, but close for now.

@hyoban hyoban closed this Jul 5, 2024
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