From 06162b3712b6ff2e25e1b20a139fe5c42e95b123 Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Fri, 27 Sep 2019 10:44:42 +0200 Subject: StackView: fix an issue where the current item was hidden When a StackView has an item A as the current item, calling replace(B) and then replace(A) would result in A being hidden when all transitions were finished. When an item is finishing its transition, we can check to see if that item exists in the stack (i.e. was pushed while it was transitioning), and if so, don't hide it. The patch is based on the one from Anthony Groyer. Fixes: QTBUG-57267 Change-Id: I441559c54a35c577261074bc7f0c923aeb3ca330 Reviewed-by: Richard Moe Gustavsen --- src/quicktemplates2/qquickstackview_p.cpp | 22 +++++++++-- tests/auto/controls/data/tst_stackview.qml | 59 ++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/src/quicktemplates2/qquickstackview_p.cpp b/src/quicktemplates2/qquickstackview_p.cpp index 7cb943a3..a280e31d 100644 --- a/src/quicktemplates2/qquickstackview_p.cpp +++ b/src/quicktemplates2/qquickstackview_p.cpp @@ -267,7 +267,11 @@ void QQuickStackViewPrivate::viewItemTransitionFinished(QQuickItemViewTransition element->setStatus(QQuickStackView::Active); } else if (element->status == QQuickStackView::Deactivating) { element->setStatus(QQuickStackView::Inactive); - element->setVisible(false); + QQuickStackElement *existingElement = element->item ? findElement(element->item) : nullptr; + // If a different element with the same item is found, + // do not call setVisible(false) since it needs to be visible. + if (!existingElement || element == existingElement) + element->setVisible(false); if (element->removal || element->isPendingRemoval()) removed += element; } @@ -275,11 +279,21 @@ void QQuickStackViewPrivate::viewItemTransitionFinished(QQuickItemViewTransition if (transitioner && transitioner->runningJobs.isEmpty()) { // ~QQuickStackElement() emits QQuickStackViewAttached::removed(), which may be used // to modify the stack. Set the status first and make a copy of the destroyable stack - // elements to exclude any modifications that may happen during the loop. (QTBUG-62153) + // elements to exclude any modifications that may happen during qDeleteAll(). (QTBUG-62153) setBusy(false); - QList elements = removed; + QList removedElements = removed; removed.clear(); - qDeleteAll(elements); + + for (QQuickStackElement *removedElement : qAsConst(removedElements)) { + // If an element with the same item is found in the active stack list, + // forget about the item so that we don't hide it. + if (removedElement->item && findElement(removedElement->item)) { + QQuickItemPrivate::get(removedElement->item)->removeItemChangeListener(removedElement, QQuickItemPrivate::Destroyed); + removedElement->item = nullptr; + } + } + + qDeleteAll(removedElements); } removing.remove(element); diff --git a/tests/auto/controls/data/tst_stackview.qml b/tests/auto/controls/data/tst_stackview.qml index c6515f77..c15ce8ea 100644 --- a/tests/auto/controls/data/tst_stackview.qml +++ b/tests/auto/controls/data/tst_stackview.qml @@ -1345,4 +1345,63 @@ TestCase { ignoreWarning(new RegExp(".*cannot replace while already in the process of removing elements")) control.clear(StackView.Immediate) } + + Component { + id: rectangleComponent + Rectangle {} + } + + Component { + id: qtbug57267_StackViewComponent + + StackView { + id: stackView + + popEnter: Transition { + XAnimator { from: (stackView.mirrored ? -1 : 1) * -stackView.width; to: 0; duration: 400; easing.type: Easing.Linear } + } + popExit: Transition { + XAnimator { from: 0; to: (stackView.mirrored ? -1 : 1) * stackView.width; duration: 400; easing.type: Easing.Linear } + } + pushEnter: Transition { + XAnimator { from: (stackView.mirrored ? -1 : 1) * stackView.width; to: 0; duration: 400; easing.type: Easing.Linear } + } + pushExit: Transition { + XAnimator { from: 0; to: (stackView.mirrored ? -1 : 1) * -stackView.width; duration: 400; easing.type: Easing.Linear } + } + replaceEnter: Transition { + XAnimator { from: (stackView.mirrored ? -1 : 1) * stackView.width; to: 0; duration: 400; easing.type: Easing.Linear } + } + replaceExit: Transition { + XAnimator { from: 0; to: (stackView.mirrored ? -1 : 1) * -stackView.width; duration: 400; easing.type: Easing.Linear } + } + } + } + + function test_qtbug57267() { + let redRect = createTemporaryObject(rectangleComponent, testCase, { color: "red" }) + verify(redRect) + let blueRect = createTemporaryObject(rectangleComponent, testCase, { color: "blue" }) + verify(blueRect) + let control = createTemporaryObject(qtbug57267_StackViewComponent, testCase, + { "anchors.fill": testCase, initialItem: redRect }) + verify(control) + + control.replace(blueRect) + compare(control.currentItem, blueRect) + compare(control.depth, 1) + + // Wait until the animation has started and then interrupt it by pushing the redRect. + tryCompare(control, "busy", true) + control.replace(redRect) + // The blue rect shouldn't be visible since we replaced it and therefore interrupted its animation. + tryCompare(blueRect, "visible", false) + // We did the replace very early on, so the transition for the redRect should still be happening. + compare(control.busy, true) + compare(redRect.visible, true) + + // After finishing the transition, the red rect should still be visible. + tryCompare(control, "busy", false) + compare(redRect.visible, true) + } } -- cgit v1.2.3