1026180 - [vertical homescreen] Dragging an icon to the top of the screen should activate at the bottom of edit header

Status: VERIFIED FIXED

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

Description

[Jacqueline Savory [:jsavory] UX]

While dragging an icon to the top of the screen, currently the top scroll only activates at the very top of the screen which overlaps with the edit mode header and can be awkward to use. 

When the user drags an icon to the top of the screen it should accelerate near the bottom of the edit mode header instead.

Comment 1

[Chris Lord [:cwiiis]]

I'll take this.

Comment 2

[Chris Lord [:cwiiis]]

Attachment: Activate scrolling at bottom of edit header

Comment 3

[Kevin Grandon :kgrandon]

Comment on attachment 8441413 [details] [review]
Activate scrolling at bottom of edit header

Clearing review for now, just had one question - is it possible to do this without an explicit method like setEditHeaderMargin? Exposing these methods on the gaia-grid seems like it could get out of control quickly, so it would be good to explore alternatives if we can.

Also we just landed bug 1026166 which moves the edit header outside of .scrollable - not sure if that changes the approach at all. I'm wondering if we actually revert that, and possibly change the size of gaia-grid if necessary?

Or alternatively the edit mode header is about the same size as the current rocketbar. Could we perhaps modify the movement heuristics based on offsetTop of the gaia-grid?

Comment 4

[Chris Lord [:cwiiis]]

(In reply to Kevin Grandon :kgrandon from comment #3)
> Comment on attachment 8441413 [details] [review]
> Activate scrolling at bottom of edit header
> 
> Clearing review for now, just had one question - is it possible to do this
> without an explicit method like setEditHeaderMargin? Exposing these methods
> on the gaia-grid seems like it could get out of control quickly, so it would
> be good to explore alternatives if we can.
> 
> Also we just landed bug 1026166 which moves the edit header outside of
> .scrollable - not sure if that changes the approach at all. I'm wondering if
> we actually revert that, and possibly change the size of gaia-grid if
> necessary?
> 
> Or alternatively the edit mode header is about the same size as the current
> rocketbar. Could we perhaps modify the movement heuristics based on
> offsetTop of the gaia-grid?

So I'm not too keen on having a method like this, but unless the header is actually inside the grid, I don't see how we can infer it. We can't use offsetTop, as the grid could be placed arbitrarily in a flowing container and if it was, offsetTop could end up not being what we expect.

An alternative that I considered was to infer it from either style padding or margin - which we'd still need to set like we have (as the edit header doesn't have an explicit css height), but would mean we don't need to expose a method.

The problem then is any adverse effect they may have on the layout - we may have to compensate for that in grid_view, but it wouldn't be anything too drastic. I'm not hugely keen on this method either though, as it's co-opting the meaning of the style and the behaviour would be unexpected.

Another, not-so-different alternative would be to expose a different method, something like setHeaderElement (or even just have it as a property), which might be a little cleaner than setting the size explicitly.

Comment 5

[Chris Lord [:cwiiis]]

Comment on attachment 8441413 [details] [review]
Activate scrolling at bottom of edit header

Made proposed change and re-noming for review as discussed.

Comment 6

[Kevin Grandon :kgrandon]

Comment on attachment 8441413 [details] [review]
Activate scrolling at bottom of edit header

Going to pass this off to James due to being swamped in reviews today. It would be good to get a set of fresh eyes onto whether or not we should have a setEditHeaderElement method.

I'm actually wondering if the proper thing to do isn't to move this into the component. James - thoughts?

Comment 7

[James Lal [:lightsofapollo]]

Comment on attachment 8441413 [details] [review]
Activate scrolling at bottom of edit header

Looks good but see my comment about adding a comment

@kgrandon We may want to figure out how to split up the grid to make more sense but I don't think we can do that until we figure out all the things it supposed to be doing... So for now lets put it in there.

Comment 8

[Chris Lord [:cwiiis]]

Merged to master: http://git.io/GpL44A

Comment 9

[Kevin Grandon :kgrandon]

Going to block 989848 so we can track uplifts easier.

Comment 10

[Kevin Grandon :kgrandon]

Comment on attachment 8441413 [details] [review]
Activate scrolling at bottom of edit header

This is needed for the vertical homescreen.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/9bcbc3ad31aba651691f8700059bdd0ee0927b59

Comment 12

[jlorenzo (disabled)]

I guess we need more a UX feedback for that fix. Would you mind having a look at it, Jaqueline?

Comment 13

[Jacqueline Savory [:jsavory] UX]

I've taken a look at this on master and didn't encounter any issues. Looks good to me!

Comment 14

[Shally Li（leave from Mozilla）]

This bug has been successfully verified on latest Flame v2.0.
See attachment: verified_v2.0.mp4
Reproduce rate: 0/5

STR:
1.Long tap blank area of Homescreen to edit mode.
2.Drag a icon to the top of Homescreen.
**The icon can be dragged nearby or on the Edit mode header.

Flame 2.0 build:
Gaia-Rev        2989f2b2bd12fcc0e9c017d2db766e76a55873b8
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/4048cc627fdf
Build-ID        20150129000205
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150129.040447
FW-Date         Thu Jan 29 04:0

Comment 15

[Shally Li（leave from Mozilla）]

Attachment: verified_v2.0.MP4