796563 - Add animation when going into and out of browser settings

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Browser, defect, P3)

Description

[GH to BZ]

[GitHub issue by nhirata on 2012-08-28T21:40:08Z, https://github.com/mozilla-b2g/gaia/issues/4052]
## Environment :
Otoro phone, build 2012-08-28
Taken from default.xml in b2g-distro: 
* "platform_build" revision= 76e88b579737d7b5078063dcbec60c2ad2c70465
* "gaia" revision= b3826df518b6df1bd8183333a3c1fe0cb96bd440 
* "mozilla-central" revision= b7fb1238bedd2d419e2c86ce221da53c531ebb3a
* "gonk-misc" revision= a9b6133e492017d34703ae3f24ea1fa9349cbcce

## Repro :
1.  launch browser
2. click on the +
3. click on the gear icon

## Expected :
1. some sort of transition animation between the gear and going back

## Actual :
1. no transition animation for going to settings nor going away from settings.

## Note :

Comment 1

[GH to BZ]

[GitHub comment by autonome on 2012-08-28T22:22:49Z]
Not even close to a blocker.

Comment 2

[GH to BZ]

[GitHub comment by benfrancis on 2012-08-29T15:46:32Z]
Last I heard UX were still debating what the animation should be since the settings icon got moved :) (marked needsUXInput)

Comment 3

[Erwan LE BESCOND]

Attachment: https://github.com/mozilla-b2g/gaia/pull/7945.patch

I use the same animation as the application "communication / contact" for the settings screen ... I do not know if this effect is Gaia compliant ?

I'll let you redirect code review to the right person ...

Comment 4

[Kevin Grandon :kgrandon]

Hi Erwan!

Thanks for the patch, and sorry no one's gotten around to reviewing it. I believe we should ask ben francis for a review, but before we do that - could you make a small update?

We should not need to have the -moz- prefix in these selectors, so if you could remove them, I think we could get this landed soon.

Comment 5

[Ben Francis [:benfrancis]]

UX: Should there be an animation transition when entering browser settings and if so, what? This was never specified.

Comment 6

[Stephany Wilkes]

Reassigning this to Rob. This will be lower on our priority list, though, since it's neither leo nor tef blocking (which is our highest priority at the moment).

Comment 7

[Rob MacDonald [:robmac]]

Sorry, I'm having problems viewing patches on my phone. That said, Contacts uses the correct transition, so if you've used this as a model then we should be fine. 

To confirm, the animation enters from the bottom of the screen and quickly moves up. Closing Settings does the reverse with the screen sliding down.

Comment 8

[Ben Francis [:benfrancis]]

Attachment: Screenshot of browser settings

Thanks for the response Rob. That makes sense when entering settings, but once inside settings there's a back button top left to exit the settings again. Don't you think it would be odd to click an arrow pointing to the left, then for the panel to animate down to the bottom? (See screenshot attached).

Comment 9

[Joshua Smith [:joshua-s]]

Assigning to Erwan as he has a patch already.

Comment 10

[Dale Harvey (:daleharvey)]

Comment on attachment 709660 [details] [diff] [review]
https://github.com/mozilla-b2g/gaia/pull/7945.patch

Clearing Deitrich as the reviewer.

This works good, however when clicking on settings the page screen animates in from the left behind the settings screen animating up from the bottom, we should definitely only have one panel animating at a time, this is a little tricky as the transition both transitions are triggered as part of the same switchScreen call, it should be quite simple to show the settings screen independently of switching global screen, with the added benefit of when we go back from settings we will be on the previously opened tabs menu (whereas previously we went back to the page screen)

The previous message about removing the moz prefix is also right

I also agree with Ben that clicking the back button to animate down is confusing, if we are keeping parity with the contacts app then the back button on the settings panel should be replaced with a 'done' button on the right as in contacts

Aologies again for the delay on this, if you mark any follow up patches with :benfrancis or I :daleharvey as a reviewer it will get looked at asap

(also mention if you dont plan on following up this patch because if so I will take it)

Thanks

Comment 11

[Dale Harvey (:daleharvey)]

Havent heard back in a while, and this is in our current sprint so stealing

Comment 12

[Dale Harvey (:daleharvey)]

Attachment: Add animation when entering and leaving settings

Few things to note, the animation is set to 0.5 seconds which matches other uses of this popup settings screen, also the behaviour has been changed, when you exit settings you will be back on the tab screen (which is where the settings app was opened), having the popup revert back to the screen it was started from makes more sense and also matches the behaviour of its use elsewhere (but will needinfo Ian just to make sure)

Comment 13

[Dale Harvey (:daleharvey)]

Hey Ian, just wanted to run the above changes by you

Comment 14

[Dale Harvey (:daleharvey)]

As for a test plan

I dont think we want to test the specific animation, but I do think the test we should add for this would be to ensure the setting page is visible after pressing the settings button, and that it dissapears when 'Done' is pressed

Comment 15

[Ian Barlow (:ibarlow)]

