fix fade animation for toast

[Johann-S]

Fixes: #27779

[ysds]

I think it is nice to have animation when showing too.

[Johann-S]

I'm a bit mix matched about that 🤔 IMO a toast should be visible ASAP so the animation will slower the display

[ysds]

Yes, let's allow the same approach as modal. https://getbootstrap.com/docs/4.1/components/modal/#remove-animation

[Johann-S]

it work as the same of our Modal plugin, if you remove the fade class or set animation to false you won't have any animation.

[MartijnCuppens]

IMO a toast should be visible ASAP so the animation will slower the display

I'm following Johann here. Toasts & modals both have a different purpose.

[MartijnCuppens]

@Johann-S, can you also remove the toasts from the DOM like we do with the alerts (https://deploy-preview-27792--twbs-bootstrap4.netlify.com/docs/4.1/components/alerts/#dismissing)?

The problem with not removing them is that there's a margin above the second toast if we dismiss the first: https://deploy-preview-27792--twbs-bootstrap4.netlify.com/docs/4.1/components/toasts/#stacking

[ysds]

The major OS's notification UI components have animation when display.

• iOS / macOS: Notification
• Android: Snackbar
• Windows: Balloon

It is OK that there is no animation at default, but it should be allowed customization IMO.

[MartijnCuppens]

Ow, I like that customizable idea!

[XhmikosR]

Maybe we should merge this and make it customizable in a another PR?

[MartijnCuppens]

@XhmikosR, this is still an issue: #27792 (comment)

[MartijnCuppens]

See #27792 (comment)

[XhmikosR]

@MartijnCuppens Not a blocker to me, at least.

Removing from DOM will be handled in other ticket (#27807)

[Johann-S]

The customization of the show animation can be tackled in an other PR, so we should merge this one, except if you have any other concerns ?

[MartijnCuppens]

LGTM

[rafalp]

If every UI item has fade-in animation, but toast doesn't, you'll get the clunky feeling.

On the other side, 300ms animation is hardly a delay that would break the world, especially in the web app that already deals with latency because it may be waiting for API response, pooling the backend for events, using features like refetchInterval={} that separate user seconds away from learning something.

[Johann-S]

Alert don't have fade-in animation too.

I don't understand your second point 😟 The animation delay can be changed but I don't think it's what you mean 🤔

[rafalp]

@Johann-S

I don't understand your second point 😟 The animation delay can be changed but I don't think it's what you mean 🤔

My point is: web applications are not always realtime, most events are signalized to user with delay, so thinking that component animation is a problem is misguided. ;)

[Johann-S]

Oh ok now I understand 👍
That's true, but folks can enable/disable that animation