fix(Modal): Fix wrong types on ModalProps

[golota60]

Resolves: #5732

[golota60]

Sorry for waiting such a long time on this, had a busy week.

Anyways, this change probably picked up(and allowed me to fix) a bug where onExited wasn't passing a node(see screenshot)

I've removed passing of ...args in handleEnter, handleExit, handleExiting,handleExited - I've checked and it doesn't break anything, and I'm pretty sure they weren't supposed to be there in the first place(I've traced the code and there is always only one node argument passed).

I've tried to trace a reason as to why/when ..args were added(and if they're passed for a reason) and found #3187 , which leads me to believe they are not supposed to be there

[kyletsang]

Sorry for waiting such a long time on this, had a busy week.

Anyways, this change probably picked up(and allowed me to fix) a bug where onExited wasn't passing a node(see screenshot)

I've removed passing of ...args in handleEnter, handleExit, handleExiting,handleExited - I've checked and it doesn't break anything, and I'm pretty sure they weren't supposed to be there in the first place(I've traced the code and there is always only one node argument passed).

I've tried to trace a reason as to why/when ..args were added(and if they're passed for a reason) and found #3187 , which leads me to believe they are not supposed to be there

Nice catch on the missing node arg in onExited.

We should keep the args, because all the "enter" callbacks do have an additional isAppearing arg in them: (http://reactcommunity.org/react-transition-group/transition/#Transition-prop-onEnter). There's still an issue in the TransitionCallbacks type in helpers.ts though, because it only specify a node arg.

[golota60]

Thanks for looking into this!

It seems like ...args should be only in handleEnter, cause according to http://reactcommunity.org/react-transition-group/transition/#Transition-prop-onExit, onExit,onExiting and onExited are passing only node.

Commited the change, but it still needs react-bootstrap/react-overlays#962.

[kyletsang]

Thanks for looking into this!

It seems like ...args should be only in handleEnter, cause according to http://reactcommunity.org/react-transition-group/transition/#Transition-prop-onExit, onExit,onExiting and onExited are passing only node.

Commited the change, but it still needs react-bootstrap/react-overlays#962.

handleEntering needs it as well. Hmm seems like CI is failing, but shouldn't be related to your changes...

[golota60]

@kyletsang I wasn't able to pin down why ...args wasn't working(TS just gave me A spread argument must either have a tuple type or be passed to a rest parameter. which wasnt really helpful). But since we only will ever pass isAppearing, I just decided to replace ...args with it.

But in order for this to pass CI, we need react-bootstrap/react-overlays#962 to be merged & released.

[kyletsang]

@golota60, sorry missed that PR. It's merged and released in 5.1.1 now

[golota60]

@kyletsang everything should be ready to merge 😄

[kyletsang]

Thanks!