bug: v3 Drawer not easing out when closed.

[tyler-audio]

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

[arasrezaei]

also have wrong transition rtl enabled...

[tansanDOTeth]

also have wrong transition rtl enabled...

What's rtl?

[ezetojo]

also have wrong transition rtl enabled...

What's rtl?

Right To Left

[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

[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

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

[saadeghi]

Can I fix this? @saadeghi

Sure. Much appreciated 🙌

[tyler-audio]

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.

[tansanDOTeth]

@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

[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 can work on this if you want

[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'm willing to work on this if you want

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

[saadeghi]

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

[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!

[tansanDOTeth]

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

[megagames-me]

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

[tansanDOTeth]

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

Intended.

[megagames-me]

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

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

[tansanDOTeth]

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?

[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?

[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?

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.

[saadeghi]

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

[megagames-me]

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.

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

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

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

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.