1141500 - Delay after releasing finger when swiping open/close utility tray

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Chris Lord [:cwiiis]]

When you swipe open or close the utility tray, after lifting your finger there is a short pause before the animation finishes.

I think this is because the animation is ease-in. ease-in definitely looks nicer than linear for the entire animation, but this animation needs to start half-way through an ease-in, really - we may need to define a custom easing.

Alternatively, use a linear animation when the drag is over a certain velocity and an ease-in when below.

Ideally this would be physics-based, but that's tricky. A nice way of doing this would be to leverage async scrolling physics and css scroll-snapping, but I don't think that's feasible without either animations tied to scroll position (+ async support for that) or async animated clip rects.

Comment 1

[zhaoyang]

Hi Chris Lord,

   Do you have new progress about this bug? I am keenly interested in this

   thanks

Comment 2

[Chris Lord [:cwiiis]]

(In reply to zhaoyang from comment #1)
> Hi Chris Lord,
> 
>    Do you have new progress about this bug? I am keenly interested in this
> 
>    thanks

I may get to this today, but if not, I'll get to it on Monday (I'm on PTO tomorrow and away over the weekend).

If there's no report in this bug by the end of the day and you can complete it before Monday, feel free to take it.

Comment 3

[Chris Lord [:cwiiis]]

Looks like my assumptions were incorrect, this is already a linear animation... The delay might be due to work caused by style-changes initiated at the point of animation initiation in that case...

Either way, looking at this, will hopefully come back with a solution whatever the issue ends up being.

Comment 4

[Chris Lord [:cwiiis]]

So significant slow-down is caused by setting the 'utility-tray' class on the screen, but there's a reasonable amount of other work going on that I think could just as well be done after the transition finishes.

Going to see if I can restructure things a little, and also change up the transition so that its speed is dependent on the motion initiated by the user.

Comment 5

[GitHub Autolander Bot]

Attachment: [gaia] Cwiiis:bug1141500-touch-delay-utility-tray > mozilla-b2g:master

Comment 6

[Chris Lord [:cwiiis]]

