Merge pull request #71 from matthewvogt/delay-remove-fix

ListView contentHeight should include delayRemove-d items

src/quick/items/qquickgridview.cpp

@@ -276,9 +276,13 @@ qreal QQuickGridViewPrivate::originPosition() const
 qreal QQuickGridViewPrivate::lastPosition() const
 {
     qreal pos = 0;
-    if (model && model->count()) {
-        // get end position of last item
-        pos = (rowPosAt(model->count() - 1) + rowSize());
+    if (model && (model->count() || !visibleItems.isEmpty())) {
+        qreal lastRowPos = model->count() ? rowPosAt(model->count() - 1) : 0;
+        if (!visibleItems.isEmpty()) {
+            // If there are items in delayRemove state, they may be after any items linked to the model
+            lastRowPos = qMax(lastRowPos, static_cast<FxGridItemSG*>(visibleItems.last())->rowPos());
+        }
+        pos = lastRowPos + rowSize();
     }
     return pos;
 }

src/quick/items/qquickitemview.cpp

@@ -1333,7 +1333,7 @@ void QQuickItemView::geometryChanged(const QRectF &newGeometry, const QRectF &ol
 {
     Q_D(QQuickItemView);
     d->markExtentsDirty();
-    if (isComponentComplete() && d->isValid())
+    if (isComponentComplete() && (d->isValid() || !d->visibleItems.isEmpty()))
         d->forceLayoutPolish();
     QQuickFlickable::geometryChanged(newGeometry, oldGeometry);
 }
@@ -1793,7 +1793,7 @@ void QQuickItemViewPrivate::updateViewport()
 {
     Q_Q(QQuickItemView);
     qreal extra = headerSize() + footerSize();
-    qreal contentSize = isValid() ? (endPosition() - startPosition()) : 0.0;
+    qreal contentSize = isValid() || !visibleItems.isEmpty() ? (endPosition() - startPosition()) : 0.0;
     if (layoutOrientation() == Qt::Vertical)
         q->setContentHeight(contentSize + extra);
     else
@@ -1811,6 +1811,7 @@ void QQuickItemViewPrivate::layout()
     if (!isValid() && !visibleItems.count()) {
         clear();
         setPosition(contentStartOffset());
+        updateViewport();
         if (transitioner)
             transitioner->setPopulateTransitionEnabled(false);
         inLayout = false;

src/quick/items/qquicklistview.cpp

@@ -432,14 +432,24 @@ qreal QQuickListViewPrivate::lastPosition() const
 {
     qreal pos = 0;
     if (!visibleItems.isEmpty()) {
-        int invisibleCount = visibleItems.count() - visibleIndex;
+        int invisibleCount = INT_MIN;
+        int delayRemovedCount = 0;
         for (int i = visibleItems.count()-1; i >= 0; --i) {
             if (visibleItems.at(i)->index != -1) {
-                invisibleCount = model->count() - visibleItems.at(i)->index - 1;
+                // Find the invisible count after the last visible item with known index
+                invisibleCount = model->count() - (visibleItems.at(i)->index + 1 + delayRemovedCount);
                 break;
+            } else if (visibleItems.at(i)->attached->delayRemove()) {
+                ++delayRemovedCount;
             }
         }
-        pos = (*(--visibleItems.constEnd()))->endPosition() + invisibleCount * (averageSize + spacing);
+        if (invisibleCount == INT_MIN) {
+            // All visible items are in delayRemove state
+            invisibleCount = model->count();
+        }
+        pos = (*(--visibleItems.constEnd()))->endPosition();
+        if (invisibleCount > 0)
+            pos += invisibleCount * (averageSize + spacing);
     } else if (model && model->count()) {
         pos = (model->count() * averageSize + (model->count()-1) * spacing);
     }

tests/auto/quick/qquickgridview/data/contentHeightWithDelayRemove.qml

@@ -0,0 +1,47 @@
+import QtQuick 2.1
+
+Item {
+    width: 400
+    height: 600
+    function takeOne()
+    {
+        gridView.model.remove(2)
+    }
+    function takeThree()
+    {
+        gridView.model.remove(4)
+        gridView.model.remove(2)
+        gridView.model.remove(0)
+    }
+    function takeAll()
+    {
+        gridView.model.clear()
+    }
+
+    GridView {
+        id: gridView
+
+        property bool useDelayRemove
+
+        height: parent.height
+        width: 400
+        cellWidth: width/2
+        model: ListModel {
+            ListElement { name: "A" }
+            ListElement { name: "B" }
+            ListElement { name: "C" }
+            ListElement { name: "D" }
+            ListElement { name: "E" }
+        }
+        delegate: Text {
+            id: wrapper
+            height: 100
+            text: index + gridView.count
+            GridView.delayRemove: gridView.useDelayRemove
+            GridView.onRemove: SequentialAnimation {
+                PauseAnimation { duration: wrapper.GridView.delayRemove ? 100 : 0 }
+                PropertyAction { target: wrapper; property: "GridView.delayRemove"; value: false }
+            }
+        }
+    }
+}

tests/auto/quick/qquickgridview/tst_qquickgridview.cpp

@@ -209,6 +209,9 @@ private slots:
 
     void displayMargin();
 
+    void contentHeightWithDelayRemove_data();
+    void contentHeightWithDelayRemove();
+
 private:
     QList<int> toIntList(const QVariantList &list);
     void matchIndexLists(const QVariantList &indexLists, const QList<int> &expectedIndexes);
@@ -6346,6 +6349,74 @@ void tst_QQuickGridView::displayMargin()
     delete window;
 }
 
+void tst_QQuickGridView::contentHeightWithDelayRemove_data()
+{
+    QTest::addColumn<bool>("useDelayRemove");
+    QTest::addColumn<QByteArray>("removeFunc");
+    QTest::addColumn<int>("countDelta");
+    QTest::addColumn<qreal>("contentHeightDelta");
+
+    QTest::newRow("remove without delayRemove")
+            << false
+            << QByteArray("takeOne")
+            << -1
+            << qreal(-1 * 100.0);
+
+    QTest::newRow("remove with delayRemove")
+            << true
+            << QByteArray("takeOne")
+            << -1
+            << qreal(-1 * 100.0);
+
+    QTest::newRow("remove with multiple delayRemove")
+            << true
+            << QByteArray("takeThree")
+            << -3
+            << qreal(-2 * 100.0);
+
+    QTest::newRow("clear with delayRemove")
+            << true
+            << QByteArray("takeAll")
+            << -5
+            << qreal(-3 * 100.0);
+}
+
+void tst_QQuickGridView::contentHeightWithDelayRemove()
+{
+    QFETCH(bool, useDelayRemove);
+    QFETCH(QByteArray, removeFunc);
+    QFETCH(int, countDelta);
+    QFETCH(qreal, contentHeightDelta);
+
+    QQuickView *window = createView();
+    window->setSource(testFileUrl("contentHeightWithDelayRemove.qml"));
+    window->show();
+    QVERIFY(QTest::qWaitForWindowExposed(window));
+
+    QQuickGridView *gridview = window->rootObject()->findChild<QQuickGridView*>();
+    QTRY_VERIFY(gridview != 0);
+
+    const int initialCount(gridview->count());
+    const int eventualCount(initialCount + countDelta);
+
+    const qreal initialContentHeight(gridview->contentHeight());
+    const int eventualContentHeight(qRound(initialContentHeight + contentHeightDelta));
+
+    gridview->setProperty("useDelayRemove", useDelayRemove);
+    QMetaObject::invokeMethod(window->rootObject(), removeFunc.constData());
+    QTest::qWait(50);
+    QCOMPARE(gridview->count(), eventualCount);
+
+    if (useDelayRemove) {
+        QCOMPARE(qRound(gridview->contentHeight()), qRound(initialContentHeight));
+        QTRY_COMPARE(qRound(gridview->contentHeight()), eventualContentHeight);
+    } else {
+        QCOMPARE(qRound(gridview->contentHeight()), eventualContentHeight);
+    }
+
+    delete window;
+}
+
 QTEST_MAIN(tst_QQuickGridView)
 
 #include "tst_qquickgridview.moc"

tests/auto/quick/qquicklistview/data/contentHeightWithDelayRemove.qml

@@ -0,0 +1,46 @@
+import QtQuick 2.1
+
+Item {
+    width: 400
+    height: 600
+    function takeOne()
+    {
+        listView.model.remove(2)
+    }
+    function takeThree()
+    {
+        listView.model.remove(4)
+        listView.model.remove(2)
+        listView.model.remove(0)
+    }
+    function takeAll()
+    {
+        listView.model.clear()
+    }
+
+    ListView {
+        id: listView
+
+        property bool useDelayRemove
+
+        height: parent.height
+        width: 400
+        model: ListModel {
+            ListElement { name: "A" }
+            ListElement { name: "B" }
+            ListElement { name: "C" }
+            ListElement { name: "D" }
+            ListElement { name: "E" }
+        }
+        delegate: Text {
+            id: wrapper
+            height: 100
+            text: index + listView.count
+            ListView.delayRemove: listView.useDelayRemove
+            ListView.onRemove: SequentialAnimation {
+                PauseAnimation { duration: wrapper.ListView.delayRemove ? 100 : 0 }
+                PropertyAction { target: wrapper; property: "ListView.delayRemove"; value: false }
+            }
+        }
+    }
+}

tests/auto/quick/qquicklistview/tst_qquicklistview.cpp

@@ -231,6 +231,9 @@ private slots:
 
     void layoutChange();
 
+    void contentHeightWithDelayRemove();
+    void contentHeightWithDelayRemove_data();
+
 private:
     template <class T> void items(const QUrl &source);
     template <class T> void changed(const QUrl &source);
@@ -7402,6 +7405,74 @@ void tst_QQuickListView::layoutChange()
     }
 }
 
+void tst_QQuickListView::contentHeightWithDelayRemove_data()
+{
+    QTest::addColumn<bool>("useDelayRemove");
+    QTest::addColumn<QByteArray>("removeFunc");
+    QTest::addColumn<int>("countDelta");
+    QTest::addColumn<qreal>("contentHeightDelta");
+
+    QTest::newRow("remove without delayRemove")
+            << false
+            << QByteArray("takeOne")
+            << -1
+            << qreal(-1 * 100.0);
+
+    QTest::newRow("remove with delayRemove")
+            << true
+            << QByteArray("takeOne")
+            << -1
+            << qreal(-1 * 100.0);
+
+    QTest::newRow("remove with multiple delayRemove")
+            << true
+            << QByteArray("takeThree")
+            << -3
+            << qreal(-3 * 100.0);
+
+    QTest::newRow("clear with delayRemove")
+            << true
+            << QByteArray("takeAll")
+            << -5
+            << qreal(-5 * 100.0);
+}
+
+void tst_QQuickListView::contentHeightWithDelayRemove()
+{
+    QFETCH(bool, useDelayRemove);
+    QFETCH(QByteArray, removeFunc);
+    QFETCH(int, countDelta);
+    QFETCH(qreal, contentHeightDelta);
+
+    QQuickView *window = createView();
+    window->setSource(testFileUrl("contentHeightWithDelayRemove.qml"));
+    window->show();
+    QVERIFY(QTest::qWaitForWindowExposed(window));
+
+    QQuickListView *listview = window->rootObject()->findChild<QQuickListView*>();
+    QTRY_VERIFY(listview != 0);
+
+    const int initialCount(listview->count());
+    const int eventualCount(initialCount + countDelta);
+
+    const qreal initialContentHeight(listview->contentHeight());
+    const int eventualContentHeight(qRound(initialContentHeight + contentHeightDelta));
+
+    listview->setProperty("useDelayRemove", useDelayRemove);
+    QMetaObject::invokeMethod(window->rootObject(), removeFunc.constData());
+    QTest::qWait(50);
+    QCOMPARE(listview->count(), eventualCount);
+
+    if (useDelayRemove) {
+        QCOMPARE(qRound(listview->contentHeight()), qRound(initialContentHeight));
+        QTRY_COMPARE(qRound(listview->contentHeight()), eventualContentHeight);
+    } else {
+        QCOMPARE(qRound(listview->contentHeight()), eventualContentHeight);
+    }
+
+    delete window;
+}
+
 QTEST_MAIN(tst_QQuickListView)
 
 #include "tst_qquicklistview.moc"