daisyui icon indicating copy to clipboard operation
daisyui copied to clipboard

bug: v3 Drawer not easing out when closed.

Open tyler-audio opened this issue 2 years ago โ€ข 6 comments

What version of daisyUI are you using?

3.0.0

Describe your issue

Prior to the release of 3.0.0 the drawer would transition out of view upon closing. I could not identify the specific issue here but it seems that once the drawer-toggle becomes unchecked, the menu's position shifts far below the viewport, but the transition still occurs. It appears to be this way in the documentation as well, so I'm not sure if this was intentional or not.

Further inspection of the previous release's docs, the drawer component transitions back properly. Forgive me if this was not clear. I can provide examples if need be.

What browsers are you seeing the problem on?

Chrome

Reproduction URL (optional)

https://play.tailwindcss.com/tuvGBNRfca

tyler-audio avatar Jun 01 '23 22:06 tyler-audio

also have wrong transition rtl enabled...

arasrezaei avatar Jun 02 '23 21:06 arasrezaei

also have wrong transition rtl enabled...

What's rtl?

tansanDOTeth avatar Jun 03 '23 03:06 tansanDOTeth

also have wrong transition rtl enabled...

What's rtl?

Right To Left

ezetojo avatar Jun 03 '23 07:06 ezetojo

Can I fix this? @saadeghi

LMK if this is the expected behavior @tyler-audio

https://github.com/saadeghi/daisyui/assets/53368431/3281d8f1-5eee-4d7d-90e2-30e51370c565

anjantalatam avatar Jun 03 '23 09:06 anjantalatam

Can I fix this? @saadeghi

LMK if this is the expected behavior @tyler-audio

Screen.Recording.2023-06-03.at.2.29.57.PM.mov

v2 is the expected behavior: https://v2.daisyui.com/components/drawer/

tansanDOTeth avatar Jun 03 '23 09:06 tansanDOTeth

Can I fix this? @saadeghi

Sure. Much appreciated ๐Ÿ™Œ

saadeghi avatar Jun 04 '23 13:06 saadeghi

My apologies.

Can I fix this? @saadeghi

LMK if this is the expected behavior @tyler-audio

Screen.Recording.2023-06-03.at.2.29.57.PM.mov

This looks close to the expected behavior. Reference v2 docs to be sure. It looks like the overlay might have some transition on it as well.

tyler-audio avatar Jun 06 '23 03:06 tyler-audio

@saadeghi I might hop in to fix this. I'm looking at the CSS differences between V2 and V3 and noticed many changes. Is it as simple as just moving more of that code back into V3? Could you give me a general direction on which area to look?

https://github.com/saadeghi/daisyui/blob/v2.52.0/src/components/styled/drawer.css https://github.com/saadeghi/daisyui/blob/master/src/components/styled/drawer.css

tansanDOTeth avatar Jun 07 '23 00:06 tansanDOTeth

Can I fix this? @saadeghi

LMK if this is the expected behavior @tyler-audio

Screen.Recording.2023-06-03.at.2.29.57.PM.mov

@anjantalatam Hey, any updates on this? I can work on this if you want

megagames-me avatar Jun 12 '23 11:06 megagames-me

Can I fix this? @saadeghi LMK if this is the expected behavior @tyler-audio Screen.Recording.2023-06-03.at.2.29.57.PM.mov

@anjantalatam Hey, any updates on this? I'm willing to work on this if you want

I would say go for it since they've haven't replied for a week.

tansanDOTeth avatar Jun 12 '23 11:06 tansanDOTeth

Yes, PRs are welcome.
Honestly I'm a little busy with other issues right now and this one has lower priority.

Here's the contribution guide: https://github.com/saadeghi/daisyui/blob/master/.github/CONTRIBUTING.md

Let me know if you needed help

saadeghi avatar Jun 12 '23 13:06 saadeghi

@saadeghi I believe I have fixed the transition issues. They were blocked because the overlay's dimensions became 0x0 the moment the drawer was closed, and the side was made invisible also when the drawer was closed. However, the animation timings look a bit dodgy now seeing it close properly so I'd just like to make sure with you that this is fine.

https://github.com/saadeghi/daisyui/assets/54014569/b2b6b120-37ab-4393-bb3f-993ed7975732

