1074798 - (app-grouping) Make creating new groups match spec

Status: RESOLVED FIXED

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

Description

[Chris Lord [:cwiiis]]

Currently creating new groups is a bit fiddly as the groups pop in after a short hover over an undetermined area. In the spec, there are indicators for where you should drag to create a new group, which animate to illustrate what will happen.

Filing this bug to deal with making this match spec.

Comment 1

[Chris Lord [:cwiiis]]

Attachment: Make group dragging behaviour more closely match spec

This overhauls group dragging behaviour to make it match spec (and is generally much nicer). I wouldn't be hugely surprised if some tests break, I'll fix them as results come in.

Comment 2

[Chris Lord [:cwiiis]]

Comment on attachment 8497498 [details] [review]
Make group dragging behaviour more closely match spec

I'm happy with whoever gets to the review first - I don't think it's so significant that it needs two reviews.

Comment 3

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8497498 [details] [review]
Make group dragging behaviour more closely match spec

I left a couple of contacts but great work as always! thanks a lot

Comment 4

[Kevin Grandon :kgrandon]

Comment on attachment 8497498 [details] [review]
Make group dragging behaviour more closely match spec

I'm not completely against landing yet, but I noticed a few behavior problems when playing with this. I've only tested it on a nexus 4, so I'm not sure if it's a problem with the resolution difference, but it's stuff we should probably fix. I can provide screenshots of these if it would help. 

A few of the things I've noticed with the drop indicators:

- Sometimes one can get stuck to the bottom of the group I'm dragging.
- Sometimes it goes into the "active" state when I'm dragging a group over one, I'd imagine that shouldn't be the case.
- When I'm dragging a group, should these show up?
- It seems like there's a regression with scrolling in edit mode. It's hard to describe, but I have difficulty scrolling to the top of the page now after entering edit mode. I don't have this problem in master.

Overall I think this is a step in the right direction, but it's a bit more buggy than I would like to land. I do understand the desire to move quickly though, so if you'd like to squash this and land with Cristian's review I'm fine with that, or I can also take a look tomorrow if there's been some fixes here.

Comment 5

[Chris Lord [:cwiiis]]

(In reply to Kevin Grandon :kgrandon from comment #4)
> - Sometimes one can get stuck to the bottom of the group I'm dragging.

Ends up this is caused by the disabling of horizontal overflow... I only did this because I noticed and it was annoying me, but it's really a separate bug - if I can't fix it, I'll just omit this patch.

> - Sometimes it goes into the "active" state when I'm dragging a group over
> one, I'd imagine that shouldn't be the case.

Over one...? Not sure what this is - the group should only go active (blue) when dropping it would put it down in that place (so it goes blue if you drag over where it originally was, at the top, at the bottom or over a divider) - I'll double-check all of this.

> - When I'm dragging a group, should these show up?

Do you mean the blue dividers? If so, yes (that's what the spec dictates) - they grow when they're valid drop targets. Maybe they should only go blue at all when they're valid, but that's something to be amended in the design :)

> - It seems like there's a regression with scrolling in edit mode. It's hard
> to describe, but I have difficulty scrolling to the top of the page now
> after entering edit mode. I don't have this problem in master.

Again, the overflow thing. Will fix.

> Overall I think this is a step in the right direction, but it's a bit more
> buggy than I would like to land. I do understand the desire to move quickly
> though, so if you'd like to squash this and land with Cristian's review I'm
> fine with that, or I can also take a look tomorrow if there's been some
> fixes here.

I'll fix the overflow issues, wait for another set of green tests, then land. If I can't fix the overflow stuff quickly, I'll just land without it - it's a pre-existing bug anyway.

Comment 6

[Chris Lord [:cwiiis]]

Overflow thing fixed, comments addressed, merged: https://github.com/mozilla-b2g/gaia/commit/17af3b55663fab02f4f2b9104d917dec4cf7020f