1054239 - [system][notifications-tray] Swiping up anywhere on the open tray should close

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System::Status bar, Utility tray, Notification, defect)

Description

[Wilson Page [:wilsonpage]]

It should be as easy as possible to close the notifications tray, but the touch target is currently very small; requiring two hands or repositioning of hand to reach.

The Android tray can be closed by swiping up anywhere for minimal effort.

VIDEO: https://www.youtube.com/watch?v=9oxfAaP2B-M

Comment 1

[Wilson Page [:wilsonpage]]

Chris can you add you latest thoughts on this. I'm happy to look into this and take it up if it's quickie :)

Comment 2

[Chris Lord [:cwiiis]]

Would be nice - I think you could get the desired effect by just adding the right element to this list: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/utility_tray.js#L57

But there may be issues when dragging over notifications? Might need some finessing (you could look at the event object and make sure that the original target is correct, I think?)

Should be pretty quick to fix, go for it :)

Comment 3

[Wilson Page [:wilsonpage]]

Attachment: pull-request (master)

I may have overlooked some stuff, but seems to work well to me.

I make sure that the tray doesn't move when the user is touching buttons, list-items, or notifications.

Do I need to add tests somewhere?

Comment 4

[Chris Lord [:cwiiis]]

Comment on attachment 8620484 [details] [review]
pull-request (master)

Left a comment on github - approach looks sound to me. I'm not on the approved reviewers list though, so you may want to seek someone else's review (Alive?)

As for tests, I think adding a marionette test to make sure you can drag the tray closed using the background would be nice (or failing that, a unit test - though I imagine the marionette test may actually be easier to write in this case).

Comment 5

[Wilson Page [:wilsonpage]]

Comment on attachment 8620484 [details] [review]
pull-request (master)

Etienne are you able to take a look at this?

Comment 6

[Wilson Page [:wilsonpage]]

Comment on attachment 8620484 [details] [review]
pull-request (master)

Moving review over to gmarty as he is familiar with this area.

Comment 7

[Chris Lord [:cwiiis]]

(In reply to Wilson Page [:wilsonpage] from comment #6)
> Comment on attachment 8620484 [details] [review]
> pull-request (master)
> 
> Moving review over to gmarty as he is familiar with this area.

Don't forget to add that test before you commit this :)

Comment 8

[Guillaume Marty [:gmarty]]

Comment on attachment 8620484 [details] [review]
pull-request (master)

Welcome to the sysfe team Wilson :-P

This patch looks good to me but as it is quite complex, I'd also ask for a second reviewer. Etienne, what do you think?

Comment 9

[Etienne Segonzac (:etienne)]

Comment on attachment 8620484 [details] [review]
pull-request (master)

I randomly stumbled on a pretty significant issue while testing on device so not setting r+ just yet :)
But this is a welcomed and clean change.

We might be able to add a marionette test for the issue with the rocketbar and the bad transition-duration. But in any case I think we should add a basic marionette test where we swipeUp() from an empty notification area.

Comment 12

[Etienne Segonzac (:etienne)]

(In reply to Etienne Segonzac (:etienne) from comment #9)
> But in any case I think we should add a
> basic marionette test where we swipeUp() from an empty notification area.

Drive by comment: we still need to add this test :)

Comment 13

[Alberto Pastor [:albertopq]]

Comment on attachment 8620484 [details] [review]
pull-request (master)

Wow, great job! Just a couple of comments:

- It would be a nice to have to transform the utility tray to a BaseUI module, now that we are cleaning it up. Happy to do it in a follow up.

- I saw a small bug: when the utility tray is active, if you tap anywhere in the overlay, it gets stuck some pixels from the bottom

- Another issue: I saw that the icons in the statusbar weren't updating correctly, and I know we are pausing/resuming the statusbar with the utilitytraywillshow/abort events. If you start dragging down and then stop in the middle for a while, you can see a [object DOMError] in the console and the utiliytrayabortopen never gets fired.

Flag me again when those issues are solved and I'll take another look. Thanks!

Comment 14

[Alberto Pastor [:albertopq]]

Comment on attachment 8620484 [details] [review]
pull-request (master)

r=me making sure that the tests that are failing in the PR, are failing in master as well. If they are intermitents, let's have 1 green at least. Thanks!

Comment 15

[Wilson Page [:wilsonpage]]

We have one failing unit-test related to preventDefault() on 'mousedown' events. Alberto can you please comment on what the intended behaviour is here. From earlier investigations we seemed to fine 'mouse' events only impacted b2g-desktop.

I feel like we may be safe to remove the 'keyboardimeswitcher' logic from utility_tray.js entirely.

https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/utility_tray.js#L231

Comment 16

[Alberto Pastor [:albertopq]]

I'm not too familiar with that part of the code, but I think you are right. Let's remove that part of the code, making sure that the feature added in bug 893560, still works. Thanks!

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/21256d7665f972255d198f8af81a8df4bd0e0fc4

Comment 18

[Shally Li（leave from Mozilla）]

This bug has been verified as "pass" on the latest build of Flame KK 2.5 and Aires KK 2.5 by the STR in comment 0.

Actual results: Users can drag the notification panel up from any empty area or bottom, and notification bar shows normally.
See attachment: verified_Flame_v2.5.3gp
Reproduce rate: 0/15


Device: Flame KK 2.5 (Pass)
Build ID               20150812150204
Gaia Revision          52f3ea58df38e5427f6afeb636bc6ad01d24022f
Gaia Date              2015-08-12 16:40:43
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/cf932fc931dcd19f425934db79bec641ebe2a8a9
Gecko Version          43.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150812.182303
Firmware Date          Wed Aug 12 18:23:12 EDT 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Aries KK 2.5(Pass)
Build ID               20150812230643
Gaia Revision          52f3ea58df38e5427f6afeb636bc6ad01d24022f
Gaia Date              2015-08-12 16:40:43
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/7649ffe28b67aa2dad0f67ea01500c0ff91b2bac
Gecko Version          43.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150812.222959
Firmware Date          Wed Aug 12 22:30:07 UTC 2015
Bootloader             s1

Comment 19

[Ed Morley [:emorley]]

Thank you Wilson - the UX is really nice now :-)

Comment 20

[Shally Li（leave from Mozilla）]

Attachment: verified_Flame_v2.5.3gp