Talor-A / react-native-message-bar

A notification bar alert displayed at the top of the screen for react-native
MIT License
55 stars 27 forks source link

Fix show/hide completion callback #34

Closed chrisgibbs44 closed 6 years ago

chrisgibbs44 commented 6 years ago

Otherwise setting the completion callback to e.g. this._showMessageBarAlertComplete() executes the callback straight away, not after the animation finishes

Setting it to this._showMessageBarAlertComplete only partially works - .bind(this) is needed to resolve the references to 'this' inside the callback

Talor-A commented 6 years ago

Thanks! I'll give this a quick test and merge if everything seems ok

Talor-A commented 6 years ago

What’s the status of this? Still WIP or ready for review? Thanks for the improvements btw

chrisgibbs44 commented 6 years ago

Oh yeah sorry, yeah it's only commit 4db7e2a that can be merged back in really, the rest of these are custom fixes for our fork (we only use slide from top, and have been a bit pushed for time to work out a general fix for the initial animation lag)

chrisgibbs44 commented 6 years ago

The show/hide animation lag is due to the component starting way off screen (by the full screen width / height) - if the animation starts from there, then most of the animation has been missed by the time it actually arrives on screen.

Initially I tried a solution that automatically recalculated the component size for the onLayout event in View.Animated and started the animation by only this amount off-screen, but this was a bit experimental - ideally the component size needs to be calculated before it's displayed (i.e. before the onLayout event).

In the end we've gone for a custom approach of starting all slide from top animations from 30% off the top of the screen, as our notifications don't use more than this, but this is for our use only really - for a general approach we need to figure out a way of automatically calculating the correct component size after the title / message is set but before the animation starts.

chrisgibbs44 commented 6 years ago

Actually the Android fix for 5e1e05c can be merged in, but that needs extending in a similar way to the other 3 slide types

chrisgibbs44 commented 6 years ago

Closing this PR for now, feel feel to grab selected commits from it