1120624 - [Homescreen] When a Smart Collection is added to a collapsed group, group should expand. SC should be added to the specific group where user invoked the action menu (via long press).

Status: VERIFIED FIXED

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

Description

[KTucker [:KTucker][Inactive 3/4/2016]]

Attachment: SmartCollectionAppGroupingLogcat.txt

When the user creates a new smart collection it is added to the last group but the collapsed group does not expand. This is inconsistent with the expected behavior of installing an app from the Marketplace. 

Repro Steps:
1)  Updated Flame to Build ID: 20150112010228
2)  On the homescreen collapse all the groups. 
3)  Tap and hold your finger on an empty part of the screen.
4)  Tap "Add Smart Collection" and add a collection to the homescreen.
5)  Observe the collapsed groups on the homescreen.

Actual:
The created smart collection is added to the last group but the last group is not automatically expanded.

Expected:
The last group is expanded after adding a new smart collection to indicate to the user that is was created.

Environmental Variables
Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash)
Build ID: 20150112010228
Gecko: https://hg.mozilla.org/mozilla-central/rev/bb8d6034f5f2
Gaia: f5e481d4caf9ffa561720a6fc9cf521a28bd8439
Platform Version: 37.0a1
Firmware: V18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Notes:
Repro frequency: 5/5 
See attached: video, logcat

Comment 1

[KTucker [:KTucker][Inactive 3/4/2016]]

This issue does not occur on the Flame 2.1 because the collapsing and expanding of groups did not exist.

Comment 2

[Peter Bylenga [:PBylenga]]

Nominating to block, inconsistent behavior leading to poor UX, installation is not immediately shown (confirmed).

Comment 3

[Gregor Wagner [:gwagner]]

Is this behavior expected?

Comment 4

[katieC]

Yes, this is the expected behaviour. 

In the above scenario, when the Smart Collection is added, the last group (originally in a collapsed state) should expand to show the user that the Smart Collection has successfully been added to the home screen. 

