Don't move view content position item 0 is being replaced

Usually if item 0 moves down, the next item becomes the first visible
item and all content moves down. Don't do this if another item is
moving up to take its place.

Change-Id: If625cfc0eaa70fd1094c7bc0fdd29f5f56766a42
Reviewed-on: http://codereview.qt.nokia.com/2260
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 307aeae..1aa7292 100644 (file)
--- a/src/declarative/items/qsggridview.cpp
+++ b/src/declarative/items/qsggridview.cpp
@@ -1490,9 +1490,19 @@ void QSGGridView::itemsMoved(int from, int to, int count)
     Q_D(QSGGridView);
     if (!isComponentComplete())
         return;
+    d->updateUnrequestedIndexes();
+
+    if (d->visibleItems.isEmpty()) {
+        d->refill();
+        return;
+    }
+
+    d->moveReason = QSGGridViewPrivate::Other;
+    FxGridItemSG *firstVisible = static_cast<FxGridItemSG*>(d->firstVisibleItem());
     QHash<int,FxGridItemSG*> moved;
 
-    FxGridItemSG *firstItem = static_cast<FxGridItemSG*>(d->firstVisibleItem());
+    bool movingBackwards = from > to;
+    int firstItemIndex = firstVisible ? firstVisible->index : -1;
 
     QList<FxViewItem*>::Iterator it = d->visibleItems.begin();
     while (it != d->visibleItems.end()) {
@@ -1524,8 +1534,6 @@ void QSGGridView::itemsMoved(int from, int to, int count)
             if (!movedItem)
                 movedItem = static_cast<FxGridItemSG*>(d->createItem(item->index));
             it = d->visibleItems.insert(it, movedItem);
-            if (it == d->visibleItems.begin() && firstItem)
-                movedItem->setPosition(firstItem->colPos(), firstItem->rowPos());
             ++it;
             --remaining;
         } else {
@@ -1555,6 +1563,18 @@ void QSGGridView::itemsMoved(int from, int to, int count)
         }
     }
 
+    // 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 (movingBackwards && firstItemIndex >= 0) {
+        for (it = d->visibleItems.begin(); it != d->visibleItems.end(); ++it) {
+            if ((*it)->index == firstItemIndex) {
+                static_cast<FxGridItemSG*>(*it)->setPosition(firstVisible->colPos(),
+                                                             firstVisible->rowPos());
+                break;
+            }
+        }
+    }
+
     // Fix current index
     if (d->currentIndex >= 0 && d->currentItem) {
         int oldCurrent = d->currentIndex;

diff --git a/src/declarative/items/qsglistview.cpp b/src/declarative/items/qsglistview.cpp
index fa42602..0d6a494 100644 (file)
--- a/src/declarative/items/qsglistview.cpp
+++ b/src/declarative/items/qsglistview.cpp
@@ -1819,6 +1819,9 @@ void QSGListView::itemsMoved(int from, int to, int count)
     QHash<int,FxViewItem*> moved;
     int moveBy = 0;
 
+    bool movingBackwards = from > to;
+    int firstItemIndex = firstVisible ? firstVisible->index : -1;
+
     QList<FxViewItem*>::Iterator it = d->visibleItems.begin();
     while (it != d->visibleItems.end()) {
         FxViewItem *item = *it;
@@ -1879,6 +1882,17 @@ void QSGListView::itemsMoved(int from, int to, int count)
         }
     }
 
+    // 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 (movingBackwards && firstItemIndex >= 0) {
+        for (it = d->visibleItems.begin(); it != d->visibleItems.end(); ++it) {
+            if ((*it)->index == firstItemIndex) {
+                static_cast<FxListItemSG*>(*it)->setPosition(firstVisible->position());
+                break;
+            }
+        }
+    }
+
     // Fix current index
     if (d->currentIndex >= 0 && d->currentItem) {
         int oldCurrent = d->currentIndex;

diff --git a/tests/auto/declarative/qsggridview/tst_qsggridview.cpp b/tests/auto/declarative/qsggridview/tst_qsggridview.cpp
index d8b084a..76b3032 100644 (file)
--- a/tests/auto/declarative/qsggridview/tst_qsggridview.cpp
+++ b/tests/auto/declarative/qsggridview/tst_qsggridview.cpp
@@ -77,6 +77,7 @@ private slots:
     void removed();
     void clear();
     void moved();
+    void swapWithFirstItem();
     void changeFlow();
     void currentIndex();
     void noCurrentIndex();
@@ -650,6 +651,30 @@ void tst_QSGGridView::moved()
         QTRY_COMPARE(number->text(), model.number(i));
     }
 
+    delete canvas;
+}
+
+void tst_QSGGridView::swapWithFirstItem()
+{
+    // QTBUG_9697
+    QSGView *canvas = createView();
+    canvas->show();
+
+    TestModel model;
+    for (int i = 0; i < 30; i++)
+        model.addItem("Item" + QString::number(i), "");
+
+    QDeclarativeContext *ctxt = canvas->rootContext();
+    ctxt->setContextProperty("testModel", &model);
+    ctxt->setContextProperty("testRightToLeft", QVariant(false));
+    ctxt->setContextProperty("testTopToBottom", QVariant(false));
+
+    canvas->setSource(QUrl::fromLocalFile(SRCDIR "/data/gridview1.qml"));
+    qApp->processEvents();
+
+    QSGGridView *gridview = findItem<QSGGridView>(canvas->rootObject(), "grid");
+    QTRY_VERIFY(gridview != 0);
+
     // ensure content position is stable
     gridview->setContentY(0);
     model.moveItem(10, 0);

diff --git a/tests/auto/declarative/qsglistview/tst_qsglistview.cpp b/tests/auto/declarative/qsglistview/tst_qsglistview.cpp
index 4d6920c..19bc0f0 100644 (file)
--- a/tests/auto/declarative/qsglistview/tst_qsglistview.cpp
+++ b/tests/auto/declarative/qsglistview/tst_qsglistview.cpp
@@ -91,6 +91,7 @@ private slots:
     void qListModelInterface_clear();
     void qAbstractItemModel_clear();
 
+    void swapWithFirstItem();
     void itemList();
     void currentIndex();
     void noCurrentIndex();
@@ -879,6 +880,36 @@ void tst_QSGListView::moved()
     delete testObject;
 }
 
+void tst_QSGListView::swapWithFirstItem()
+{
+    QSGView *canvas = createView();
+    canvas->show();
+
+    TestModel model;
+    for (int i = 0; i < 30; i++)
+        model.addItem("Item" + QString::number(i), "");
+
+    QDeclarativeContext *ctxt = canvas->rootContext();
+    ctxt->setContextProperty("testModel", &model);
+
+    TestObject *testObject = new TestObject;
+    ctxt->setContextProperty("testObject", testObject);
+
+    canvas->setSource(QUrl::fromLocalFile(SRCDIR "/data/listviewtest.qml"));
+    qApp->processEvents();
+
+    QSGListView *listview = findItem<QSGListView>(canvas->rootObject(), "list");
+    QTRY_VERIFY(listview != 0);
+
+    // ensure content position is stable
+    listview->setContentY(0);
+    model.moveItem(10, 0);
+    QTRY_VERIFY(listview->contentY() == 0);
+
+    delete testObject;
+    delete canvas;
+}
+
 void tst_QSGListView::enforceRange()
 {
     QSGView *canvas = createView();