STR 1. Long press the leftmost empty space of a group (which is not the last) 2. Install a new Smart Collection 3. Verify that the Smart Collection is installed where you taped 4. Delete the Smart Collection 5. Long press at the same place in step 1 6. Reinstall the same Smart Collection Expected result Smart Collection is installed where you taped as in step 3 Actual The Smart Collection is place in the other group. See video for details. Video Link: http://mzl.la/1yt9R2x

Inconsistent UX.

If you add a different collection does the issue reproduce?

Does UX consider this as a blocker or nice to have?

Use the center of an item to calculate the nearest item index (getNearestItemIndex) instead of use the x and y https://tbpl.mozilla.org/?tree=Gaia-Try&rev=3371f2de6d8c

fix a regression bug when we move an app on edit mode https://github.com/mozilla-b2g/gaia/pull/20859 https://tbpl.mozilla.org/?tree=Gaia-Try&rev=4cb7884c8fc2

Comment on attachment 8444127 [details] [diff] [review] 1028330.patch Review of attachment 8444127 [details] [diff] [review]: ----------------------------------------------------------------- Hmm, it would be better to obsolete one patch and upload a new one. I'll take a look, thanks!

Oh, it looks like there's a github pull request? Let's just review that, thanks!

Comment on attachment 8444171 [details] Pull request Clearing review for now due to the unit test failures. I think we would rather be testing this with integration tests, so perhaps this would be a good time to remove the failing test if we can achieve the same coverage with integration tests. This is the test that's failing: https://github.com/mozilla-b2g/gaia/blob/master/apps/sharedtest/test/unit/elements/gaia_grid/grid_dragdrop_test.js#L49 https://travis-ci.org/mozilla-b2g/gaia/jobs/28164625

Freddy, do you have some time to fix the failing unit tests ? You can find a tutorial to write unit tests at https://developer.mozilla.org/en-US/Firefox_OS/Platform/Automated_testing/Gaia_unit_tests If you look at https://tbpl.mozilla.org/?tree=Gaia-Try&rev=4cb7884c8fc2 there is an orange 'G'. When you click on it, you will see the name of the unit tests that are failing. I suggest that you fix the test locally, following the tutorial I linked. Every time you will edit the test file and save the change, the test runner will re-rerun the test of this file for you (if you have launched ./bin/gaia-test). The 'Gu' test that is failing is not your fault, so you can ignore this one. Kevin, I suggest that we fix those unit tests, and open a followup for the integration tests. Is that OK for you ?

Oups. Still need the needinfo from Freddy.

(In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) from comment #10) > Kevin, I suggest that we fix those unit tests, and open a followup for the > integration tests. Is that OK for you ? I think it's fine by me, it's likely the new usage of clientWidth/clientHeight that is throwing off desktop. I looked through the test code and tbh I don't think it is covering that much useful stuff, so I thought it may be easier to write a new integration test than save it. Let's see if Freddy has any luck with the unit test, if not we can probably rip it out as long as we file a follow-up to add a better integration test.

Flagging Jacqueline on the UX questions.

Use gridItemWidth and gridItemHeight layout properties in the getNearestItemIndex method instead of clientWidth and clientHeight https://tbpl.mozilla.org/?tree=Gaia-Try&rev=8fd9530fbdd4

Comment on attachment 8444859 [details] Pull Request Thank you for your patch. I tried this on a device today and I'm noticing a less-than-optimal experience when icons reflow around the currently dragged icon. I'm not sure exactly what the cause is, but my assumption is that we need to adjust for the xAdjust/yAdjust numbers somewhere. Will attach a screenshot of what I'm seeing.

