Kevin Grandon :kgrandon Reporter
	Description • 10 years ago

STR:
- Have app grouping enabled and last group on home screen collapsed.
- Install an app from marketplace.
- Formatting of last group will be messed up.

	Chris Lord [:cwiiis] Assignee
	Comment 1 • 10 years ago

Will fix this tomorrow.

	Chris Lord [:cwiiis] Assignee
	Comment 2 • 10 years ago

We no longer use placeholders past the last divider to create new sections, so this simplifies things a bit and removes removeUntilDivider.

	Kevin Grandon :kgrandon Reporter
	Comment 3 • 10 years ago

Comment on attachment 8515034 [details]
Don't append apps to a collapsed group when installing

Code looks fine, but I have one concern about what happens when the home screen gets killed. Left a comment on github, re-flag me when ready. Thanks!

	Chris Lord [:cwiiis] Assignee
	Comment 4 • 10 years ago

Comment on attachment 8515034 [details]
Don't append apps to a collapsed group when installing

So you're right, I think we can just take the add-to-expanded-group path every time, the initial load in works via synchronising first, then any apps that weren't in the verticalhome store will go via the addIconToGrid path (and any apps that aren't in the store are ones that were installed while the homescreen was killed)

It bears out in testing locally, and if it's incorrect, I really hope automated testing catches it :p

	Kevin Grandon :kgrandon Reporter
	Comment 5 • 10 years ago

Comment on attachment 8515034 [details]
Don't append apps to a collapsed group when installing

Hey Chris - did you push a new version? When I install an app after manually triggering an OOM kill, I still see them getting appended to the collapsed groups. The same goes for bookmarks - I guess they should have the same logic during an OOM kill?

I believe this is going into the synchronize path here: https://github.com/Cwiiis/gaia/blob/bug1091007-install-collapsed-group/apps/verticalhome/js/sources/application.js#L191

Perhaps we should consider removing the second argument to addIconToGrid()?

	Chris Lord [:cwiiis] Assignee
	Comment 6 • 10 years ago

Comment on attachment 8515034 [details]
Don't append apps to a collapsed group when installing

damnit, forgot to actually add the changed files...

	Kevin Grandon :kgrandon Reporter
	Comment 7 • 10 years ago

Comment on attachment 8515034 [details]
Don't append apps to a collapsed group when installing

Looks great now that the changes are in. Thanks!

	Kevin Grandon :kgrandon Reporter
	Comment 8 • 10 years ago

In master: https://github.com/mozilla-b2g/gaia/commit/06ff917623f0a15aa51ca18fc681225782f7f647

	Hung Nguyen
	Comment 9 • 10 years ago

I'm not sure if this is related but as of today's master, when you install an app with the last group collapsed, it will create a new group and place the newly installed app in it. If the last group is opened, the newly app will go in there instead.

This behavior continues if the last group is collapsed so you could end up with new group every time you install a new app (with the last group collapsed). 

I think this approach is a bit unpredictable and was wondering if we would consider a simpler design? Maybe have a pseudo "downloads" group that appears at the last position when something is downloaded/bookmarked? 

I'm not sure what the solution is but I would like to get Katie's feedback on it.

	katieC
	Comment 10 • 10 years ago

hey Hung,

I just reviewed Maria's spec (pages 13-14). The spec'd behaviour:

>If the last group is collapsed a new group is created and the icon is placed as the first icon in the new group. 

>If the last group is expanded the new icon is placed in the next available slot in the unnamed group.

As for the scenario you mention Hung, only if the user collapses the last group, then adds a new app from marketplace, will a new group will be created. That said, I agree, this could create a lot of unnecessary new groups, if every time the user downloads an app from Marketplace (or stars a web app or bookmarks a site). I'll mock something up to discuss in our next sfe sync.

Chris - Is it possible to detect if the last group has been made by the system (downloads/bookmarks) or made by the user (in edit mode)?

	Chris Lord [:cwiiis] Assignee
	Comment 11 • 10 years ago

(In reply to katieC from comment #10)
> Chris - Is it possible to detect if the last group has been made by the
> system (downloads/bookmarks) or made by the user (in edit mode)?

I'm not sure what this would consist of? If an app install creates a new group, when does that group switch over to being a user-created group - when you collapse it? Or when you add new apps to it? Never? What if the user moves the group between other groups? Or moves it, then moves it back?

We could put a marker on groups to give them a particular designation, but I don't know what set of circumstances would be sensible to do this in.

As to the original comment, I don't think there's really a problem here. You'd only end up with lots of groups if you installed lots of apps one-by-one, collapsing the newly-created group between each install. Why would you do that? I don't think this could happen by accident...

	katieC
	Comment 12 • 10 years ago

Based on comment 9 + some UX review... New general rule for adding new apps to homescreen:
New apps (hosted/packaged/bookmarks/web apps) are always added to the last group in the next available slot. If the last group is collapsed, expand group to show the user where new app has been added.

Ni'ing Chris to make the update. Thanks!

	Chris Lord [:cwiiis] Assignee
	Comment 13 • 10 years ago

Filed bug 1102220 to track this change.

	Brogan Zumwalt [Inactive]
	Comment 14 • 10 years ago

Issue verified fixed on Flame 3.0

When user installs app from Marketplace, it behaves correctly in regards to app grouping on homescreen regardless of whether the group it is placed (the last group available on homescreen) in is collapsed or expanded. If the group is collapsed, the group expands and the new icon is added to the previously collapsed group on return to homescreen. If the last available group was already expanded, it stays expanded. If all app groups are collapsed, only the app grouping where the app was added to is expanded, the rest of the groups remain collapsed. All of these were checked on LTR as well as RTL languages.

Device: Flame 3.0 Master
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