Please note: If the user initiates a long press (on any blank/empty space) within any group (collapsed or expanded, it invokes the Action Menu. User taps 'Add Smart Collections'. Once the user selects the Collection(s) they'd like to add and taps 'OK', the Smart Collection is added to the group where the long press originated. If the group was originally in a collapsed state, the group should expand (and possibly scroll) to reveal the added Smart Collection into focus (viewable screen area), confirming to the user that the selected Smart Collection has successfully been added to their home screen.

Comment 5

[Chris Lord [:cwiiis]]

(We triaged this and decided it wasn't a blocker)

Comment 6

[katieC]

Updated bug title to ensure comment 4 is included (and understood) in bug fix.

Comment 7

[GitHub Autolander Bot]

Attachment: [PullReq] Cwiiis:bug1120624-expand-group-when-adding-collection to mozilla-b2g:master

Comment 8

[Chris Lord [:cwiiis]]

Comment on attachment 8559235 [details] [review]
[PullReq] Cwiiis:bug1120624-expand-group-when-adding-collection to mozilla-b2g:master

I guess the positioning of added collections has been broken for a while? Either that or I broke it in bug 1121149? Anyway, this fixes that and also makes sure to expand the group a collection gets inserted in.

I didn't write a test for this for two reasons:
1- It's pretty much impossible to write marionette tests that involve the app grid and scrolling because our simulator ends up scrolling the grid while you drag icons around
2- UX (and myself) are pretty keen to kill collections at some point and it feels bad to spend significant time writing a test that will soon be obsoleted.

In fairness, probably not the best of justifications, but I'd appreciate a pass this time :)

Comment 9

[Kevin Grandon :kgrandon]

Comment on attachment 8559235 [details] [review]
[PullReq] Cwiiis:bug1120624-expand-group-when-adding-collection to mozilla-b2g:master

This seems to be working well for me, and I think I'm fine skipping the test for the reasons you stated. Nice work.

Comment 10

[Chris Lord [:cwiiis]]

Merged to master: https://github.com/mozilla-b2g/gaia/commit/b08beea578e47d64714ac46b270ffb641c74cbde

Comment 11

[Hermes Cheng (inactive after July 27, 2015)]

Is this going to uplift to 2.2?

Comment 12

[Chris Lord [:cwiiis]]

(In reply to Hermes Cheng[:hermescheng] from comment #11)
> Is this going to uplift to 2.2?

Yes, but I'd like it to sit on master for a day or two to make sure there are no obvious issues.

Comment 13

[katieC]

This is working well, one small question -  when I add a Smart Collection to a collapsed or expanded group, the app name appears first, then the icon appears. Is there a way to have them appear at the same time?

Comment 14

[Chris Lord [:cwiiis]]

(In reply to katieC (last day: Feb 05, 2015) from comment #13)
> This is working well, one small question -  when I add a Smart Collection to
> a collapsed or expanded group, the app name appears first, then the icon
> appears. Is there a way to have them appear at the same time?

I don't know about this code, so I can't really answer authoritatively - I expect the icon is generated asynchronously(?) If that's the case, I'd have thought we could delay actually adding until the icon is generated, though that might be a bit confusing - I wonder if we could display a placeholder until generation completes too?

Comment 15

[Joshua Mitchell (Inactive)]

Verified fixed on the latest Nightly Flame 3.0 build

Results: The collapsed group will expand automatically when adding a smart collection

Device: Flame 3.0 (KK - Nightly - 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

Comment 16

[Chris Lord [:cwiiis]]

Comment on attachment 8559235 [details] [review]
[PullReq] Cwiiis:bug1120624-expand-group-when-adding-collection to mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: Confusing behaviour when adding collections to homescreen
[Testing completed]: Manual testing, been on master for a week, verified by QA
[Risk to taking this patch] (and alternatives if risky): Low risk of behaviour being broken in a different way (but testing looks good)
[String changes made]: None

Comment 17

[Tiffanie Shakespeare [:tif]]

Hey guys! Something I've noticed with the auto expand behaviour is that it doesn't scroll so that the icon is then visible on the screen (if it is not). Should I file another bug?

STR
1. Have one group with many apps (a screen full) in it
2. Make sure the group is collapsed
3. Press & hold to get the button "Add Smart Collections"
4. Select 1 or more items to add

Expected:
Group expands and scrolls to bottom of group

Actual:
Group expands, but I do not see the newly added Smart Collection

Comment 18

[Chris Lord [:cwiiis]]

(In reply to Tiffanie Shakespeare from comment #17)
> Hey guys! Something I've noticed with the auto expand behaviour is that it
> doesn't scroll so that the icon is then visible on the screen (if it is
> not). Should I file another bug?
> 
> STR
> 1. Have one group with many apps (a screen full) in it
> 2. Make sure the group is collapsed
> 3. Press & hold to get the button "Add Smart Collections"
> 4. Select 1 or more items to add
> 
> Expected:
> Group expands and scrolls to bottom of group
> 
> Actual:
> Group expands, but I do not see the newly added Smart Collection

This is quite similar to bug 1119650 and bug 1121036, but I suppose is dissimilar enough to have another bug to track it - it wouldn't hurt anyway :)

Comment 19

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/a575289ad8983ea1b39ffaad16b17ae20f1f2aeb

Comment 20

[Tiffanie Shakespeare [:tif]]

Follow up Bug 1132235

Comment 21

[Hermes Cheng (inactive after July 27, 2015)]

verify 2.2

Build ID               20150310162504
Gaia Revision          5af6f8d5d6161dea02002634c6d0a570a122e5dd
Gaia Date              2015-03-10 19:17:12
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/ec87adb8cf13
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150310.200728
Firmware Date          Tue Mar 10 20:07:39 EDT 2015
Bootloader             L1TC000118D0