shadcn-ui / ui

Beautifully designed components that you can copy and paste into your apps. Accessible. Customizable. Open Source.
https://ui.shadcn.com
MIT License
73.59k stars 4.53k forks source link

Support for exit animations on Dialogs, Menus,... #72

Closed sanvishal closed 1 year ago

sanvishal commented 1 year ago

I've been using this component library for a couple of days now, LOVING IT!

The only part that's missing is the exit animations, especially for the dialogs and menu components. They animate-in nicely but they disappear instantly on unmount. I'm somewhat new to radixUI, but I read this from radix which basically uses the data-[state=open/close] attributes to animate the state transitions (and this is what accordions seem to use on this library with animations declared on tw config)

So, is not having exit animations on certain components an intentional design choice?

shadcn commented 1 year ago

@sanvishal Some of the exit animations are intentionally not added.

Example: for alert dialog you want visual feedback (animation) when the dialog enters but you want it to "get it out of your way" when exit. (I tried adding exit animations and it felt weird/sluggish).

Some are probably oversight on my part. Can you let me know which components and I can fix?

Thank you.

sanvishal commented 1 year ago

thanks, but for now, I've only seen them missing on dialogs, dropdown menus, tooltips, and popovers. I think your justification about sluggishness makes sense to me in some cases.

I'll create a PR or a separate issue if I find any other components. (feel free to close this one)

its-monotype commented 1 year ago

probably it would be nice to add exit animation on Sheet component

natac13 commented 1 year ago

I was wondering this myself today. I was trying to modify the dialog to be a slide in nav from the left. I was able to get the enter slide in going but the exit I'm having trouble with. I have tried force mount on the portal but I was getting sluggish animations

joaom00 commented 1 year ago

Hey @natac13, how are you doing the exit animation?

natac13 commented 1 year ago

Hey @natac13, how are you doing the exit animation?

I followed something similar to this. However I was not very successful. My idea was to remove the framer motion stuff from the example but leave the open prop. I then made a duplicate of the DialogContent component from this library and used the forceMount prop on the portal then tried to use the data-[state=closed] prefix to handle the exit animation but was getting bad results

joaom00 commented 1 year ago

With tailwind, just use data-* modifier: data-[state=open]:animate-show data-[state=closed]:animate-hide

https://codesandbox.io/p/sandbox/pedantic-aryabhata-1ykgmk?file=%2FApp.jsx

sanvishal commented 1 year ago

I tried to add an exit animation to the dialog component, but it just doesn't work for some reason. I added the dialog component in the next-template imported from @/components/ui/dialog.

I added data-[state=closed]:animate-out data-[state=closed]:fade-out-10 data-[state=closed]:zoom-out-50 to the <DialogContent /> here

I've also added logs to the onAnimationStart/End callback, this is what I got after opening and closing the dialog 1 time

start anim: enter
end anim: enter

additionally, I should've also gotten the exit animation log too while closing the dialog, but it seems like radix did not suspend the "unmounting" till the animation finishes up. Is this a radix-specific problem or some other? because the codesandbox by @joaom00 seems to work just fine.

its-monotype commented 1 year ago

I tried to add an exit animation to the dialog component, but it just doesn't work for some reason. I added the dialog component in the next-template imported from @/components/ui/dialog.

I added data-[state=closed]:animate-out data-[state=closed]:fade-out-10 data-[state=closed]:zoom-out-50 to the <DialogContent /> here

I've also added logs to the onAnimationStart/End callback, this is what I got after opening and closing the dialog 1 time

start anim: enter
end anim: enter

additionally, I should've also gotten the exit animation log too while closing the dialog, but it seems like radix did not suspend the "unmounting" till the animation finishes up. Is this a radix-specific problem or some other? because the codesandbox by @joaom00 seems to work just fine.

I have the same problem when I tried to add exit animations to Radix components using tailwind-animate plugin, however with self writing animations everything works fine

sanvishal commented 1 year ago

well, that's a minor inconvenience, TIL :)

thanks, I'll try with non-tailwindy animations!

joaom00 commented 1 year ago

@its-monotype @sanvishal Using tailwind-animate seems to work for me

https://codesandbox.io/p/sandbox/busy-noether-9ew9u2?file=%2FApp.jsx

About the onAnimationEnd not firing on close, it's a issue https://github.com/radix-ui/primitives/issues/1020

its-monotype commented 1 year ago

@joaom00 Wow, it’s strange, but I really tried it now and it works, I don’t even know what happened then, maybe a bug or blunted somewhere. Thankee! Also, you can revisit DialogOverlay animation in the example then, using that plugin like that: 'data-[state=open]:animate-in data-[state=open]:fade-in', 'data-[state=closed]:animate-out data-[state=closed]:fade-out'

its-monotype commented 1 year ago

