Closed Bug 1143226 (fastandfurious) Opened 5 years ago Closed 5 years ago

animations throughout Gaia combine for a generally sluggish user experience

Categories

(Firefox OS Graveyard :: Gaia, defect)

x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dietrich, Assigned: dietrich)

References

()

Details

(Keywords: perf)

Attachments

(1 file, 6 obsolete files)

The animations throughout the system feel very slow and heavyweight after long periods of heavy use.

As an experiment, I reduced all animations across Gaia greater than 0.2s down to 0.2s.

The result feels very snappy and SO much nicer to use throughout the day. My tasks feel so much more continuous.

Patch tested on Flame and Sony Z3 Compact.

The high-end device is great for identifying where we're slow because of enforced slowness vs hardware restriction. Turns out we felt slow in many places and it was not due to resource constraints - it was intentional in the design and/or implementation.

I want to test on a lower spec device next.

I haven't tested every single one of the changes, but wanted to put this up here to get feedback on the idea.

There are a few other places I haven't quite nailed yet - like the transition from app to homescreen after hitting the home button. Feels like ~200ms delay between hitting the button and the app starting to close. So more work still to do.
Attached file pull request (obsolete) —
Comment on attachment 8577522 [details] [review]
pull request

Requesting some feedback from folks.

Any reason to not do this?
Flags: needinfo?(felash)
Attachment #8577522 - Flags: feedback?(eperelman)
Comment on attachment 8577522 [details] [review]
pull request

I like the approach of making animations shorter or non-existent. The only thing to be wary for: is the purpose of the animation to gloss over other processing taking place? For example, is the animation being run while some other longer running background processing taking place? I'd prefer to have a longer animation to cover up the fact that I can't interact yet rather than a quick animation where I still need to wait for something to load or the screen is frozen.
Attachment #8577522 - Flags: feedback?(eperelman) → feedback+
Thanks Eli. You're right that we shouldn't change the animations that are purposefully long to convey progress on a longer running process of some kind. I tried to keep an eye out for that in this patch, but will do a thorough review prior to requesting final code review.
Yeah I think it's a good idea.

We could have a look at animations too. For example I tried to make the notifications animation faster in bug 1137613 (but I can't find why the Gij test sometimes fails so I haven't landed it yet).
Flags: needinfo?(felash)
Great, thans for the feedback Julien. I'll move on to doing a closer look at each change in the patch next.

Who is the right person for final review?
I'd ask review from a peer for each app. For such a patch it should be quite quick to review, I think. But they're the best to test properly edge cases in their respective apps.
Autolander is broken, so Kevin manually did a try run and looks like fields of green: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=395e10bb3e8c18fe51bece02d9937b29eaa5322b
Unrotted.
Attachment #8577522 - Attachment is obsolete: true
Attachment #8580959 - Attachment is obsolete: true
Attachment #8580967 - Attachment is obsolete: true
Attachment #8581956 - Attachment is obsolete: true
Comment on attachment 8589918 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/29072

Review requests:

* Miller for Calendar
* Etienne for Callscreen
* Wilson for Camera
* Marcus for Clock
* Kevin for Collections
* Etienne for Comms
* Salva for Usage
* James for Email
* Tim for FM Radio
* David for Gallery
* Tim for Keyboard
* Jim for Ringtones
* Evelyn for Settings
* Julien for SMS
* Fernando for FXA
* Tim for Lockscreen
* Tim for System
* Kevin for Verticalhome
* David for Video
* Gabriele for WAP push
* David for Shared
Attachment #8589918 - Flags: review?(mmedeiros)
Attachment #8589918 - Flags: review?(etienne)
Attachment #8589918 - Flags: review?(wilsonpage)
Attachment #8589918 - Flags: review?(m)
Attachment #8589918 - Flags: review?(kgrandon)
Attachment #8589918 - Flags: review?(salva)
Attachment #8589918 - Flags: review?(jrburke)
Attachment #8589918 - Flags: review?(m) → review+
Attachment #8589918 - Flags: review?(timdream)
Attachment #8589918 - Flags: review?(dflanagan)
Nice work, few questions:

