-
Notifications
You must be signed in to change notification settings - Fork 797
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
Daisy UI v4 #656
Daisy UI v4 #656
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GJ @technophile-04 !! Is looking good to me 🙌
Only found something a bit weird with the button background colors on hover, I'll send a couple of videos (very bad quality after compressing, can send uncompress in tg):
In the rainbowkit and faucet buttons is easier to see.
Ohh yes didn't notice it initially, actually this behavior was updated in daisy UI v4 by default :( Screen.Recording.2023-12-12.at.9.04.05.PM.movLeft side is V4 and the right side is v3. Tried digging in and the problem seems that DaisyUI v4 has removed the
Possible Solution :After saadeghi/daisyui#2647 (comment) , I think it kind of makes sense to keep things as it since there is also no way to properly get the previous dasiyUI v3 effect. We get the hsl value for One improvement we can do is if we feel the black overlay is too much we can the overlay a bit lighter :
Code for this in ".btn:hover": {
backgroundColor: "color-mix(in oklab, oklch(var(--btn-color)), black 4%)",
borderColor: "color-mix(in oklab, oklch(var(--btn-color)),black 4%)",
}, Previous edit :We can add something like this `tailwind.config.ts` :".btn-secondary:hover": {
backgroundColor: "color-mix(in oklab, oklch(var(--s)), black 7%)",
borderColor: "color-mix(in oklab, oklch(var(--s)),black 7%)",
}, but then we have to do it for So not sure whether to keep it default as DasiyUI default or we tweak it ? cc @carletex @rin-st @sverps will keep digging if we could find a better way to tweak it Also created this saadeghi/daisyui#2647 just incase we get the proper solution |
Awesome research @technophile-04 !! Knowing this is an intended change from Daisy v4, I would probably just leave it like it is, and let users define the hover bg color on their apps (if they don't like the default hover effect). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great @technophile-04.Thanks for this!
Left a comment with a couple of little things
Description
Fixes #629