Fix positioning of items after move

If a move happened after a contentY change but before a refill, the
position of the first visible item wasn't calculated correctly.

If the first item from visibleItems was above the content start
position and a move operation caused the items below it to move
away, this first item was not being moved to the correct position
(i.e. above the next available visible item) causing the following
visible items to be positioned incorrectly on the next refill.

ListView supported this previously but only adjusted positioning for
items before the content position, instead of for all moved items.

Fixed for both ListView and GridView and added more move-related tests.

Task-number: QTBUG-20528
Change-Id: I2ba1a2f5e49f790e694c6e1486f649f10d09c256
Reviewed-on: http://codereview.qt.nokia.com/2261
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: Martin Jones <martin.jones@nokia.com>

2 files changed, 26 insertions, 3 deletions

diff --git a/src/declarative/items/qsggridview.cpp b/src/declarative/items/qsggridview.cpp
index 1aa7292060..8559379d20 100644
--- a/src/declarative/items/qsggridview.cpp
+++ b/src/declarative/items/qsggridview.cpp
@@ -1500,10 +1500,18 @@ void QSGGridView::itemsMoved(int from, int to, int count)
     d->moveReason = QSGGridViewPrivate::Other;
     FxGridItemSG *firstVisible = static_cast<FxGridItemSG*>(d->firstVisibleItem());
     QHash<int,FxGridItemSG*> moved;
+    int moveByCount = 0;
 
     bool movingBackwards = from > to;
     int firstItemIndex = firstVisible ? firstVisible->index : -1;
 
+    // if visibleItems.first() is above the content start pos, and the items
+    // beneath it are moved, ensure this first item is later repositioned correctly
+    // (to above the next visible item) so that subsequent layout() is correct
+    bool repositionFirstItem = firstVisible
+            && d->visibleItems.first()->position() < firstVisible->position()
+            && from > d->visibleItems.first()->index;
+
     QList<FxViewItem*>::Iterator it = d->visibleItems.begin();
     while (it != d->visibleItems.end()) {
         FxViewItem *item = *it;
@@ -1511,6 +1519,8 @@ void QSGGridView::itemsMoved(int from, int to, int count)
             // take the items that are moving
             item->index += (to-from);
             moved.insert(item->index, static_cast<FxGridItemSG*>(item));
+            if (repositionFirstItem)
+                moveByCount++;
             it = d->visibleItems.erase(it);
         } else {
             if (item->index > from && item->index != -1) {
@@ -1594,6 +1604,12 @@ void QSGGridView::itemsMoved(int from, int to, int count)
         d->releaseItem(item);
     }
 
+    // Ensure we don't cause an ugly list scroll.
+    if (d->visibleItems.count() && moveByCount > 0) {
+        FxGridItemSG *first = static_cast<FxGridItemSG*>(d->visibleItems.first());
+        first->setPosition(first->colPos(), first->rowPos() + ((moveByCount / d->columns) * d->rowSize()));
+    }
+
     d->layout();
 }
 
diff --git a/src/declarative/items/qsglistview.cpp b/src/declarative/items/qsglistview.cpp
index 0d6a4947f4..abfb4fe9a0 100644
--- a/src/declarative/items/qsglistview.cpp
+++ b/src/declarative/items/qsglistview.cpp
@@ -1815,13 +1815,19 @@ void QSGListView::itemsMoved(int from, int to, int count)
 
     d->moveReason = QSGListViewPrivate::Other;
     FxViewItem *firstVisible = d->firstVisibleItem();
-    qreal firstItemPos = firstVisible->position();
     QHash<int,FxViewItem*> moved;
     int moveBy = 0;
 
     bool movingBackwards = from > to;
     int firstItemIndex = firstVisible ? firstVisible->index : -1;
 
+    // if visibleItems.first() is above the content start pos, and the items
+    // beneath it are moved, ensure this first item is later repositioned correctly
+    // (to above the next visible item) so that subsequent layout() is correct
+    bool repositionFirstItem = firstVisible
+            && d->visibleItems.first()->position() < firstVisible->position()
+            && from > d->visibleItems.first()->index;
+
     QList<FxViewItem*>::Iterator it = d->visibleItems.begin();
     while (it != d->visibleItems.end()) {
         FxViewItem *item = *it;
@@ -1829,7 +1835,7 @@ void QSGListView::itemsMoved(int from, int to, int count)
             // take the items that are moving
             item->index += (to-from);
             moved.insert(item->index, item);
-            if (item->position() < firstItemPos)
+            if (repositionFirstItem)
                 moveBy += item->size();
             it = d->visibleItems.erase(it);
         } else {
@@ -1913,7 +1919,8 @@ void QSGListView::itemsMoved(int from, int to, int count)
     }
 
     // Ensure we don't cause an ugly list scroll.
-    static_cast<FxListItemSG*>(d->visibleItems.first())->setPosition(d->visibleItems.first()->position() + moveBy);
+    if (d->visibleItems.count())
+        static_cast<FxListItemSG*>(d->visibleItems.first())->setPosition(d->visibleItems.first()->position() + moveBy);
 
     d->updateSections();
     d->layout();