Closed Bug 1026180 Opened 5 years ago Closed 5 years ago

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

Categories

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

x86
macOS
defect
Not set

Tracking

(b2g-v1.4 unaffected, b2g-v2.0 verified, b2g-v2.1 verified)

VERIFIED FIXED
2.0 S5 (4july)
Tracking Status
b2g-v1.4 --- unaffected
b2g-v2.0 --- verified
b2g-v2.1 --- verified

People

(Reporter: jsavory, Assigned: cwiiis)

References

Details

(Whiteboard: [systemsfe])

Attachments

(2 files)

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.
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking-]
I'll take this.
Assignee: nobody → chrislord.net
Status: NEW → ASSIGNED
Attachment #8441413 - Flags: review?(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?
Attachment #8441413 - Flags: review?(kgrandon)
(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.
Whiteboard: [systemsfe]
Target Milestone: --- → 2.0 S4 (20june)
Comment on attachment 8441413 [details] [review]
Activate scrolling at bottom of edit header

Made proposed change and re-noming for review as discussed.
Attachment #8441413 - Flags: review?(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?
Attachment #8441413 - Flags: review?(kgrandon) → review?(jlal)
Target Milestone: 2.0 S4 (20june) → 2.0 S5 (4july)
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.
Attachment #8441413 - Flags: review?(jlal) → review+
Merged to master: http://git.io/GpL44A
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Going to block 989848 so we can track uplifts easier.
Blocks: vertical-homescreen
No longer blocks: vertical-home-next
Comment on attachment 8441413 [details] [review]
Activate scrolling at bottom of edit header

This is needed for the vertical homescreen.
Attachment #8441413 - Flags: approval-gaia-v2.0?(bbajaj)
Flags: needinfo?(jlorenzo)
Keywords: verifyme
Attachment #8441413 - Flags: approval-gaia-v2.0?(bbajaj) → approval-gaia-v2.0+
I guess we need more a UX feedback for that fix. Would you mind having a look at it, Jaqueline?
Flags: needinfo?(jlorenzo) → needinfo?(jsavory)
I've taken a look at this on master and didn't encounter any issues. Looks good to me!
Flags: needinfo?(jsavory)
Status: RESOLVED → VERIFIED
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
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking-] → [VH-FL-blocking-][VH-FC-blocking-][MGSEI-Triage+]
You need to log in before you can comment on or make changes to this bug.