1118311 - (App-grouping) Dragging an expanded group offsets the dragged group incorrectly

Status: VERIFIED FIXED

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

Description

[Chris Lord [:cwiiis]]

If you long-press an expanded group to drag it, it ends up offset a long way from the user's finger.

This is a regression caused by bug 1092327.

Comment 1

[Chris Lord [:cwiiis]]

Attachment: Fix dragging expanded groups

This fixes the touch offset being incorrect if the dragged item changes size after the drag begins.

Comment 2

[Chris Lord [:cwiiis]]

Comment on attachment 8544673 [details] [review]
Fix dragging expanded groups

Removing review, this doesn't work quite correctly...

Comment 4

[Chris Lord [:cwiiis]]

Comment on attachment 8544673 [details] [review]
Fix dragging expanded groups

Fixed. Whoever gets there first, please :)

Comment 5

[Kevin Grandon :kgrandon]

Comment on attachment 8544673 [details] [review]
Fix dragging expanded groups

Looks fine to me, but a few things -

1 - Appears you have someone elses commit in the PR, maybe a rebase problem?

2 - Do you think we can write a test for this? Maybe something like asserting the position of a group after long-pressing on it in a marionette test?

3 - When testing this I noticed that when long-pressing on a group, it would not immediately highlight blue. I think it should? That can probably be another bug though.

Comment 6

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8544673 [details] [review]
Fix dragging expanded groups

Hi, I am comfortable with Kevin's review here, thanks a lot for your good job

Comment 7

[Chris Lord [:cwiiis]]

(In reply to Kevin Grandon :kgrandon from comment #5)
> Comment on attachment 8544673 [details] [review]
> Fix dragging expanded groups
> 
> Looks fine to me, but a few things -
> 
> 1 - Appears you have someone elses commit in the PR, maybe a rebase problem?
> 
> 2 - Do you think we can write a test for this? Maybe something like
> asserting the position of a group after long-pressing on it in a marionette
> test?
> 
> 3 - When testing this I noticed that when long-pressing on a group, it would
> not immediately highlight blue. I think it should? That can probably be
> another bug though.

So, ends up 3 was caused by a problem that I've picked up due to a marionette test I wrote for 2 - so good call :) Will fix and push (fix isn't big, test is tiny).

Comment 8

[Chris Lord [:cwiiis]]

Comment on attachment 8544673 [details] [review]
Fix dragging expanded groups

Actually, I think the fix changes enough that it's worth another look. Sorry!

Comment 9

[Kevin Grandon :kgrandon]

Comment on attachment 8544673 [details] [review]
Fix dragging expanded groups

Looks good to me. Thanks!

Comment 10

[Chris Lord [:cwiiis]]

Addressed nit and merged: https://github.com/mozilla-b2g/gaia/pull/27164

Comment 11

[Chris Lord [:cwiiis]]

Merged follow-up to fix linter error that I missed: https://github.com/mozilla-b2g/gaia/pull/27246

Comment 12

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

have used below build to verify it.

Build ID               20150112162500
Gaia Revision          7c5b27cad370db377b18a742d3f3fdb0070e899f
Gaia Date              2015-01-12 17:32:38
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/2c37b89bdd86
Gecko Version          37.0a2
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150112.214430
Firmware Date          Mon Jan 12 21:44:41 EST 2015
Bootloader             L1TC00011880