Don't move content forwards when items are moved down

Previously, if items moved down within a view, the content position
would effectively drop down. E.g. for a (0,5,3) move that moved 3
items from 0 to 5, the content y would move to the position of index
3, since it became the new first item. However, this makes it
difficult to move transitions for move() operations in these cases
since these items do not move (since the content position moves
instead). With this fix, the content position does not move, and items
will always move if they are moved.

Note this behaviour was previously implemented for backwards
movements, e.g. a (5,0,3) move but was not enabled for a forwards
(0,5,3) move.

Change-Id: I1c5a19e3c36347a4aa0cf6e31c975967a7eeada9
Reviewed-on: http://codereview.qt-project.org/5576
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: Martin Jones <martin.jones@nokia.com>

diff --git a/src/declarative/items/qsggridview.cpp b/src/declarative/items/qsggridview.cpp
index dfcd1b2..099d62e 100644 (file)
--- a/src/declarative/items/qsggridview.cpp
+++ b/src/declarative/items/qsggridview.cpp
@@ -574,6 +574,8 @@ void QSGGridViewPrivate::repositionPackageItemAt(QSGItem *item, int index)
 
 void QSGGridViewPrivate::resetItemPosition(FxViewItem *item, FxViewItem *toItem)
 {
+    if (item == toItem)
+        return;
     FxGridItemSG *toGridItem = static_cast<FxGridItemSG*>(toItem);
     static_cast<FxGridItemSG*>(item)->setPosition(toGridItem->colPos(), toGridItem->rowPos());
 }

diff --git a/src/declarative/items/qsgitemview.cpp b/src/declarative/items/qsgitemview.cpp
index 63040c1..666b471 100644 (file)
--- a/src/declarative/items/qsgitemview.cpp
+++ b/src/declarative/items/qsgitemview.cpp
@@ -1489,16 +1489,27 @@ bool QSGItemViewPrivate::applyModelChanges()
     for (int i=0; i<addedItems.count(); ++i)
         addedItems.at(i)->attached->emitAdd();
 
-    // if first visible item is moving but another item is moving up to replace it,
-    // do this positioning now to avoid shifting all content forwards
+    // if the first visible item has moved, ensure another one takes its place
+    // so that we avoid shifting all content forwards
+    // (don't use items from removedBeforeFirstVisible - if an item is removed from
+    // before the first visible, the first visible should not move upwards)
     if (firstVisible && firstItemIndex >= 0) {
+        bool found = false;
         for (int i=0; i<movedBackwards.count(); i++) {
             if (movedBackwards[i]->index == firstItemIndex) {
+                // an item has moved backwards up to the first visible's position
                 resetItemPosition(movedBackwards[i], firstVisible);
                 movedBackwards.removeAt(i);
+                found = true;
                 break;
             }
         }
+        if (!found) {
+            // first visible item has moved forward, another item takes its place
+            FxViewItem *item = visibleItem(firstItemIndex);
+            if (item)
+                resetItemPosition(item, firstVisible);
+        }
     }
 
     // Ensure we don't cause an ugly list scroll

diff --git a/src/declarative/items/qsglistview.cpp b/src/declarative/items/qsglistview.cpp
index 4aaf131..7f9ce3a 100644 (file)
--- a/src/declarative/items/qsglistview.cpp
+++ b/src/declarative/items/qsglistview.cpp
@@ -716,6 +716,8 @@ void QSGListViewPrivate::repositionPackageItemAt(QSGItem *item, int index)
 
 void QSGListViewPrivate::resetItemPosition(FxViewItem *item, FxViewItem *toItem)
 {
+    if (item == toItem)
+        return;
     static_cast<FxListItemSG*>(item)->setPosition(toItem->position());
 }
 

diff --git a/tests/auto/declarative/qsggridview/tst_qsggridview.cpp b/tests/auto/declarative/qsggridview/tst_qsggridview.cpp
index 6ec5378..eab291f 100644 (file)
--- a/tests/auto/declarative/qsggridview/tst_qsggridview.cpp
+++ b/tests/auto/declarative/qsggridview/tst_qsggridview.cpp
@@ -737,7 +737,12 @@ void tst_QSGGridView::moved_data()
     QTest::newRow("move multiple forwards, within visible items")
             << 0.0
             << 0 << 5 << 3
-            << 60.0;    // moved 3 items (i.e. 1 row) down
+            << 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
+            << 60.0;      // row of 3,4,5 has moved down
 
     QTest::newRow("move multiple forwards, from non-visible -> visible")
             << 120.0     // show 6-23
@@ -757,7 +762,7 @@ void tst_QSGGridView::moved_data()
     QTest::newRow("move multiple forwards, from visible -> non-visible (move first item)")
             << 0.0
             << 0 << 16 << 3
-            << 60.0;
+            << 0.0;
 
 
     QTest::newRow("move multiple backwards, within visible items")

diff --git a/tests/auto/declarative/qsglistview/tst_qsglistview.cpp b/tests/auto/declarative/qsglistview/tst_qsglistview.cpp
index 9642b8f..c1be773 100644 (file)
--- a/tests/auto/declarative/qsglistview/tst_qsglistview.cpp
+++ b/tests/auto/declarative/qsglistview/tst_qsglistview.cpp
@@ -981,7 +981,7 @@ void tst_QSGListView::moved_data()
     QTest::newRow("move 1 forwards, from visible -> non-visible (move first item)")
             << 0.0
             << 0 << 16 << 1
-            << 20.0;
+            << 0.0;
 
 
     QTest::newRow("move 1 backwards, within visible items")
@@ -1018,7 +1018,12 @@ void tst_QSGListView::moved_data()
     QTest::newRow("move multiple forwards, within visible items")
             << 0.0
             << 0 << 5 << 3
-            << 20.0 * 3;
+            << 0.0;
+
+    QTest::newRow("move multiple forwards, before visible items")
+            << 140.0     // show 7-22
+            << 4 << 5 << 3      // 4,5,6 move to below 7
+            << 20.0 * 3;      // 4,5,6 moved down
 
     QTest::newRow("move multiple forwards, from non-visible -> visible")
             << 80.0     // show 4-19
@@ -1038,7 +1043,7 @@ void tst_QSGListView::moved_data()
     QTest::newRow("move multiple forwards, from visible -> non-visible (move first item)")
             << 0.0
             << 0 << 16 << 3
-            << 20.0 * 3;
+            << 0.0;
 
 
     QTest::newRow("move multiple backwards, within visible items")