Closed
Bug 1082675
Opened 10 years ago
Closed 10 years ago
(App-grouping) Enable app-grouping by default
Categories
(Firefox OS Graveyard :: Gaia::Homescreen, defect)
Tracking
(b2g-master verified)
VERIFIED
FIXED
2.1 S9 (21Nov)
Tracking | Status | |
---|---|---|
b2g-master | --- | verified |
People
(Reporter: cwiiis, Assigned: cwiiis)
References
Details
(Whiteboard: [systemsfe])
Attachments
(1 file)
(deleted),
text/x-github-pull-request
|
crdlc
:
review+
kgrandon
:
review+
kcaldwell
:
ui-review+
hnguyen
:
ui-review+
|
Details |
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•10 years ago
|
||
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.
Flags: needinfo?(wmathanaraj)
Flags: needinfo?(rmacdonald)
Flags: needinfo?(pdolanjski)
Comment 2•10 years ago
|
||
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.
Flags: needinfo?(wmathanaraj)
Comment 3•10 years ago
|
||
(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.
Flags: needinfo?(pdolanjski)
Assignee | ||
Comment 4•10 years ago
|
||
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...
Depends on: 927349
Assignee | ||
Updated•10 years ago
|
Blocks: app-grouping
Comment 5•10 years ago
|
||
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•10 years ago
|
||
+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.
Assignee | ||
Comment 7•10 years ago
|
||
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.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → chrislord.net
Status: NEW → ASSIGNED
Assignee | ||
Comment 8•10 years ago
|
||
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 :)
Attachment #8519980 -
Flags: review?(kgrandon)
Attachment #8519980 -
Flags: review?(crdlc)
Assignee | ||
Comment 9•10 years ago
|
||
Comment on attachment 8519980 [details]
Enable app-grouping permanently
Falling for ui-review, to make sure that we're ready to do this.
Attachment #8519980 -
Flags: ui-review?(kcaldwell)
Comment 10•10 years ago
|
||
Comment on attachment 8519980 [details]
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!
Attachment #8519980 -
Flags: review?(kgrandon)
Attachment #8519980 -
Flags: review?(crdlc)
Assignee | ||
Comment 11•10 years ago
|
||
Fixed tests locally, waiting on green try before flagging.
Updated•10 years ago
|
Attachment #8519980 -
Flags: ui-review?(hnguyen)
Assignee | ||
Comment 12•10 years ago
|
||
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•10 years ago
|
||
Comment on attachment 8519980 [details]
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.
Attachment #8519980 -
Flags: ui-review?(hnguyen) → ui-review+
Attachment #8519980 -
Flags: ui-review?(kcaldwell) → ui-review+
Assignee | ||
Comment 14•10 years ago
|
||
Comment on attachment 8519980 [details]
Enable app-grouping permanently
I think this will be green now... https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=81afd17b5871
Attachment #8519980 -
Flags: review?(kgrandon)
Attachment #8519980 -
Flags: review?(crdlc)
Comment 15•10 years ago
|
||
Comment on attachment 8519980 [details]
Enable app-grouping permanently
Very clean and excellent job, thanks
Attachment #8519980 -
Flags: review?(crdlc) → review+
Comment 16•10 years ago
|
||
Comment on attachment 8519980 [details]
Enable app-grouping permanently
Looks good to me. Let's do this!
Attachment #8519980 -
Flags: review?(kgrandon) → review+
Assignee | ||
Comment 17•10 years ago
|
||
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...
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 18•10 years ago
|
||
(In reply to Chris Lord [:cwiiis] from comment #17)
> Merged!
\o/
Updated•10 years ago
|
Whiteboard: [systemsfe]
Target Milestone: --- → 2.1 S9 (21Nov)
Updated•10 years ago
|
Flags: needinfo?(rmacdonald)
Comment 19•10 years ago
|
||
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
status-b2g-master:
--- → verified
Flags: needinfo?(ktucker)
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in
before you can comment on or make changes to this bug.
Description
•