diff options
author | Mitch Curtis <mitch.curtis@qt.io> | 2018-12-04 15:44:00 +0100 |
---|---|---|
committer | Mitch Curtis <mitch.curtis@qt.io> | 2019-01-08 13:15:09 +0000 |
commit | 9c7429219d36e8eb40e1fe6e679715c89209fc40 (patch) | |
tree | d5a4999c774adf49f724886995ad05e3387caa7d | |
parent | ce9940ca550d70931248a98fc4d47f10959a9a9e (diff) |
SplitView: schedule another layout if requested sizes changed during layout
As mentioned in the review of ed87e837, there could be a scenario where the
user sets the preferred size of an item inside the onWidthChanged handler
of another item:
onWidthChanged: if (width < 10) secondItem.SplitView.preferredWidth = 100
Before this patch, this would result in the preferredWidth assignment being
ignored since it happened during a layout.
This patch adds some auto tests to ensure that this works, as the previous
patch (that converted layouts to be done in polish/updatePolish cycles)
already fixed the issue.
It also adds a check to avoid doing too many layouts in the case of
one of the split handles being dragged.
Change-Id: Ide519b33a2fa3bf746ae3793e0671fd1750c70d8
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
-rw-r--r-- | src/quicktemplates2/qquicksplitview.cpp | 49 | ||||
-rw-r--r-- | src/quicktemplates2/qquicksplitview_p_p.h | 2 | ||||
-rw-r--r-- | tests/auto/controls/data/tst_splitview.qml | 73 |
3 files changed, 119 insertions, 5 deletions
diff --git a/src/quicktemplates2/qquicksplitview.cpp b/src/quicktemplates2/qquicksplitview.cpp index 64ccb6c7..502d2b3a 100644 --- a/src/quicktemplates2/qquicksplitview.cpp +++ b/src/quicktemplates2/qquicksplitview.cpp @@ -383,6 +383,20 @@ void QQuickSplitViewPrivate::layoutResizeSplitItems(qreal &usedWidth, qreal &use attached = qobject_cast<QQuickSplitViewAttached*>( qmlAttachedPropertiesObject<QQuickSplitView>(item, true)); } + + /* + Users could conceivably respond to size changes in items by setting attached + SplitView properties: + + onWidthChanged: if (width < 10) secondItem.SplitView.preferredWidth = 100 + + We handle this by doing another layout after the current layout if the + attached/implicit size properties are set during this layout. However, we also + need to set preferredWidth/Height here (for reasons mentioned in the comment above), + but we don't want this to count as a request for a delayed layout, so we guard against it. + */ + m_ignoreNextLayoutRequest = true; + if (horizontal) attached->setPreferredWidth(newItemSize); else @@ -430,14 +444,17 @@ void QQuickSplitViewPrivate::layoutResizeSplitItems(qreal &usedWidth, qreal &use attached = qobject_cast<QQuickSplitViewAttached*>( qmlAttachedPropertiesObject<QQuickSplitView>(item, true)); } + + m_ignoreNextLayoutRequest = true; + if (horizontal) attached->setPreferredWidth(newItemSize); else attached->setPreferredHeight(newItemSize); - // We still need to use requestedWidth in the setWidth() call below, + // We still need to use requestedSize in the setWidth()/setHeight() call below, // because sizeData has already been calculated and now contains an old - // effectivePreferredWidth value. + // effectivePreferredWidth/Height value. requestedSize = newItemSize; qCDebug(qlcQQuickSplitView).nospace() << " - " << index << ": resized (dragged) " << item @@ -1618,11 +1635,23 @@ void QQuickSplitViewAttached::setPreferredWidth(qreal width) { Q_D(QQuickSplitViewAttached); d->m_isPreferredWidthSet = true; + // Make sure that we clear this flag now, before we emit the change signals + // which could cause another setter to be called. + auto splitViewPrivate = QQuickSplitViewPrivate::get(d->m_splitView); + const bool ignoreNextLayoutRequest = splitViewPrivate->m_ignoreNextLayoutRequest; + splitViewPrivate->m_ignoreNextLayoutRequest = false; + if (qFuzzyCompare(width, d->m_preferredWidth)) return; d->m_preferredWidth = width; - d->requestLayoutView(); + + if (!ignoreNextLayoutRequest) { + // We are currently in the middle of performing a layout, and the user (not our internal code) + // changed the preferred width of one of the split items, so request another layout. + d->requestLayoutView(); + } + emit preferredWidthChanged(); } @@ -1674,11 +1703,23 @@ void QQuickSplitViewAttached::setPreferredHeight(qreal height) { Q_D(QQuickSplitViewAttached); d->m_isPreferredHeightSet = true; + // Make sure that we clear this flag now, before we emit the change signals + // which could cause another setter to be called. + auto splitViewPrivate = QQuickSplitViewPrivate::get(d->m_splitView); + const bool ignoreNextLayoutRequest = splitViewPrivate->m_ignoreNextLayoutRequest; + splitViewPrivate->m_ignoreNextLayoutRequest = false; + if (qFuzzyCompare(height, d->m_preferredHeight)) return; d->m_preferredHeight = height; - d->requestLayoutView(); + + if (!ignoreNextLayoutRequest) { + // We are currently in the middle of performing a layout, and the user (not our internal code) + // changed the preferred height of one of the split items, so request another layout. + d->requestLayoutView(); + } + emit preferredHeightChanged(); } diff --git a/src/quicktemplates2/qquicksplitview_p_p.h b/src/quicktemplates2/qquicksplitview_p_p.h index 560276f0..5d71d461 100644 --- a/src/quicktemplates2/qquicksplitview_p_p.h +++ b/src/quicktemplates2/qquicksplitview_p_p.h @@ -106,7 +106,6 @@ public: static QQuickSplitViewPrivate *get(QQuickSplitView *splitView); -private: Qt::Orientation m_orientation = Qt::Horizontal; QQmlComponent *m_handle = nullptr; QVector<QQuickItem*> m_handleItems; @@ -119,6 +118,7 @@ private: qreal m_rightOrBottomItemSizeBeforePress = 0.0; int m_fillIndex = -1; bool m_layingOut = false; + bool m_ignoreNextLayoutRequest = false; bool m_resizing = false; }; diff --git a/tests/auto/controls/data/tst_splitview.qml b/tests/auto/controls/data/tst_splitview.qml index 40e7dda0..28c00179 100644 --- a/tests/auto/controls/data/tst_splitview.qml +++ b/tests/auto/controls/data/tst_splitview.qml @@ -1847,4 +1847,77 @@ TestCase { control.restoreState(settings.value("splitView")) compare(lastItem.SplitView.preferredWidth, 123) } + + function test_changePreferredSizeDuringLayout() { + var control = createTemporaryObject(threeSizedItemsComponent, testCase) + verify(control) + + var firstItem = control.itemAt(0) + var secondItem = control.itemAt(1) + secondItem.widthChanged.connect(function() { + if (secondItem.width < 10) + firstItem.SplitView.preferredWidth = 50 + }) + + // Change the size of the item so that a layout happens, but + // make the size small enough that the item's onWidthChanged handler gets triggered. + // The onWidthChanged handler will set the preferredWidth of another item during the + // layout, so we need to make sure the assignment isn't lost since we return early in that case. + secondItem.implicitWidth = 5 + verify(isPolishScheduled(control)) + verify(waitForItemPolished(control)) + compare(secondItem.width, 5) + compare(firstItem.width, 50) + + // Now do the same for height. + control.orientation = Qt.Vertical + secondItem.heightChanged.connect(function() { + if (secondItem.height < 10) + firstItem.SplitView.preferredHeight = 50 + }) + // Get the polishes for the orientation out of the way so that they + // don't intefere with our results. + verify(isPolishScheduled(control)) + verify(waitForItemPolished(control)) + + secondItem.implicitHeight = 5 + verify(isPolishScheduled(control)) + verify(waitForItemPolished(control)) + compare(secondItem.height, 5) + compare(firstItem.height, 50) + } + + // When the user drags a handle, we internally set preferredWidth/Height + // to reflect the new value. However, we also have to make sure that when + // we do so, it doesn't trigger a delayed layout. This is why we have + // m_ignoreNextDelayedLayoutRequest. This test checks that + // m_ignoreNextDelayedLayoutRequest doesn't interfere with any action from + // the user that results in a delayed layout. + function test_changePreferredSizeDuringLayoutWhileDraggingHandle() { + var control = createTemporaryObject(threeSizedItemsComponent, testCase) + verify(control) + + var firstItem = control.itemAt(0) + var secondItem = control.itemAt(1) + firstItem.widthChanged.connect(function() { + if (firstItem.width === 0) + secondItem.SplitView.preferredWidth = 50 + }) + + // Start dragging the handle. + var handles = findHandles(control) + var targetHandle = handles[0] + mousePress(targetHandle) + verify(control.resizing) + var localPos = testCase.mapToItem(targetHandle, 15, testCase.height / 2) + + // Move the handle to the very left, so that the item's width becomes zero. + mouseMove(targetHandle, -100, targetHandle.height / 2) + verify(control.resizing) + compare(firstItem.width, 0) + compare(secondItem.SplitView.preferredWidth, 50) + compare(secondItem.width, 50) + mouseRelease(targetHandle, -100, targetHandle.height / 2, Qt.LeftButton) + verify(!control.resizing) + } } |