1120877 - Homescreen menu has no transition

Status: VERIFIED FIXED

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

Description

[Chris Lord [:cwiiis]]

Long-pressing in a blank area on the home-screen brings up a menu, where you can access settings/change the background/add a smart collection.

This menu just appears instantly, rather than with a transition, like the majority of other dialogs in FxOS.

I'm going to assume the transition should be the same as the very similar menu in the system browser.

This is an easily fixed bit of polish that can be encountered very easily by the user, so suggesting blocking status.

Comment 1

[Gregor Wagner [:gwagner]]

We are already shipping with this defect. Lets treat it as a polish bug but we wouldn't hold the release for it.

Comment 2

[GitHub Autolander Bot]

Attachment: [gaia] Cwiiis:bug1120877-gaia-menu-transition > mozilla-b2g:master

Comment 3

[Chris Lord [:cwiiis]]

Comment on attachment 8563131 [details] [review]
[gaia] Cwiiis:bug1120877-gaia-menu-transition > mozilla-b2g:master

Just CSS, and one small JS change - the JS change is because we immediately call show after lazy-loading the element, so the transition doesn't happen if we don't delay it somehow. I tried to add some hacky style flushing in the element itself, but couldn't get anything to work :(

Collection is the only other user of GaiaMenu it seems, and it doesn't lazy-load its context menu, so requires no change.

The transition uses the same style as the shared option_menu script/style.

Comment 4

[Kevin Grandon :kgrandon]

Comment on attachment 8563131 [details] [review]
[gaia] Cwiiis:bug1120877-gaia-menu-transition > mozilla-b2g:master

A nice bit of polish. Thank you!

Comment 5

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/b7045dc6e013f691816b64be855fcabbf3363ff3

Comment 6

[Kevin Grandon :kgrandon]

Chris - do we want to uplift this to 2.2?

Comment 7

[Chris Lord [:cwiiis]]

Comment on attachment 8563131 [details] [review]
[gaia] Cwiiis:bug1120877-gaia-menu-transition > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: Context menu appears and disappears with no transition on homescreen and in collections
[Testing completed]: Basically just a CSS change, tested manually in both situations GaiaMenu is used
[Risk to taking this patch] (and alternatives if risky): Low/no risk, verified both uses of GaiaMenu are still functioning correctly (except now with transitions)
[String changes made]: None

Comment 8

[Chris Lord [:cwiiis]]

(In reply to Kevin Grandon :kgrandon from comment #6)
> Chris - do we want to uplift this to 2.2?

(yes :))

Comment 9

[Kevin Grandon :kgrandon]

Comment on attachment 8563131 [details] [review]
[gaia] Cwiiis:bug1120877-gaia-menu-transition > mozilla-b2g:master

Clearing for now.

Comment 10

[Kevin Grandon :kgrandon]

Turns out this broke unit tests, and I didn't check gaia-try before landing, because it seems it didn't run on this pull request.  This is my fault, sorry about that. In the future autolander will catch this. Backed out: https://github.com/mozilla-b2g/gaia/commit/a30df6905faf0f4c103d6779deda83ee425f3d07

http://ftp.mozilla.org/pub/mozilla.org/b2g/tinderbox-builds/b2g-inbound-linux64_gecko/1423736860/b2g-inbound_ubuntu64_vm-b2gdt_test-gaia-unit-bm54-tests1-linux64-build1087.txt.gz

Chris - Could you address/re-land this?

Comment 11

[Kevin Grandon :kgrandon]

Sinon fake timers can probably easily fix this if added to: contextmenu_handler_test.js

I think you just need to add: this.sinon.clock.tick();

Comment 12

[GitHub Autolander Bot]

Attachment: [gaia] Cwiiis:bug1120877-gaia-menu-transition > mozilla-b2g:master

Comment 13

[Chris Lord [:cwiiis]]

Comment on attachment 8563351 [details] [review]
[gaia] Cwiiis:bug1120877-gaia-menu-transition > mozilla-b2g:master

Carrying review, waiting for tests to complete.

Comment 14

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/bcaa78a55cb8b85caed553405fa830aa8796f5d6

Comment 15

[Chris Lord [:cwiiis]]

Comment on attachment 8563351 [details] [review]
[gaia] Cwiiis:bug1120877-gaia-menu-transition > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: Context menu appears and disappears with no transition on homescreen and in collections
[Testing completed]: Basically just a CSS change, tested manually in both situations GaiaMenu is used
[Risk to taking this patch] (and alternatives if risky): Low/no risk, verified both uses of GaiaMenu are still functioning correctly (except now with transitions)
[String changes made]: None

Comment 16

[Ryan VanderMeulen [:RyanVM]]

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

Comment 17

[Shally Li（leave from Mozilla）]

This bug has been verified as "pass" on latest Nightly build of Flame v2.2&master by the STR in Comment 0.

Actual results: This context menu appears with a transition on homescreen and in collections.
See attachment: verified_2.2&master.3gp
Reproduce rate: 0/15


Device: Flame v2.2 (Verified) 
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: Flame master (Verified)
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            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150705.193055
Firmware Date          Sun Jul  5 19:31:07 EDT 2015
Bootloader             L1TC000118D0

Comment 18

[Shally Li（leave from Mozilla）]

Attachment: verified_v2.2&master.3gp