The reporters bug does NOT repro on: Flame 2.1 Master, Flame 2.0, OpenC 2.1 master, OpenC 2.0, Buri 2.1, Buri 2.0 Actual Results: Adding then deleting and re-adding a collection will place the reinstalled collection in the expected location. Environmental Variables: Device: Flame Master Build ID: 20140624090716 Gaia: 2944695a89c3281d49dbe5ff9c0cd26c8318e2ba Gecko: 215b93e07e1d Version: 33.0a1 (Master) Firmware Version: v122 ----------------------------------------- Environmental Variables: Device: Flame 2.0 Build ID: 20140624093150 Gaia: 7260d58fb2b4665ebe614f94d822b8407bd95f58 Gecko: 23457787dfa1 Version: 32.0a2 (2.0) Firmware Version: v122 ----------------------------------------- Environmental Variables: Device: Open_C Master Build ID: 20140624090716 Gaia: 2944695a89c3281d49dbe5ff9c0cd26c8318e2ba Gecko: 215b93e07e1d Version: 33.0a1 (Master) Firmware Version: P821A10V1.0.0B06_LOG_DL ----------------------------------------- Environmental Variables: Device: Open_C 2.0 Build ID: 20140624093150 Gaia: 7260d58fb2b4665ebe614f94d822b8407bd95f58 Gecko: 23457787dfa1 Version: 32.0a2 (2.0) Firmware Version: P821A10V1.0.0B06_LOG_DL ---------------------------------------- Environmental Variables: Device: Buri Master Build ID: 20140624090716 Gaia: 2944695a89c3281d49dbe5ff9c0cd26c8318e2ba Gecko: 215b93e07e1d Version: 33.0a1 (Master) Firmware Version: v1.2device.cfg ----------------------------------------- Environmental Variables: Device: Buri 2.0 Build ID: 20140624133052 Gaia: 7260d58fb2b4665ebe614f94d822b8407bd95f58 Gecko: 55155b4af80d Version: 32.0a2 (2.0) Firmware Version: v1.2device.cfg

(In reply to bhavana bajaj [:bajaj] [NOT reading Bugmail, needInfo please] from comment #2) > If you add a different collection does the issue reproduce? I was unable to repro with any collection.

Johan - Can you retest here? The QAnalysts can't reproduce the bug here.

Not able to reproduce either anymore.

Removing flag as bug is resolved

The original report talk about a bug for a re-installation. I didn't see anything like this in the source code. But while testing, I found that when we press at the bottom of a placeholder, the smart collection isn't created in the right place (see attachment). It's because the "getNearestItemIndex" function take into account X and Y of the gridItem. And this origin point is located on the top left corner of the item. Doing the center of the element the origin point, solve the bug I describe and I think it's what the user expect. But it causes other bugs for the drag and drop I've tried to solve.

Thanks Freddy - I agree that fixing this should improve the drag/drop usability slightly. It would be good to fix this, though we need to fix the icon reflowing issue that I mentioned above. Re-opening to track this effort, and renaming the bug to match what we're doing here.

Sorry, I confused some of my bugmail and took this mistakenly - Assigning it back. If that's ok, treat my pull request as a feedback comment - the patch is almost identical :)

Comment on attachment 8451527 [details] Use the center of the item when calculating the nearest grid item I'd like to fold in any updates to Freddy's original PR here if possible, as they are quite similar in nature. (I'm still seeing the reflowing regression with this pull request as well)

A tip with the missing rearranging, GridDragDrop offsets the coordinates by an amount, I assume to counteract this bug(?) Search for 'Testing with some extra offset' in grid_dragdrop.js - if we don't want to get rid of that, we can at least compensate for it in positionIcon, where the call to getNearestItemIndex is made.

Sorry about that, I think this is leftover from the prototyping days :) I believe the idea was that you want to adjust the icon so you can see it better as you drag it around. I think we probably need to continue to make those adjustments, but take that adjustment into account for the center position calculation.

Mass update: Resolve wontfix all issues with legacy homescreens. As of 2.6 we have a new homescreen and having these issues open is confusing. All issues will block bug 1231115 so we can use that to re-visit any of these if needed.