1 - Have we had anyone on UX play with the before/after yet? Should we flag them?
2 - Have we considered putting 0.2s into some CSS variable global file, for ease of configuration/changing at a later date?
Flags: needinfo?(dietrich)
Attachment #8589918 - Flags: review?(squibblyflabbetydoo)
Flags: needinfo?(dietrich)
Attachment #8589918 - Flags: review?(ehung)
Attachment #8589918 - Flags: review?(felash)
Attachment #8589918 - Flags: review?(ferjmoreno)
Attachment #8589918 - Flags: review?(gsvelto)
(In reply to Kevin Grandon :kgrandon from comment #14)
> Nice work, few questions:
> 
> 1 - Have we had anyone on UX play with the before/after yet? Should we flag
> them?

No, I'll ask for UX feedback from Gordon, who directed our approach WRT responsiveness.

> 2 - Have we considered putting 0.2s into some CSS variable global file, for
> ease of configuration/changing at a later date?

I'd love that, but would prefer not to block these changes on that happening.
Comment on attachment 8589918 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/29072

Hi Gordon, can you give your thoughts on this system-wide change to reduce animation durations to 0.2s?
Attachment #8589918 - Flags: feedback?(gbrander)
Attachment #8589918 - Flags: review?(dflanagan) → review+
Note, per earlier discussion I confirmed that no changes here are part of any animation that is forced to be longer than 0.2s for a specific reason... that I could determine. The reviewers will surely let us know though!
(In reply to Dietrich Ayala (:dietrich) from comment #15)
> (In reply to Kevin Grandon :kgrandon from comment #14)
> > 2 - Have we considered putting 0.2s into some CSS variable global file, for
> > ease of configuration/changing at a later date?
> 
> I'd love that, but would prefer not to block these changes on that happening.

I think that it would be a shame to land without using a CSS file, but if we do, let's certainly follow-up asap. I can probably help with this if needed.
Depends on: 1152591
Attachment #8589918 - Flags: review?(kgrandon) → review+
Comment on attachment 8589918 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/29072

Please get ui-review+ and correct the commit message before landing.
Attachment #8589918 - Flags: review?(timdream) → review+
Comment on attachment 8589918 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/29072

Code looks good, but I'm hesitant about this landing without some tests as to whether these are actually the best numbers, instead of merely less-bad...
Attachment #8589918 - Flags: review?(squibblyflabbetydoo) → review+
Attachment #8589918 - Flags: review?(ferjmoreno) → review+
Comment on attachment 8589918 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/29072

LGTM in contacts.
Attachment #8589918 - Flags: review?(francisco) → review+
Comment on attachment 8589918 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/29072

You'll need to rebase, camera and gallery seems to have moved.

I'm not convinced by the change in the SMS app; there is quite a lag before the 2 animations (likely caused by a reflow behind the scene) and so the lag + a quick animation makes me feel uneasy.

Here are the 2 behaviors:
* panel sliding: just press "new message" or open a thread, then go back, etc. There is a noticeable delay before the animation starts. Maybe this is because I have lots of data though.
* "select message" panel: open a message thread, press the "..." top right button, press "select messages" => look at animation. Then press the cross and look at the animation again.

I'm keeping the r? until you try it and tell me what you think.
I'll also ui-review our Designer Fang for these 2 behaviors.


And on a side note, Dietrich, you can ask review to several people in one go by using a comma in the review text input :)
Attachment #8589918 - Flags: ui-review?(fshih)
Attachment #8589918 - Flags: review?(mmedeiros) → review+
Comment on attachment 8589918 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/29072

r+ for email, as long as ux signs off on it.
Attachment #8589918 - Flags: review?(jrburke) → review+
Attachment #8589918 - Flags: review?(ehung) → review+
Comment on attachment 8589918 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/29072

R+ for the wappush, communications/dialer and callscreen apps which are the parts I'm a peer/owner.
Attachment #8589918 - Flags: review?(gsvelto) → review+
Comment on attachment 8589918 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/29072

Nice one mate!

I flashed my personal phone with your patch, big improvement! Slide between panels on Settings still looked a little slow, has that been touched?
Attachment #8589918 - Flags: review?(wilsonpage) → review+
Comment on attachment 8589918 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/29072

Wow, Cost Control looks very fast when you open settings. Although the change is harmless I would want to take into account an opinion from the UX team.

Juwei, do you agree on this change or could you ping the person in charge of Gaia's animations? Just to let them know. Thank you.
Attachment #8589918 - Flags: review?(salva)
Attachment #8589918 - Flags: review+
Attachment #8589918 - Flags: feedback?(jhuang)
Need info Przemek to check the overall animation.
Flags: needinfo?(pabratowski)
Attachment #8589918 - Flags: feedback?(jhuang) → feedback+
Hi Juwei, I believe Amy was looking into the animations across the 2.0 OS a little while ago.
Flags: needinfo?(pabratowski) → needinfo?(amlee)
(In reply to Julien Wajsberg [:julienw] (PTO April 6th) from comment #23)
>
> And on a side note, Dietrich, you can ask review to several people in one go
> by using a comma in the review text input :)

WOW I wish I had known that before...
Updated to tip, and converted to use the css var kgrandon added in bug 1152591.
Assignee: nobody → dietrich
Attachment #8589918 - Attachment is obsolete: true
Attachment #8593775 - Attachment is obsolete: true
Attachment #8589918 - Flags: ui-review?(fshih)
Attachment #8589918 - Flags: review?(felash)
Attachment #8589918 - Flags: feedback?(gbrander)
(In reply to Jim Porter (:squib) from comment #20)
> Comment on attachment 8589918 [details] [review]
> Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/29072
>
> Code looks good, but I'm hesitant about this landing without some tests as
> to whether these are actually the best numbers, instead of merely less-bad...

This is the transition for showing/hiding the toast acknowledging that you've created and saved a new ringtone.

There are other (excruciating) bugs for minimizing how long that toast lives and whether it should be modal, however the CSS here is solely determining the length of that toast entry/exit.

In my tests it now animates in quickly, and out quickly, but the lifespan of its complete visibility is unchanged.

(In reply to Etienne Segonzac (:etienne) from comment #21)
> Comment on attachment 8589918 [details] [review]
> Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/29072
>
> Forwarding to Francisco for the Contacts change.
>
> For the system part, please update the following JS constants related to the
> transition durations:

Done!

(In reply to Julien Wajsberg [:julienw] (PTO April 6th) from comment #23)
> I'm not convinced by the change in the SMS app; there is quite a lag before
> the 2 animations (likely caused by a reflow behind the scene) and so the lag
> + a quick animation makes me feel uneasy.
>
> Here are the 2 behaviors:
> * panel sliding: just press "new message" or open a thread, then go back,
> etc. There is a noticeable delay before the animation starts. Maybe this is
> because I have lots of data though.

I tested this, and with no profile data I do actually see a tiny delay. I think that delay does not make the animation speedup a bad effect. You should definitely fix that delay.

> * "select message" panel: open a message thread, press the "..." top right
> button, press "select messages" => look at animation. Then press the cross
> and look at the animation again.

I very much like how it animates in and out faster now. Especially the animate in, because you have to wait for our modal alert too. This makes the user wait needlessly for less time.

I’m curious about the web activities modal. I don’t think I changed that, and it feels slow still. Will ask Fabrice.

(In reply to Wilson Page [:wilsonpage] from comment #26)
> I flashed my personal phone with your patch, big improvement! Slide between
> panels on Settings still looked a little slow, has that been touched?

Good catch! Must be in JS somewhere… looking now. There’s a delay before the animation too, ughgh.
Flags: needinfo?(squibblyflabbetydoo)
Flags: needinfo?(felash)
Dietrich, I think the more there is data, the more there is the delay. I have a lot of data. Can you try again with (for example) "make reference-workload-high" ?
Flags: needinfo?(felash)
Attachment #8593777 - Flags: ui-review?(epang)
Comment on attachment 8593777 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/29562

Took a look overall and it looks good to me! R+
Flagging Amy so she's aware of this and reviews when she returns from PTO.  Amy, if you see anything that needs refinement please open a follow up bug.  Thanks!
Attachment #8593777 - Flags: ui-review?(epang) → ui-review+
(In reply to Dietrich Ayala (:dietrich) from comment #33)
> This is the transition for showing/hiding the toast acknowledging that
> you've created and saved a new ringtone.
> 
> There are other (excruciating) bugs for minimizing how long that toast lives
> and whether it should be modal, however the CSS here is solely determining
> the length of that toast entry/exit.
> 
> In my tests it now animates in quickly, and out quickly, but the lifespan of
> its complete visibility is unchanged.

Well, I'm not a UX person so I'm not sure that the number is "right" (maybe the transition should be even faster, for example), but I suppose I don't actually care one way or the other.
Flags: needinfo?(squibblyflabbetydoo)
(In reply to Jim Porter (:squib) from comment #36)
> Well, I'm not a UX person so I'm not sure that the number is "right" (maybe
> the transition should be even faster, for example), but I suppose I don't
> actually care one way or the other.

Yeah, exactly! I don't know for sure either, but it definitely feels nicer in daily use for me.

I'm working with the UX team to design guidelines so that we can figure out what the optimal defaults should be for transitions and animations like this.
Comment on attachment 8593777 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/29562

Hi, 

If a new guideline is being worked on for transitions I think this is a good fix for now.
Flags: needinfo?(amlee)
Thanks Amy! Who would be developing that guideline - you and Patryk? How can others get involved?

I think the only thing blocking now is to figure some differences in results in app opening that Omega found. I'm waiting to hear what his build configuration is, so I can try to reproduce.

Also, my blog post on Hacks about this was published today. https://hacks.mozilla.org/2015/04/firefox-os-animations-the-dark-cubic-bezier-of-the-soul/
(In reply to Dietrich Ayala (:dietrich) from comment #39)
> Thanks Amy! Who would be developing that guideline - you and Patryk? How can
> others get involved?
> 
> I think the only thing blocking now is to figure some differences in results
> in app opening that Omega found. I'm waiting to hear what his build
> configuration is, so I can try to reproduce.
> 
> Also, my blog post on Hacks about this was published today.
> https://hacks.mozilla.org/2015/04/firefox-os-animations-the-dark-cubic-
> bezier-of-the-soul/

Hi Dietrich, 

From my understanding we are not currently at that stage yet. NI Przemek if he can provide more input.
Flags: needinfo?(pabratowski)
Sorry, I still object the specific changes in the SMS app, I don't think they bring a better experience, at least with my workload. Especially the panel slide transition.

This is not really a big deal because this slide transition will move away with the new architecture anyway (and I think we can make it .2s with the new architecutre).
I found something related that might interest you: between v2.1 and v2.2 we "fixed" how the dialogs are appearing in the SMS app; in v2.1 they were appearing asap but now we use an animation. And since project silk we wait for a reflow before doing the animation.

The result is that it's now a lot slower to show all dialogs in SMS than it used to be in previous versions. Before we were waiting for a reflow, now we wait for a reflow + the animation. If we can make this animation even shorter than .2s I'd be happy :)
(In reply to Julien Wajsberg [:julienw] (PTO -> Apr 27) from comment #43)
> If we can make this animation even
> shorter than .2s I'd be happy :)

Sure, what value did you find was optimal?
Flags: needinfo?(felash)
(In reply to Julien Wajsberg [:julienw] (PTO -> Apr 27) from comment #34)
> Dietrich, I think the more there is data, the more there is the delay. I
> have a lot of data. Can you try again with (for example) "make
> reference-workload-high" ?

On master, doing "APP=sms make reference-workload-heavy" breaks SMS app - no messages, and compose/settings buttons don't do anything. It's working for you?
Why don't you just give choice for users. I.e. for me it is nice to see animations ... for first time 'omg, what a nice animations are there!' and then turn them off to work more effectively and faster. Those animations delays user work. I agree that there must be done some work to optimize them but it would be nice also to just turn them off.
(In reply to mac from comment #46)
> Why don't you just give choice for users. I.e. for me it is nice to see
> animations ... for first time 'omg, what a nice animations are there!' and
> then turn them off to work more effectively and faster. Those animations
> delays user work. I agree that there must be done some work to optimize them
> but it would be nice also to just turn them off.

This should be possible since the animation variable now lives inside of a theme. Users can replace the default theme with a new theme with any animation value, including 0 - for instant/no animations.
(In reply to Dietrich Ayala (:dietrich) from comment #45)
> (In reply to Julien Wajsberg [:julienw] (PTO -> Apr 27) from comment #34)
> > Dietrich, I think the more there is data, the more there is the delay. I
> > have a lot of data. Can you try again with (for example) "make
> > reference-workload-high" ?
> 
> On master, doing "APP=sms make reference-workload-heavy" breaks SMS app - no
> messages, and compose/settings buttons don't do anything. It's working for
> you?

Ah yeah, I've seen this issue, I need to fix it. I bet you have issues about "ActorParent" in the console.

The "fix" is to "adb shell rm -rf /data/local/storage/permanent/chrome/idb/*ssm*" before running the command. I'll file a separate bug for this later.
(In reply to Dietrich Ayala (:dietrich) from comment #44)
> (In reply to Julien Wajsberg [:julienw] (PTO -> Apr 27) from comment #43)
> > If we can make this animation even
> > shorter than .2s I'd be happy :)
> 
> Sure, what value did you find was optimal?

I tried various values and .1s looks actually weird. So let's keep 0.2ms for this.

Also when I compare against v2.1 (I check using the option menu displayed when tapping the top right icon in a conversation) I think the delay before the animation kicks in is really longer than it should. If we wait for the reflow, well, the reflow time should be approximately the time needed to display the menu in v2.1... So the time to have a user feedback (either displaying the full menu in v2.1 or starting the animation in master) should be the same. But it's not.

In master, there is ~700ms before the animation even starts. In v2.1 I think the menu appears in less than 200ms. I think we should profile this, but this is likely in a separate bug.
Flags: needinfo?(felash)
Tests passed, so pushed:

https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=9d19ae7dd104fd253a8b47f06adacd8cdc0d2281

https://github.com/mozilla-b2g/gaia/commit/07a1a20b86931ee9911b16df94df60a178825bb2

If you notice places where animations are still slow, or are now too abrupt, file a new bug.

Thanks everyone for all the help!
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Blocks: 1160661
Hey Dietrich, where did you get any r+ for the SMS part ?

I know you were eager to land this but this is not how things should happen.

Is there any bug filed for the additional issues I outlined in comment 49?
Flags: needinfo?(pabratowski)
(In reply to Julien Wajsberg [:julienw] (PTO May 1st) from comment #51)
> Hey Dietrich, where did you get any r+ for the SMS part ?
> 
> I know you were eager to land this but this is not how things should happen.
> 
> Is there any bug filed for the additional issues I outlined in comment 49?

Whoa, if this is the case, I completely misunderstood you, and please accept my apology. I am quite aware of the process, and did not try to circumvent it.

Comment #49 concluded that you were a-ok with 200ms, which is what this patch does. If I misunderstood, I'm glad to back out the SMS portion of the patch immediately. I'll ping you on IRC to figure out what you would like to do.
Cleared it up on IRC, leaving as-is for now.
Depends on: 1161481
Depends on: 1161167
No longer depends on: 1161167
Depends on: 1162264
Depends on: 1178553
Depends on: 1193172
Depends on: 1198143
You need to log in before you can comment on or make changes to this bug.