QQuickItemView: always honor the removeDisplaced animation

The animation was not being performed if the delayRemove attached
property was changed by the handler of the remove() attached signal.
We need to run the delayed transitions not only if we have an animation
for the target item, but also if we have an animation for the items
being displaced.
(The flag variables can safely be obtained outside of the for loop,
given that their value should not change during the loop iteration)

Task-number: QTBUG-57225
Change-Id: I8c138677d7dcdf63e0932ec5cf7738c0caeb2ab8
Reviewed-by: J-P Nurmi <jpnurmi@qt.io>
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>

3 files changed, 45 insertions, 4 deletions

diff --git a/src/quick/items/qquickitemview.cpp b/src/quick/items/qquickitemview.cpp
index 8f3fe9f0f2..66438ee37d 100644
--- a/src/quick/items/qquickitemview.cpp
+++ b/src/quick/items/qquickitemview.cpp
@@ -1246,16 +1246,26 @@ void QQuickItemViewPrivate::itemGeometryChanged(QQuickItem *item, QQuickGeometry
 void QQuickItemView::destroyRemoved()
 {
     Q_D(QQuickItemView);
+
+    bool hasRemoveTransition = false;
+    bool hasRemoveTransitionAsTarget = false;
+    if (d->transitioner) {
+        hasRemoveTransition = d->transitioner->canTransition(QQuickItemViewTransitioner::RemoveTransition, false);
+        hasRemoveTransitionAsTarget = d->transitioner->canTransition(QQuickItemViewTransitioner::RemoveTransition, true);
+    }
+
     for (QList<FxViewItem*>::Iterator it = d->visibleItems.begin();
             it != d->visibleItems.end();) {
         FxViewItem *item = *it;
         if (item->index == -1 && (!item->attached || item->attached->delayRemove() == false)) {
-            if (d->transitioner && d->transitioner->canTransition(QQuickItemViewTransitioner::RemoveTransition, true)) {
+            if (hasRemoveTransitionAsTarget) {
                 // don't remove from visibleItems until next layout()
                 d->runDelayedRemoveTransition = true;
                 QObject::disconnect(item->attached, SIGNAL(delayRemoveChanged()), this, SLOT(destroyRemoved()));
                 ++it;
             } else {
+                if (hasRemoveTransition)
+                    d->runDelayedRemoveTransition = true;
                 d->releaseItem(item);
                 it = d->visibleItems.erase(it);
             }
diff --git a/tests/auto/quick/qquickgridview/data/qtbug57225.qml b/tests/auto/quick/qquickgridview/data/qtbug57225.qml
index 7f5e67822c..3871e5d273 100644
--- a/tests/auto/quick/qquickgridview/data/qtbug57225.qml
+++ b/tests/auto/quick/qquickgridview/data/qtbug57225.qml
@@ -52,6 +52,7 @@ Rectangle {
     GridView {
         id: grid
 
+        property int displaceTransitionsDone: 0
         property bool animationDone: false
         property point minimumPosition: Qt.point(0, 0)
 
@@ -73,8 +74,8 @@ Rectangle {
                     properties: "x,y"
                     duration: root.duration
                     easing.type: Easing.OutBounce
-                    easing.amplitude: 10.0      // longer-lasting bounce to trigger bug
                 }
+                ScriptAction { script: grid.displaceTransitionsDone += 1 }
             }
         }
 
diff --git a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp
index 7333cc3ceb..388ecc2ab8 100644
--- a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp
+++ b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp
@@ -154,6 +154,7 @@ private slots:
     void multipleTransitions_data();
     void multipleDisplaced();
     void regression_QTBUG_57225();
+    void regression_QTBUG_57225_data();
 
     void inserted_leftToRight_RtL_TtB();
     void inserted_leftToRight_RtL_TtB_data();
@@ -5801,10 +5802,15 @@ void tst_QQuickGridView::multipleDisplaced()
 
 void tst_QQuickGridView::regression_QTBUG_57225()
 {
+    QFETCH(int, initialCount);
+    QFETCH(int, removeIndex);
+    QFETCH(int, removeCount);
+    QFETCH(int, expectedDisplaceTransitions);
+
     // deleting all visible items should not cause a repositioning of said items.
 
     QaimModel model;
-    for (int i = 0; i < 20; i++)
+    for (int i = 0; i < initialCount; i++)
         model.addItem("Original item" + QString::number(i), "");
 
     QQuickView *window = createView();
@@ -5818,7 +5824,7 @@ void tst_QQuickGridView::regression_QTBUG_57225()
     QVERIFY(gridview != 0);
     QTRY_COMPARE(QQuickItemPrivate::get(gridview)->polishScheduled, false);
 
-    model.removeItems(0, 8);
+    model.removeItems(removeIndex, removeCount);
     QTRY_VERIFY(gridview->property("animationDone").toBool());
 
     // verify that none of the removed items has moved to a negative position
@@ -5826,9 +5832,33 @@ void tst_QQuickGridView::regression_QTBUG_57225()
     QVERIFY(minimumPosition.x() >= 0);
     QVERIFY(minimumPosition.y() >= 0);
 
+    // wait some more time to let the displaced transition happen
+    QTest::qWait(window->rootObject()->property("duration").toInt());
+    QTRY_VERIFY2(gridview->property("displaceTransitionsDone").toInt() >= expectedDisplaceTransitions,
+                 QByteArray::number(gridview->property("displaceTransitionsDone").toInt()).constData());
+
     delete window;
 }
 
+void tst_QQuickGridView::regression_QTBUG_57225_data()
+{
+    QTest::addColumn<int>("initialCount");
+    QTest::addColumn<int>("removeIndex");
+    QTest::addColumn<int>("removeCount");
+    QTest::addColumn<int>("expectedDisplaceTransitions");
+
+    // no displace transitions should happen
+    QTest::newRow("remove all visible items") <<
+        20 << 0 << 8 << 0;
+
+    // check that the removal animation is performed
+    QTest::newRow("remove items in between") <<
+        20 << 1 << 2 << 3;
+
+    QTest::newRow("remove items in between - 2") <<
+        20 << 2 << 3 << 1;
+}
+
 void tst_QQuickGridView::cacheBuffer()
 {
     QQuickView *window = createView();