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

Daisy UI v4 #656

Merged
merged 10 commits into from Dec 19, 2023
Merged

Daisy UI v4 #656

merged 10 commits into from Dec 19, 2023

Conversation

technophile-04
Copy link
Collaborator

Description

Fixes #629

Copy link
Collaborator

@rin-st rin-st left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

Copy link
Collaborator

@Pabl0cks Pabl0cks left a 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.

Check videos

Buttons_hover bg-color-2

Buttons_hover bg-color-1

@technophile-04
Copy link
Collaborator Author

technophile-04 commented Dec 12, 2023

Only found something a bit weird with the button background colors on hover,

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.mov

Left side is V4 and the right side is v3.


Tried digging in and the problem seems that DaisyUI v4 has removed the *-focus i.e they are also no more generating --sf (secondary-focus) color and --pf (primary-focus) color instead they are suggesting to manually do it with color-mix checkout the docs here -> Breaking changes-colorTheme

Daisy V4 hover classes Dasiy V3 hover classes
Screenshot 2023-12-12 at 9 42 51 PM Screenshot 2023-12-12 at 9 39 50 PM

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 --sf and hardcode them in tailwind.config.ts for .btn-secondary:hover similar for primary etc but if people update their theme color then they again need to manually update this too.

One improvement we can do is if we feel the black overlay is too much we can the overlay a bit lighter :

Before After
ezgif com-video-to-gif-converted ezgif com-optimize (1)

Code for this in tailwind.config.ts :

  ".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 .btn-primary:hover etc too and not sure if we should tweak daisy UI defaults, also the above snipped doesn't seem to generate exact color as dasiUI v3 --sf :(

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

@Pabl0cks
Copy link
Collaborator

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).

Copy link
Member

@carletex carletex left a 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

@carletex carletex merged commit 00ec800 into main Dec 19, 2023
1 check passed
@carletex carletex deleted the daisyUI-v4 branch December 19, 2023 16:57
@github-actions github-actions bot mentioned this pull request Dec 21, 2023
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.

Upgrade to DaisyUI v4
4 participants