(In reply to Dale Harvey (:daleharvey) from comment #13)
> Hey Ian, just wanted to run the above changes by you

Works for me.

Comment 16

[Ben Francis [:benfrancis]]

Comment on attachment 779556 [details] [review]
Add animation when entering and leaving settings

The animation feels a bit odd (it's slow and makes the spatial model quite complex), but if Ian is OK with it then I am. Maybe it's just because I'm used to the old design.

There's a bit of a bug in the implementation though, when clicking the "About browser" button. The About page loads in a new tab underneath the settings panel but the settings panel stays in the foreground.

Comment 17

[Ian Barlow (:ibarlow)]

I wouldn't mind trying a test build, but I could use some help as I'm not sure how to actually do that on a b2g device...

Comment 18

[Dale Harvey (:daleharvey)]

if you plug your phone in, make sure you enable remote debugging (Settings -> Device Information -> More Information -> Developer -> Remote Debugging) then

    $ git clone https://github.com/mozilla-b2g/gaia.git gaia
    $ cd gaia
    $ curl https://github.com/mozilla-b2g/gaia/pull/11103.patch | git apply -
    $ NOFTU=1 make reset-gaia

If you phone doesnt boot, ensure you have the latest build (https://pvtbuilds.mozilla.org/pub/mozilla.org/b2g/nightly/mozilla-central-unagi-eng/latest/ for unagi) then redo the make reset-gaia

Comment 19

[Dale Harvey (:daleharvey)]

The animation feels slow for me too, but if we speed it up we should speed it up everywhere and not have different speeds for the same animation, I would suggest if we want to do that to file a follow up, missed the about browser thing thanks, will fix

Comment 20

[Dale Harvey (:daleharvey)]

Comment on attachment 779556 [details] [review]
Add animation when entering and leaving settings

Now hides the setting screen when cicking the 'about web browser' button

Comment 21

[Ben Francis [:benfrancis]]

Comment on attachment 779556 [details] [review]
Add animation when entering and leaving settings

From a technical point of view this code is fine, but I'd feel happier if we could get a UI review before landing.

This animation works fine in the contacts app, but because in the browser the settings panel settings is launched from the side tray (as it is in calendar), the transition feels a little odd. It complicates the spatial model by having a panel slide in from the bottom to overlap a side tray which is already overlapped by the main screen which slides out to the left. Note that in the calendar app there is no animation (as was the previous behaviour in the browser), I think I actually personally preferred that.

As a side note, because of the speed of the animation it's actually physically possible for the user to close the tab tray as the settings panel is animating in. There's no real problem if the user does this, it just feels a bit odd.

Comment 22

[Ian Barlow (:ibarlow)]

Dale, is there a new patch I should be trying here? I tried adding the one from comment 18 and nothing changed in the browser.

Comment 23

[Dale Harvey (:daleharvey)]

nope thats the patch, if you dont see a change then something went wrong, might need someone around locally to take a look

Comment 24

[Dale Harvey (:daleharvey)]

Attachment: 796563-browser.zip

Comment 25

[Ian Barlow (:ibarlow)]

*Finally* got a patch working on my phone \o/

(In reply to Ben Francis [:benfrancis] from comment #21)
> This animation works fine in the contacts app, but because in the browser
> the settings panel settings is launched from the side tray (as it is in
> calendar), the transition feels a little odd. 

I sort of agree, but I also think it's an improvement over what we had before which felt jarring an unfinished. A couple of things to consider that may improve the mental model problem you're describing:

* Put the settings menu somewhere else, like in an overflow menu in the bottom toolbar. It really shouldn't belong in the tabs tray to begin with. 
* Make the tab background colour different from the Settings title bar background colour, to make it more obvious that they are very different parts of the UI
* Explore other transitions for revealing settings (and do this everywhere we access settings), like a crossfade or zoom. 

> It complicates the spatial
> model by having a panel slide in from the bottom to overlap a side tray
> which is already overlapped by the main screen which slides out to the left.
> Note that in the calendar app there is no animation (as was the previous
> behaviour in the browser), I think I actually personally preferred that.

I can see your point, but I feel the opposite, that when comparing the two it feels jarring to enter the calendar settings UI without any kind of screen transition. 

> 
> As a side note, because of the speed of the animation it's actually
> physically possible for the user to close the tab tray as the settings panel
> is animating in. There's no real problem if the user does this, it just
> feels a bit odd.

Is it possible to lock the UI when a user opens Settings? I actually managed to create a new tab after opening settings, so when I backed out I was in the awesomescreen which felt pretty weird.

------

ui-r+, and we can address these other things in follow-up bugs.

Comment 26

[Dale Harvey (:daleharvey)]

Awesome glad you got it working, can hopefully get a flow going for future UX reviews, feel free to bug me on irc if you get stuck with them.

The ability to do things while animations are running is a bit of a global bug, will open a new bug as it should be fixed in a single refactoring.

Comment 27

[Dale Harvey (:daleharvey)]

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

Comment 28

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

"gecko" revision="8c240c67f76c"
"gecko" revision="4dd254baf424523d3256ae47bdc20f9b2b8d5217"
"gaia" revision="b527d7406b4eed3bacdee78597bb4ff21bf9f7d0"
Build ID: 2013-08-13-04-02-22
MC/master build
Unagi

Verified