Fix for moving multiple items to top of view

Top item wasn't positioned correctly when moving multiple items
backwards to the top of the view. Don't move visibleItems.first() if
we've already repositioned the top item correctly.

Task-number: QTBUG-22762
Change-Id: Ib216adea719044ee55349478237473ff3194a326
Reviewed-by: Martin Jones <martin.jones@nokia.com>

diff --git a/src/declarative/items/qquickitemview.cpp b/src/declarative/items/qquickitemview.cpp
index 79787d7..63855ae 100644 (file)
--- a/src/declarative/items/qquickitemview.cpp
+++ b/src/declarative/items/qquickitemview.cpp
@@ -1491,18 +1491,18 @@ bool QQuickItemViewPrivate::applyModelChanges()
     // if the first visible item has moved, ensure another one takes its place
     // so that we avoid shifting all content forwards
     // (if an item is removed from before the first visible, the first visible should not move upwards)
+    bool movedBackToFirstVisible = false;
     if (firstVisible && firstItemIndex >= 0) {
-        bool found = false;
         for (int i=0; i<insertResult.movedBackwards.count(); i++) {
             if (insertResult.movedBackwards[i]->index == firstItemIndex) {
                 // an item has moved backwards up to the first visible's position
                 resetItemPosition(insertResult.movedBackwards[i], firstVisible);
                 insertResult.movedBackwards.removeAt(i);
-                found = true;
+                movedBackToFirstVisible = true;
                 break;
             }
         }
-        if (!found && !allInsertionsBeforeVisible) {
+        if (!movedBackToFirstVisible && !allInsertionsBeforeVisible) {
             // first visible item has moved forward, another visible item takes its place
             FxViewItem *item = visibleItem(firstItemIndex);
             if (item)
@@ -1516,10 +1516,14 @@ bool QQuickItemViewPrivate::applyModelChanges()
         // since it will be used to position all subsequent items
         if (insertResult.movedBackwards.count() && origVisibleItemsFirst)
             resetItemPosition(visibleItems.first(), origVisibleItemsFirst);
-        qreal moveBackwardsBy = insertResult.sizeAddedBeforeVisible;
-        for (int i=0; i<insertResult.movedBackwards.count(); i++)
-            moveBackwardsBy += insertResult.movedBackwards[i]->size();
-        moveItemBy(visibleItems.first(), removedBeforeFirstVisibleBy, moveBackwardsBy);
+
+        // correct the first item position (unless it has already been fixed)
+        if (!movedBackToFirstVisible) {
+            qreal moveBackwardsBy = insertResult.sizeAddedBeforeVisible;
+            for (int i=0; i<insertResult.movedBackwards.count(); i++)
+                moveBackwardsBy += insertResult.movedBackwards[i]->size();
+            moveItemBy(visibleItems.first(), removedBeforeFirstVisibleBy, moveBackwardsBy);
+        }
     }
 
     // Whatever removed/moved items remain are no longer visible items.

diff --git a/tests/auto/declarative/qquickgridview/tst_qquickgridview.cpp b/tests/auto/declarative/qquickgridview/tst_qquickgridview.cpp
index a190d87..7a399eb 100644 (file)
--- a/tests/auto/declarative/qquickgridview/tst_qquickgridview.cpp
+++ b/tests/auto/declarative/qquickgridview/tst_qquickgridview.cpp
@@ -1005,6 +1005,11 @@ void tst_QQuickGridView::moved_data()
             << 0 << 5 << 3
             << 0.0;
 
+    QTest::newRow("move multiple backwards, within visible items (move first item)")
+            << 0.0
+            << 10 << 0 << 3
+            << 0.0;
+
     QTest::newRow("move multiple forwards, before visible items")
             << 120.0     // show 6-23
             << 3 << 4 << 3      // 3, 4, 5 move to after 6

diff --git a/tests/auto/declarative/qquicklistview/tst_qquicklistview.cpp b/tests/auto/declarative/qquicklistview/tst_qquicklistview.cpp
index 36824f8..b2e6645 100644 (file)
--- a/tests/auto/declarative/qquicklistview/tst_qquicklistview.cpp
+++ b/tests/auto/declarative/qquicklistview/tst_qquicklistview.cpp
@@ -1219,6 +1219,11 @@ void tst_QQuickListView::moved_data()
             << 4 << 1 << 3
             << 0.0;
 
+    QTest::newRow("move multiple backwards, within visible items (move first item)")
+            << 0.0
+            << 10 << 0 << 3
+            << 0.0;
+
     QTest::newRow("move multiple backwards, from non-visible -> visible")
             << 0.0
             << 20 << 4 << 3