search

robinhood / ticker

An Android text view with scrolling text change animation
https://medium.com/robinhood-engineering/hello-ticker-20eaf6e51689
Apache License 2.0
4.38k stars 462 forks source link

ticker is unable to smoothly transition during resize #63

Closed UnsungHero97 closed 7 years ago

UnsungHero97 commented 7 years ago

OnePlus One A0001 Android 6.0.1 Cyanogen OS 13.1.2

There's a bug that impacts the smoothness of the animations/transitions. It happens when the ticker needs to resize the dollar value string, for example, when it transitions from above $100 to below $100, or from above $10 to below $10. I've recorded a video of my screen to demonstrate the issue on my device:

https://www.dropbox.com/s/y4k3f3u6jb3wulm/device-2017-09-21-104722.mp4

This has been happening for well over a year, maybe longer. When will this be resolved?

jinatonic commented 7 years ago

Hello,

Thanks for filing the issue. This is actually not a problem with ticker (well, I guess it technically is), but it's because on the robinhood app there are simply too many data points on the 1Y graph. So when you hold down your finger at one point, the data under your finger continuously changes among 2 or 3 values which means ticker tries to animate from X to A then B then A then B then A etc. and never finishes the animation.

But yeah, we should fix that issue on robinhood side to buffer input events once you start scrubbing the graph. I'll try to get it in for a near future release.

UnsungHero97 commented 7 years ago

The issue is reproducible on the 1D and 1M graphs as well.

I just checked the Workday stock WDAY for today, and it drops below $100 around 10:10am, as well as around 12:50pm. The issue is reproducible at those dips in the 1D graph. The same happens when looking at the 1M graph for GoPro GPRO.

Let me know if you need a screen recording to demonstrate.

jinatonic commented 7 years ago

oh interesting, you are right, ill look into this.

UnsungHero97 commented 7 years ago

Thanks!

jinatonic commented 7 years ago

so after looking it ended up still being the android touch system dispatching multiple events despite your finger being completely still as I can't reproduce the issue in the ticker library or on android emulator scrubbing the GPRO graph (but I can repro it on my phone). I'll do the fix in the robinhood app.

jinatonic commented 7 years ago

The issue is fixed in robinhood and will be included in the next app update.

UnsungHero97 commented 7 years ago

Sweet! Thanks for the quick turnaround on this! Can't wait for the next app update to come through!

danh32 commented 7 years ago

I wonder if it's worth adding a safeguard in ticker as well? Like no-op the setText() call if we're already animating to the same value?

jinatonic commented 7 years ago

haha good point. this was actually supported by the library but one of my refactors on how tickercolumn works must have broke it.