1082675 - (App-grouping) Enable app-grouping by default

Status: VERIFIED FIXED

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

Description

[Chris Lord [:cwiiis]]

We're currently maintaining two home-screen experiences; with and without app-grouping. Assuming we want app-grouping in 2.2, it would be sensible to enable this by default and perhaps even remove the option to disable it sooner rather than later.

The design is still being iterated on, but at this point, the changes aren't particularly radical.

Opening this bug to start discussion and track the issues that are stopping us from enabling it.

Comment 1

[Maria Sandberg [:mushi]]

Thanks Chris. I think we've given the team a good amount of time to review and give feedback on this. 

The UX team has requested to evaluate a version with Hung's updates (no group names, left alignment etc). Flagging Rob and Peter/Wilfred here to weigh in on whether we can move grouping to default or if we'd like to wait until the new version has been tried out.

Comment 2

[Wilfred Mathanaraj [:WDM]]

How long would it take to do a review of the version with the updates? if its just few days lets do that review and then make it default.

Comment 3

[Peter Dolanjski [:pdol]]

(In reply to Wilfred Mathanaraj [:WDM] from comment #2)
> How long would it take to do a review of the version with the updates? if
> its just few days lets do that review and then make it default.

I agree.  If we're going to change it very soon, let's just wait until the change.  If, however, that'll take a few weeks, let's just enable it.

Comment 4

[Chris Lord [:cwiiis]]

Actually, I think we should block this on the animations patch landing, otherwise all the transitions are super-janky and I'd rather not give a bad first impression...

Comment 5

[Maria Sandberg [:mushi]]

Looks like we are close to a go-ahead to enable by default from the UX side. Chris, do you still think it's worth blocking on bug 927349? It seems up in the air how long it will take to get that one fixed. 

Normally I would agree with waiting until the animations patch but at this point I'm inclined to just enable it so we can get more people playing around with the feature. If we expect the patch to take longer than a week or so I say enable by default, and we'll write up an email to dev-gaia explaining the jank. Let's meet early next week for the final call.

Comment 6

[Kevin Grandon :kgrandon]

+1 for turning this on now. I would like to see us fix bug 1091007 first before enabling this by default as that's currently a bit janky. I'm currently swamped with blockers, but I can try to grab it next week if you don't get to it Chris.

Comment 7

[Chris Lord [:cwiiis]]

Yes, I suppose blocking on bug 927349 might potentially delay this quite a bit, so perhaps we shouldn't. Let's go ahead (I'll fix bug 1091007 tomorrow) without it, but I think your suggestion of posting about the jank is a good idea.

Comment 8

[Chris Lord [:cwiiis]]

Attachment: Enable app-grouping permanently

This removes the setting and enables app-grouping by default.

I'll flag this for ui-review as soon as I know the right person to flag :)

Comment 9

[Chris Lord [:cwiiis]]

Comment on attachment 8519980 [details] [review]
Enable app-grouping permanently

Falling for ui-review, to make sure that we're ready to do this.

Comment 10

[Kevin Grandon :kgrandon]

Comment on attachment 8519980 [details] [review]
Enable app-grouping permanently

This is awesome! The code looks good, but there's a few failing marionette tests that I'd like to see addressed before doing the review cycle. Since we're deprecating the setting it looks like we'll need to fix-up the tests to take that into account.

I looked at one failing test, and it looked like it was long-pressing on a divider to trigger the long-press dialog. I'd imagine that this is one of the trickier situations to solve. It may be easy if we somehow create a group with only a single app result, then you could long press on an area with no apps. Anyway, take a look if you can and re-flag me when needed. Thanks!

Comment 11

[Chris Lord [:cwiiis]]

Fixed tests locally, waiting on green try before flagging.

Comment 12

[Chris Lord [:cwiiis]]

There's one more failing python test, but it's exposed an actual bug... If it's small, I'll fix it as part of this.

Comment 13

[Hung Nguyen]

Comment on attachment 8519980 [details] [review]
Enable app-grouping permanently

Took a look at the latest master and aside from some minor potential improvements, I think we should move forward with making this default.

Comment 14

[Chris Lord [:cwiiis]]

Comment on attachment 8519980 [details] [review]
Enable app-grouping permanently

I think this will be green now... https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=81afd17b5871

Comment 15

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8519980 [details] [review]
Enable app-grouping permanently

Very clean and excellent job, thanks

Comment 16

[Kevin Grandon :kgrandon]

Comment on attachment 8519980 [details] [review]
Enable app-grouping permanently

Looks good to me. Let's do this!

Comment 17

[Chris Lord [:cwiiis]]

Merged! https://github.com/mozilla-b2g/gaia/commit/d0bc392a83fad3dd15d8f614581fcc4f36d4adbc

There was a failure in f1 to do with notifications, but I'm going to assume that's something else (this doesn't touch anything near that...)

Fingers crossed this sticks...

Comment 18

[Gregor Wagner [:gwagner]]

(In reply to Chris Lord [:cwiiis] from comment #17)
> Merged!

\o/

Comment 19

[Yeojin Chung [:YeojinC]]

This issue is verified fixed on Flame Master.

Result: App-grouping is enabled by default. 
 
Device: Flame Master (319mb, full flash)
Build ID: 20150209010211
Gaia: 0d7b35f23402c4cb29bca6b98280fec48a196dec
Gecko: 3436787a82d0
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0