Closed Bug 1028688 Opened 5 years ago Closed 5 years ago
Upon dragging an app into a Collection icon - no indication that app has been added
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)
Jason, this is a regression and I think should be reconsidered as a blocker.
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.
I'll take this and fix it, but let's just request uplift here.
Assignee: nobody → kgrandon
Target Milestone: --- → 2.0 S5 (4july)
Status: NEW → ASSIGNED
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.
Attachment #8447757 - Flags: review?(ran)
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 on attachment 8447757 [details] [review] Github pull request Plussing in order to not delay landing. Going on PTO.
Attachment #8447757 - Flags: review?(ran) → review+
Okay. Makes sense to me.
(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.
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!
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
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
You need to log in before you can comment on or make changes to this bug.