(In reply to Autolander from comment #5)
> Created attachment 8576671 [details] [review]
> [gaia] Cwiiis:bug1141500-touch-delay-utility-tray > mozilla-b2g:master

This pull-request fixes most of the lag, but there is still some perceptible delay that I think is now just because the animation takes too long - so going to try making the animation length based on the properties of the swipe that comes before it now.

Comment 7

[Chris Lord [:cwiiis]]

Comment on attachment 8576671 [details] [review]
[gaia] Cwiiis:bug1141500-touch-delay-utility-tray > mozilla-b2g:master

The first commit delays all lengthy work until after the show/hide transition finishes, and fixes up tests. No extra tests as I think the current utility tray tests are comprehensive enough and this doesn't add any new features.

The big change this first commit makes structurally is that now you have UtilityTray.shown and UtilityTray.showing, where the former represents the last complete state of the UtilityTray, and the latter represents the pending state. When the two are the  same, the utility tray is fully open/close or the user is dragging it (so basically the same as the old .shown), but when they differ, it means that the tray is transitioning, and .shown will be the old state, and .showing the pending state.


The second commit works out the swipe velocity and if it stays within sensible bounds, uses that for the animation transition duration, which provides and smoother-looking opening experience. The algorithm to determine swipe velocity is very basic and it errs on the side of caution - in cases of uncertainty, it will use the old default value for the transition time. Seems to work well on a Flame.

Comment 8

[Chris Lord [:cwiiis]]

Nominating for blocking 2.2, as this is a highly visible polish issue in a part of the code that has extensive testing (so is less likely to regress).

Comment 9

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8576671 [details] [review]
[gaia] Cwiiis:bug1141500-touch-delay-utility-tray > mozilla-b2g:master

I hope Michael can review this.

BTW,
introducing intermediate state between shown and hidden of certain UI does look like a general pattern - I hope we could have a general state machine to deal with that in BaseUI just like the transitionController.

Comment 10

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8576671 [details] [review]
[gaia] Cwiiis:bug1141500-touch-delay-utility-tray > mozilla-b2g:master

This feels so much better! I bet the velocity based animation will be even more noticeable on better devices, so this is definitely good for the future too.

I left a couple of questions/comments on github. Also, I noticed that when you touch the top of the screen, wait for the tip of the utility tray to appear, and then release, the little nub will stay at the top. I think this is because there is no animation from onTouchEnd in this case, so afterHide will never get called. Feel free to re-flag me when these issues are addressed.

Comment 11

[Michael Henretty [:mikehenrty][:mhenretty]]

Also, we have some lint errors, and a couple of test failures that look related.

Comment 12

[Gregor Wagner [:gwagner]]

Not blocking the release but lets get it uplifted if possible.

Comment 13

[Chris Lord [:cwiiis]]

Comment on attachment 8576671 [details] [review]
[gaia] Cwiiis:bug1141500-touch-delay-utility-tray > mozilla-b2g:master

I've addressed your comments and fixed the issue you pointed out (thanks!) - I think the tests/linter were actually green, but for whatever reason it's not triggering...

If you wouldn't mind reviewing this and pretending everything is green, I certainly won't land it if it isn't and I'll re-flag if any failures required major changes.

Comment 14

[Chris Lord [:cwiiis]]

No need to pretend now, managed to trigger tests and they're green :)

Comment 15

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8576671 [details] [review]
[gaia] Cwiiis:bug1141500-touch-delay-utility-tray > mozilla-b2g:master

Good work!

Comment 16

[Chris Lord [:cwiiis]]

Not sure if autolander will balk because I just updated the PR (to update the commit shortdescs)... If it does, will re-flag in the morning.

Comment 17

[GitHub Autolander Bot]

Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.

Comment 18

[Chris Lord [:cwiiis]]

darn you autolander for telling me the problem but not how to fix it (why not just append the list to the comment? Or at least a link to it?) Merged manually:

https://github.com/mozilla-b2g/gaia/commit/82976655ee26e4d593d35ba900632623fb656858

Comment 19

[Chris Lord [:cwiiis]]

Comment on attachment 8576671 [details] [review]
[gaia] Cwiiis:bug1141500-touch-delay-utility-tray > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: Jank when opening/closing the utility tray
[Testing completed]: Manual testing and comprehensive automated testing
[Risk to taking this patch] (and alternatives if risky): Slightly above low risk, but nothing has come up yet (after dog-fooding for a while and being on master for a few days)
[String changes made]: None

Comment 20

[bhavana bajaj [:bajaj]]

Pragmatic, can you please help verify this? Thanks!

Comment 21

[Parul Mathur [:pragmatic] - out of office - please ni? [:bajaj] instead]

Verified fixed on master. 

Steps to Reproduce
Swipe open or close the utility tray and lift your finger from the touch screen.

Expected Results
There is no pause before the animation finishes. 

Actual Results
Matches the expected results. The animation is very smooth. 

Test Environment
Gaia-Rev        9b6f3024e4d0e62dd057231f4b14abe1782932ab
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/e730012260a4
Build-ID        20150323010204
Version         39.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150323.043710
FW-Date         Mon Mar 23 04:37:21 EDT 2015
Bootloader      L1TC000118D0


Leaving the 'verifyme' keyword on to verify when it lands on b2g v2.2.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/51813e5cfd63d922c69424c839f1afa65138533d

Comment 23

[Cola, OOO till Oct. 8th]

Attachment: Verify_2.2.3gp

This bug has been verified as pass on latest build of Flame V2.2 & Nexus5 V2.2 by the STR in Comment12.

Actual results:
There is no pause before the animation finishes.

See attachment:Verify_2.2.3gp

Reproduce rate: 0/5

Device: Flame v2.2 build(Pass)
Build ID               20150705162505
Gaia Revision          ea11f422b687a982f0a961c9aea7858066561707
Gaia Date              2015-07-02 23:37:50
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/c0214b4c1ea0
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150705.200108
Firmware Date          Sun Jul  5 20:01:20 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus5 v2.2 build(Pass)
Build ID               20150705160206
Gaia Revision          dc6c18c0dea7af3c40bfff86c530fd877d899dc4
Gaia Date              2015-07-04 01:35:20
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/136c41fca853
Gecko Version          42.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150705.193225
Firmware Date          Sun Jul  5 19:32:44 EDT 2015
Bootloader             HHZ12f