1055685 - Contacts settings screen animates in too slowly

Status: RESOLVED FIXED

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

Description

[Dietrich Ayala (:dietrich)]

Flame, nightly OTA channel

Feels like it's struggling to make it all the way to the top.

Comment 1

[Dietrich Ayala (:dietrich)]

Requesting UX input. And also Eli for some guidelines on ms response time to feel snappy.

Comment 2

[:Eli Perelman]

If we are speaking strictly that the purpose of the animation is to give a visual indication and UI blockage of the perception of process, then according to our established guidelines, the animation should take no longer 1000ms.

Comment 3

[Stephany Wilkes]

Flagging Przemek on this one while Gordon is out.

Comment 4

[Przemek Abratowski [:przemek] UX]

Can someone clarify what animation I should look at exactly? I'm not sure what the 'Settings Animation' is. Sorry I don't know the exact speed we'd need but I can look at the animation and comment on it.

Comment 5

[Dietrich Ayala (:dietrich)]

Sorry, this was filed in Contacts, but I can see how the bug summary by itself is not very clear :)

STR:

1. open Contacts
2. click on the settings gear icon

Expected: settings screen snappily enters into view, ready to use.

Actual: settings screen sluggishly attempts to crawl into view. Ok, not THAT bad, but it doesn't feel fast.

Comment 6

[Przemek Abratowski [:przemek] UX]

I'm seeing a few issues, when I open the app for the first time the animation just jumps to the final open screen. Reset your phone and open contacts and try this, it looks like the animation doesn't actually play the first time. Then when I close and reopen the settings in contacts it plays the full slide in animation. I think the same issues is going on with the add new contact '+' icon. 

I'm also worried that this is a unique slide up animation that I'm not seeing in other apps doing similar behaviors, I'll bring this up in my UX Frameworks meeting today and get back to this bug on this part.

Comment 7

[Przemek Abratowski [:przemek] UX]

I just discussed this with the Frameworks UX team and we should not be using a slide up animation for either the 'add' or 'settings' behavior, just use the same behavior that calendar uses for "+" add.

Comment 8

[Francisco Jordano [:arcturus] [:francisco]]

I'm sure that we can follow the same strategy of lazy loading within the settings itself.

Right now we are loading all the scripts necessary in any part of settings, but we could do this as well in a lazy way, i.e. loading the sdcard/sim card dependencies when we click in import, and so on.

Comment 9

[Jose Manuel Cantera]

(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #8)
> I'm sure that we can follow the same strategy of lazy loading within the
> settings itself.
> 
> Right now we are loading all the scripts necessary in any part of settings,
> but we could do this as well in a lazy way, i.e. loading the sdcard/sim card
> dependencies when we click in import, and so on.

I'm going to provide a patch as per comment #7. Then we can think off in further improvements

Comment 10

[Jose Manuel Cantera]

Attachment: 24874.html

Comment 11

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8500893 [details]
24874.html

Perfect!! Good solution! 10x

Comment 12

[Francisco Jordano [:arcturus] [:francisco]]

Nice solution!

Comment 13

[Jose Manuel Cantera]

:pabratowski,

please could you have a look at the app and the new transitions? 

thanks!

Comment 14

[Przemek Abratowski [:przemek] UX]

Is the new transition in today's master build?

Comment 15

[Jose Manuel Cantera]

landed in master:

https://github.com/mozilla-b2g/gaia/commit/8ac5a26ca1ba5a8aaae7bf45cc80a88c2d2f8daa

Comment 16

[Jose Manuel Cantera]

(In reply to Przemek Abratowski [:przemek] UX from comment #14)
> Is the new transition in today's master build?

I have just landed. Please provide feedback. We have made use of the standard fade-in, fade-out animations present on the BBs. 

If something has to be changed we can open a follow-up or reopen this one

thanks

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Reverted for the causing the same OSX Gip failures your Gaia Try run had. Please check more carefully before merging in the future.
Master: https://github.com/mozilla-b2g/gaia/commit/8ad2e1c5460edc99f69e0ab28a217cd646514fc9

https://treeherder.mozilla.org/ui/logviewer.html#?job_id=207915&repo=gaia-try

Comment 18

[Jose Manuel Cantera]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #17)
> Reverted for the causing the same OSX Gip failures your Gaia Try run had.
> Please check more carefully before merging in the future.
> Master:
> https://github.com/mozilla-b2g/gaia/commit/
> 8ad2e1c5460edc99f69e0ab28a217cd646514fc9
> 
> https://treeherder.mozilla.org/ui/logviewer.html#?job_id=207915&repo=gaia-try

Hi,

I have rerun the tests here https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=75b728dcf305
and the same issue happens in OSX. I don't think it has something to do with my patch. ni Zacc

Comment 19

[Zac C (:zac)]

Well I think it does have something to do with your patch because these tests are green on the rest of Gaia-Try.

Comment 20

[Zac C (:zac)]

Had a closer look at this.

In the first 4 cases it is the step after tap_edit button (of a contact) that has failed. This suggests to me that the tap_edit has failed.

I noticed that the wait for the location in EditContact class (that is line 132) to be 'y' == 0, however it's possible that the location is 0 even when the element is behind another element and this resolves true immediately, then the tap executes but it executes on the wrong element. A race condition, basically. I have see this behaviour in other apps.

I'm not entirely sure on the solution (until I build Gaia myself) but in this case it might be more suitable to wait for the element you are tapping to have disappeared rather than waiting for the new element to fade in as we know that might be a bit racey.

For example you might add into the tap_edit method, after the tap but before returning the class,
self.wait_for_element_not_displayed(*self._edit_contact_button_locator)

It's probably the same for the tests that are failing on the Settings steps too.

Comment 21

[Jose Manuel Cantera]

https://github.com/mozilla-b2g/gaia/commit/b0befba79285e5931c91c900736c16439349b100

Comment 22

[Jose Manuel Cantera]

the new animation is landed and in accordance to the BB animations. UX can open a bug if they find it not appropriate or if they request any improvement. 

thanks