1028688 - Upon dragging an app into a Collection icon - no indication that app has been added

Status: VERIFIED FIXED

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

Description

[Ran Ben Aharon [:ranbena] EverythingMe]

There are 2 causes to this:

1. The newly added app is added to the end of the pinned app list. Should be first item. Pinned web results should remain being added last. (this is how it works in v1.4)

2. The icon doesn't get updated when the app is added to the Collection. This might be either because the icon update event hasn't been triggered, or that it's data is faulty (though opening the Collection does display the newly added app)

Comment 1

[Ran Ben Aharon [:ranbena] EverythingMe]

Jason, this is a regression and I think should be reconsidered as a blocker.

Comment 2

[Kevin Grandon :kgrandon]

I think the icon update will be fixed in bug 1028690, but the pinning order should be fairly trivial. I'll implement it real quick.

Comment 3

[Kevin Grandon :kgrandon]

I'll take this and fix it, but let's just request uplift here.

Comment 4

[Kevin Grandon :kgrandon]

Attachment: Github pull request

Hey Ran - could you give this a review? It turns out we need a *lot* of stuff to render the icon. I would greatly like to simplify/refactor this in the future, but for now I think this should work.

Comment 5

[Ran Ben Aharon [:ranbena] EverythingMe]

Works very well. I'm stumped by the amount of code :/

A few things I noticed:

1. If the dropped app is already pinned, there's no feedback at all. (drag "Messages" into "Social"). I think the app should be moved to top of pinned apps, and therefore the icon will have an indication.

2. Dragging into never before opened preinstalled Collection, yields the default background in the icon. I think it should be addressed but in a separate bug.

3. Dragging into never before opened preinstalled Collection that has no predefined pinned apps, yields an icon with only the dragged app. This is not different from v1.4 so perhaps we can let it go for v2.0.

Comment 6

[Ran Ben Aharon [:ranbena] EverythingMe]

Comment on attachment 8447757 [details] [review]
Github pull request

Plussing in order to not delay landing. Going on PTO.

Comment 7

[Jason Smith [:jsmith]]

Okay. Makes sense to me.

Comment 8

[Kevin Grandon :kgrandon]

(In reply to Ran Ben Aharon (Everything.me) from comment #6)
> Comment on attachment 8447757 [details] [review]
> Github pull request
> 
> Plussing in order to not delay landing. Going on PTO.

Thanks. I will try to address your comments in this patch if they are easy enough to accomplish, otherwise I will file bugs for them.

Comment 9

[Kevin Grandon :kgrandon]

Fixed issues in comment #5 and landed: https://github.com/mozilla-b2g/gaia/commit/7a067ea1cb4d0eafd5780988ff4617d78b7e5b3e

I had to sneak in a few more changes, but nothing too risky. Let me know if you have any feedback if you look at this before you leave or after you're back. Thanks!

Comment 10

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/e1dab6cffaae5f79427d0a8f4dac7400656c9682

Comment 11

[SandKing]

This issue has been verified successfully on Flame2.0&2.1
Verify video:"verify_1028688.mp4".

Flame2.0 build
Gaia-Rev        856863962362030174bae4e03d59c3ebbc182473
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/e40fe21e37f1
Build-ID        20141208000206
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141208.035628
FW-Date         Mon Dec  8 03:56:38 EST 2014
Bootloader      L1TC00011880

Flame 2.1 build:
Gaia-Rev        38e17b0219cbc50a4ad6f51101898f89e513a552
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/8b92c4b8f59a
Build-ID        20141205001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141205.035305
FW-Date         Fri Dec  5 03:53:16 EST 2014
Bootloader      L1TC00011880

Comment 12

[SandKing]

Attachment: Verified video: verify_1028688.MP4