I wasn't sure whether to ask this in the PR or not because I'm new to this (I'm 14) so any help would be appreciated. Thanks!

megagames-me avatar Jun 12 '23 14:06 megagames-me

@saadeghi I believe I have fixed the transition issues. They were blocked because the overlay's dimensions became 0x0 the moment the drawer was closed, and the side was made invisible also when the drawer was closed. However, the animation timings look a bit dodgy now seeing it close properly so I'd just like to make sure with you that this is fine.

Screen.Recording.2023-06-12.at.10.03.02.PM.mov I wasn't sure whether to ask this in the PR or not because I'm new to this (I'm 14) so any help would be appreciated. Thanks!

Almost there. If you look at the v2 documentation, you'll see there is a slight translate on the backdrop.

tansanDOTeth avatar Jun 12 '23 14:06 tansanDOTeth

Is that not a glitch? I'm not sure whether it was intended.

megagames-me avatar Jun 12 '23 14:06 megagames-me

Is that not a glitch? I'm not sure whether it was intended.

Intended.

tansanDOTeth avatar Jun 12 '23 14:06 tansanDOTeth

I've added that to the PR. Is this good?

https://github.com/saadeghi/daisyui/assets/54014569/feb894be-3bfd-429d-a9eb-3cefc8c11858

megagames-me avatar Jun 12 '23 14:06 megagames-me

I've added that to the PR. Is this good?

Screen.Recording.2023-06-12.at.10.53.17.PM.mov

Looking good! I can't tell if it's the same, but it looks like V2 opens a bit faster. Is it the same timing?

tansanDOTeth avatar Jun 12 '23 14:06 tansanDOTeth

Thanks @megagames-me I will test the PR today and I will merge it soon.

The translate on the background was intended in v2 but it was creating issues like this So I don't think it's worth it. Can you please remove the translate?

saadeghi avatar Jun 12 '23 14:06 saadeghi

Thanks @megagames-me I will test the PR today and I will merge it soon.

The translate on the background was intended in v2 but it was creating issues like this So I don't think it's worth it. Can you please remove the translate?

If this was the case, this should go in the changelog under breaking changes for upgrading from V2 to V3. I would revert back to V2 for the animation. Alternatively, it should be an opt-in/opt-out class.

tansanDOTeth avatar Jun 12 '23 15:06 tansanDOTeth

@tansanDOTeth It's not a breaking change.
It won't be the default style but you can still have that by adding these 3 class names to drawer-content:

transition-transform translate-x-0 [.drawer-toggle:checked~&]:translate-x-2

I just don't think a default style should be a tradeoff to have a nice animation in cost of having issues with all sticky elements inside the drawer content.

saadeghi avatar Jun 12 '23 15:06 saadeghi

Looking good! I can't tell if it's the same, but it looks like V2 opens a bit faster. Is it the same timing?

Thanks! It's not the same timing, but it seems like an intentional change.

The translate on the background was intended in v2 but it was creating issues like this. So I don't think it's worth it. Can you please remove the translate?

Sure, I'll do that now.

megagames-me avatar Jun 13 '23 00:06 megagames-me

@tansanDOTeth It's not a breaking change. It won't be the default style but you can still have that by adding these 3 class names to drawer-content:

transition-transform translate-x-0 [.drawer-toggle:checked~&]:translate-x-2

I just don't think a default style should be a tradeoff to have a nice animation in cost of having issues with all sticky elements inside the drawer content.

Fair enough, but it is a notable animation change. For me, that slight bounce is the detail that I love about daisyUI; I really appreciate the effort to make those tiny changes. Without it, the animation feels very comparable with most other frameworks. Also, thank you for the translate transition, I would definitely need this.

tansanDOTeth avatar Jun 13 '23 00:06 tansanDOTeth

@tansanDOTeth It's not a breaking change. It won't be the default style but you can still have that by adding these 3 class names to drawer-content:

transition-transform translate-x-0 [.drawer-toggle:checked~&]:translate-x-2

I just don't think a default style should be a tradeoff to have a nice animation in cost of having issues with all sticky elements inside the drawer content.

Fair enough, but it is a notable animation change. For me, that slight bounce is the detail that I love about daisyUI; I really appreciate the effort to make those tiny changes. Without it, the animation feels very comparable with most other frameworks. Also, thank you for the translate transition, I would definitely need this.

Like you said, maybe have a single class you can add like dialog-transform for this? I don't think it could cause any issues and would be more convenient to remember than that.

megagames-me avatar Jun 13 '23 00:06 megagames-me

@tansanDOTeth It's not a breaking change. It won't be the default style but you can still have that by adding these 3 class names to drawer-content:

transition-transform translate-x-0 [.drawer-toggle:checked~&]:translate-x-2

I just don't think a default style should be a tradeoff to have a nice animation in cost of having issues with all sticky elements inside the drawer content.

Fair enough, but it is a notable animation change. For me, that slight bounce is the detail that I love about daisyUI; I really appreciate the effort to make those tiny changes. Without it, the animation feels very comparable with most other frameworks. Also, thank you for the translate transition, I would definitely need this.

Like you said, maybe have a single class you can add like dialog-transform for this? I don't think it could cause any issues and would be more convenient to remember than that.

It would definitely be a nice compromise for people who want to make those trade-offs, but I can also understand @saadeghi perspective as the API provider. The maintenance and potential issues from having different combinations may not be worth supporting such a customization.

tansanDOTeth avatar Jun 13 '23 00:06 tansanDOTeth