Reactstrap: No animation on modal mounting

[Rootmafia]

## Issue description

• version "reactstrap": "next",
• components: Modal
• react version "^16.0.0",

There is no fade in animation on Modal appearing, have you removed it?

[virgofx]

This is a valid bug we need to investigate further.

[devonjs]

@virgofx hello! could i take a stab at this one? been wanting to contribute for a while, would investigate tomorrow after work!

[TheSharpieOne]

@devonJS go for it. There is a branch which updates the dev version of react in this project to 16. You'll probably need to branch off of that or something to see the issue since this only happens with react 16.

[virgofx]

I believe the issue is we need to force a repaint on the initial load once it applies the first class. It looks like CSSTransition has this built in; however, we opted to use Transition, so it may behoove us to switchover; however, we need to update this everywhere and ensure we use the bootstrap version of the classes (e.g. show). (If of course, this is the issue and it fixes it -- otherwise need more investigation)

[TheSharpieOne]

There is an issue which was recently open with react-transition-group: https://github.com/reactjs/react-transition-group/issues/216. Not sure if it is valid or not, but it seems similar to what we are experiencing and thus we may be able to benefit from the solution to it; regardless of whether it is an issue with react-transition-group or not

[devonjs]

@virgofx @TheSharpieOne Great, thanks for the direction on this issue. Will consider what you both mentioned above when approaching this!

[devonjs]

hmmm there seems to be a couple of things to consider for React 16:

1. As mentioned, react-transition-group seems to not work as intended
2. Seems ReactDOM.createPortal would be the preferred way to accomplish what ReactDOM.unstable_renderSubtreeIntoContainer does react/issues/10309, documentation of what it does seems this way as well

ReactDOM.createPortal is such a big feature in React 16 that for this to play with React 16 and its future versions it should be used, what do you think?

It also seems like CSSTransition got bundled into react-transition-group in the react documentation, not too sure if switching to CSSTransition would achieve better results, I'd have to do a little more research into it.

[TheSharpieOne]

601 has the task of upgrading to portals, but to better maintain compatibility with react <16 the suggestion is to use createPortal and fallback to unstable_renderSubtreeIntoContainer if it is not there. material did something similar

As for Transition vs CSSTransition Yes, we are using react-transition group, The doc you reference are old, it itself references react-transition-group v1. The v2 docs can be found here. I think the preference is for everything to use Transition to be consistent.

[devonjs]

@TheSharpieOne sent PR, I'll leave the createPortal and unstable_renderSubtreeIntoContainer fallback to who ever picks up #601

[jonathanazulay]

I was gonna file this issue and reproduced it here. Also reproduces #360

https://codesandbox.io/embed/8x3nm1l659

[chrisparton1991]

To clarify, by having no animation on modal mounting are you saying that the modal instantly appears, or that it remains invisible?

I'm experiencing an issue where my modal has a fade class attached that never gets removed, leaving it invisible. If that's the issue here, I won't raise a dupe.

[TheSharpieOne]

The fade class stays the entire time the modal exists. This is how bootstrap does it. An additional class is added to affect the opacity.

[Kielan]

@TheSharpieOne Any update on this? I see the commit referencing this issue 2 months ago is just one line long. I have set up modals a few times in react and want to give it a go but I see two separate pr's, one which is not merged.

[jonathanazulay]

@Kielan Seems to be fixed in v5.0.0-alpha4, it appears to be a fade in and out on my iPhone

[virgofx]

I also believe this is fixed. There may be a few weird cases with large timeouts, but all of my use cases seem fixed.

[jonathanballs]

I can confirm that is fixed for me.

[TheSharpieOne]

Fixed in v5, closing.