@joaom00 Is it possible to do something with the fact that because of '-translate-x-1/2 -translate-y-1/2' the animation starts from the corner?

joaom00 commented 1 year ago

@its-monotype Yes, I updated the example with tailwind-animate to look like the one in tailwind.config

sanvishal commented 1 year ago

@joaom00 Is it possible to do something with the fact that because of '-translate-x-1/2 -translate-y-1/2' the animation starts from the corner?

yep, that seems to be the issue, thanks @joaom00

digoburigo commented 1 year ago

Integration with this makes sense? tailwindcss-radix

baumerdev commented 1 year ago

I played a little bit with the code of shadcn/ui's Dialog. The main problem seemed to be the <div> between DialogPrimitive.Portal and {children}:

const DialogPortal = ({
  className,
  children,
  ...props
}: DialogPrimitive.DialogPortalProps) => (
  <DialogPrimitive.Portal className={cn(className)} {...props}>
    <div className="fixed inset-0 z-50 flex items-end justify-center sm:items-center">
      {children}
    </div>
  </DialogPrimitive.Portal>
)
DialogPortal.displayName = DialogPrimitive.Portal.displayName

This causes Radix UI's Portal to ignore animations on Overlay and Content and thus removing the Portal's content immediately. Therefore none of the animation you might have added were visible because the elements weren't longer part of the DOM.

In shadcn/ui#127 I removed the <div>. Problem now is that the positioning of the Overlay and Content relied on the parent div which used CSS's flex. Now the components are directly in the DOM and we need to position them on their own with fixed. And because this positioning is a little bit more complex the animations needed to be as well.

The result should be visually the same with an additional exit animation.

statusunknown418 commented 1 year ago

Any updates on this issue 🤕?

baumerdev commented 1 year ago

@AlvaroAquijeDiaz My PR hasn't been merged to the code base yet and meanwhile the upstream code has changed. If I find time in the next days I can adjust my PR for the current master. But if it it's urgent, the code changes in my PR/previous comment should still work if you change this in your code accordingly.

baumerdev commented 1 year ago

Based on the current main branch I re-did the exit animation for dialog and alert-dialog as well as some other components. If anyone needs it now, see the diff in my PR what to change.

yavorpunchev commented 1 year ago

This causes Radix UI's Portal to ignore animations on Overlay and Content and thus removing the Portal's content immediately. Therefore none of the animation you might have added were visible because the elements weren't longer part of the DOM.

@baumerdev I've ran into this issue before and had solved it by adding a "fake" animation to the div in question which doesn't do anything visually different:

fakeAnimation: {
  from: { opacity: '1' },
  to: { opacity: '1' },
},

That way Radix.Portal doesn't ignore the underlying Radix.Content animation.

EIIisD commented 1 year ago

This causes Radix UI's Portal to ignore animations on Overlay and Content and thus removing the Portal's content immediately. Therefore none of the animation you might have added were visible because the elements weren't longer part of the DOM.

@baumerdev I've ran into this issue before and had solved it by adding a "fake" animation to the div in question which doesn't do anything visually different:

fakeAnimation: {
  from: { opacity: '1' },
  to: { opacity: '1' },
},

That way Radix.Portal doesn't ignore the underlying Radix.Content animation.

Hey, can you provide full details on how you achieved this? Unsure if this snippet is for some animation library?

yavorpunchev commented 1 year ago

Hey, can you provide full details on how you achieved this? Unsure if this snippet is for some animation library?

This is a pure CSS animation, the library doesn't really matter (the above snippet was done with stitches, but can work with tailwind or any styling library of your choice).

EIIisD commented 1 year ago

Hey, can you provide full details on how you achieved this? Unsure if this snippet is for some animation library?

This is a pure CSS animation, the library doesn't really matter (the above snippet was done with stitches, but can work with tailwind or any styling library of your choice).

Would love to get this working but can't seem to replicate the fix you descibe - any chance you could drop a sandbox?

yavorpunchev commented 1 year ago

@eIIisd sure, here's a repo with the fix. One thing I forgot to mention is that the div in question has to he a Dialog.Overlay under the hood so that we can hook into the appropriate open/closed data attributes

shadcn commented 1 year ago

We now have exit animations on all components.

MateusPitura commented 4 months ago

After hours of trying to solve this and to searching a lot, I solve my problem. The post of @baumerdev helps me. I'll brief my learn:

  1. To use data-state in tailwind do
    data-[state=closed]:your-animation
  2. Don't put a div between Portal and Overlay or Content, otherwise the exit animation will you fail
  3. I'm centralizing my dialog with
    fixed inset-0

    When I put this code in the className of Content, I can't close the portal clicking outside, so I discovered that I can nest the content inside the overlay, and pow! I put the above code into overlay and it works for me