1022414 - [Vertical] Icon does not drop properly during edit mode

Status: VERIFIED FIXED

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

Description

[Kevin Grandon :kgrandon]

It appears that we have a bunch of bugs related to icons and them not dropping in the desired location. It's highly likely that a single patch will solve all of them, so creating a bug to track that work and will re-shift some bugs to clear up the dependency tree.

Johan/Jason - let me know if you have any problems with this approach, thanks.

Comment 1

[Jason Smith [:jsmith]]

(In reply to Kevin Grandon :kgrandon from comment #0)
> It appears that we have a bunch of bugs related to icons and them not
> dropping in the desired location. It's highly likely that a single patch
> will solve all of them, so creating a bug to track that work and will
> re-shift some bugs to clear up the dependency tree.
> 
> Johan/Jason - let me know if you have any problems with this approach,
> thanks.

Sounds fine to me - I'll nom this one to block instead of the dependencies to simplify this as well.

Comment 2

[Kevin Grandon :kgrandon]

Attachment: Github pull request

Just looking for a review for this dragdrop stuff. I also want to write some marionette tests to cover this stuff, but it seems a bit tricky - I'll work with you on that soon James.

Cwiiis - It's likely that this could conflict a lot with your patch in bug 1022713. While I do want to see these performance fixes land, it's super important that we get the "stuck icon" thing solved. This patch seems to do that by using min/max to get from and to, then buffering that a bit in the case of divider creation/deletion.

Comment 3

[Cristian Rodriguez (:crdlc)]

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

TBH I don't see anything wrong although the buffer code seems a bit tricky. I bet  to remove the "from" and "to" parameters right now and we could work on investigating what happens here and also to obtain the proper indexes. But if you want to land it there is no problem at all for me

Comment 4

[Kevin Grandon :kgrandon]

The from/to parameters will be tricky to remove without a rewrite of a lot of the system due to the way that placeholders are created/removed and dividers are removed. Let's land for now and we can always optimize later. Thanks for the review.

Comment 5

[Kevin Grandon :kgrandon]

Landed: https://github.com/mozilla-b2g/gaia/commit/b752637348f182116c5fb2ae2596f9b89959def8

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Needs a branch patch for v2.0 uplift (or approval on what deps it has).

Comment 8

[Kevin Grandon :kgrandon]

Thanks. I'm requesting approval on previous patches that are also necessary.

Comment 9

[Kevin Grandon :kgrandon]

Uplifted: https://github.com/mozilla-b2g/gaia/commit/f04e3e92f50141fec73e74800b6f3fc6d84fee34

Comment 10

[Lancy(Leave from Mozilla)]

Attachment: Verify_Video_Flame.MP4

This issue has been verified successfully on Flame 2.0 & 2.1.
See attachment: Verify_Video_Flame.MP4
Reproducing rate: 0/10

Flame v2.0 version:
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c756bd8bf3c3
Build-ID        20141201000201
Version         32.0

Flame v2.1 version:
Gaia-Rev        ccb49abe412c978a4045f0c75abff534372716c4
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/18fb67530b22
Build-ID        20141201001201
Version         34.0