zulip / zulip-mobile

Zulip mobile apps for Android and iOS.
https://zulip.com/apps/
Apache License 2.0
1.28k stars 643 forks source link

Avoid blocking UI in onPress handlers #2837

Open gnprice opened 6 years ago

gnprice commented 6 years ago

With #2799, we fixed a longstanding annoying issue in the app's UI: pressing a stream or topic in the unreads screen, the basic routine navigation move in the app, wouldn't give visual feedback on what you'd pressed. The cause was that in the onPress handler we were doing the navigation synchronously, so the animation didn't get to run until it was complete; the fix is to insert a setTimeout to let the UI respond before we do the navigation.

I just pushed similar fixes for a handful of other places in the app: see c003c77a9 and the four commits before it. (Note the delay helper I added. setTimeout works fine, except in a few places its return type doesn't match.)

There are still a bunch of places in the app where we have this issue -- if you go around into random parts of the UI deeper in the navigation, or just search for onPress and then find the corresponding widgets in the app and play with them, you find plenty. (In some of these places, if you're lucky you might see a few belated frames of the feedback animation; in others, I've never seen any.) We should fix them all.

We may want this in basically all our onPress handlers that do anything like navigate to a screen. If we can convince ourselves it's correct to put it everywhere, then we can put it in ZulipButton.js (like @jainkuniya suggested on #2799) -- that'd help a lot to handle this cleanly.

jainkuniya commented 6 years ago

We may want this in basically all our onPress handlers that do anything like navigate to a screen. If we can convince ourselves it's correct to put it everywhere, then we can put it in ZulipButton.js (like @jainkuniya suggested on #2799) -- that'd help a lot to handle this cleanly.

Let's try out this version đŸ˜…

@zulipbot claim