diff options
-rw-r--r-- | .gitignore | 1 | ||||
-rw-r--r-- | src/quicktemplates2/qquickscrollview.cpp | 66 | ||||
-rw-r--r-- | src/quicktemplates2/qquicktooltip.cpp | 3 | ||||
-rw-r--r-- | tests/auto/controls/data/tst_scrollview.qml | 89 | ||||
-rw-r--r-- | tests/auto/controls/data/tst_tooltip.qml | 65 |
5 files changed, 196 insertions, 28 deletions
@@ -112,6 +112,7 @@ Makefile* *.Release *.prl *.pro.user +*.pro.user.* *.qmlproject.user* moc_*.cpp ui_*.h diff --git a/src/quicktemplates2/qquickscrollview.cpp b/src/quicktemplates2/qquickscrollview.cpp index 7890a30a..c0f73692 100644 --- a/src/quicktemplates2/qquickscrollview.cpp +++ b/src/quicktemplates2/qquickscrollview.cpp @@ -129,8 +129,8 @@ public: QQuickFlickable *ensureFlickable(bool content); bool setFlickable(QQuickFlickable *flickable, bool content); - void updateContentWidth(); - void updateContentHeight(); + void flickableContentWidthChanged(); + void flickableContentHeightChanged(); qreal getContentWidth() const override; qreal getContentHeight() const override; @@ -154,7 +154,8 @@ public: bool wasTouched = false; QQuickFlickable *flickable = nullptr; - bool ownsFlickable = false; + bool flickableHasExplicitContentWidth = true; + bool flickableHasExplicitContentHeight = true; }; QList<QQuickItem *> QQuickScrollViewPrivate::contentChildItems() const @@ -176,8 +177,9 @@ QQuickFlickable *QQuickScrollViewPrivate::ensureFlickable(bool content) { Q_Q(QQuickScrollView); if (!flickable) { + flickableHasExplicitContentWidth = false; + flickableHasExplicitContentHeight = false; setFlickable(new QQuickFlickable(q), content); - ownsFlickable = true; } return flickable; } @@ -197,8 +199,8 @@ bool QQuickScrollViewPrivate::setFlickable(QQuickFlickable *item, bool content) QQuickScrollBarAttachedPrivate::get(attached)->setFlickable(nullptr); QObjectPrivate::disconnect(flickable->contentItem(), &QQuickItem::childrenChanged, this, &QQuickPanePrivate::contentChildrenChange); - QObjectPrivate::disconnect(flickable, &QQuickFlickable::contentWidthChanged, this, &QQuickScrollViewPrivate::updateContentWidth); - QObjectPrivate::disconnect(flickable, &QQuickFlickable::contentHeightChanged, this, &QQuickScrollViewPrivate::updateContentHeight); + QObjectPrivate::disconnect(flickable, &QQuickFlickable::contentWidthChanged, this, &QQuickScrollViewPrivate::flickableContentWidthChanged); + QObjectPrivate::disconnect(flickable, &QQuickFlickable::contentHeightChanged, this, &QQuickScrollViewPrivate::flickableContentHeightChanged); } flickable = item; @@ -210,24 +212,24 @@ bool QQuickScrollViewPrivate::setFlickable(QQuickFlickable *item, bool content) if (hasContentWidth) flickable->setContentWidth(contentWidth); else - updateContentWidth(); + flickableContentWidthChanged(); if (hasContentHeight) flickable->setContentHeight(contentHeight); else - updateContentHeight(); + flickableContentHeightChanged(); if (attached) QQuickScrollBarAttachedPrivate::get(attached)->setFlickable(flickable); QObjectPrivate::connect(flickable->contentItem(), &QQuickItem::childrenChanged, this, &QQuickPanePrivate::contentChildrenChange); - QObjectPrivate::connect(flickable, &QQuickFlickable::contentWidthChanged, this, &QQuickScrollViewPrivate::updateContentWidth); - QObjectPrivate::connect(flickable, &QQuickFlickable::contentHeightChanged, this, &QQuickScrollViewPrivate::updateContentHeight); + QObjectPrivate::connect(flickable, &QQuickFlickable::contentWidthChanged, this, &QQuickScrollViewPrivate::flickableContentWidthChanged); + QObjectPrivate::connect(flickable, &QQuickFlickable::contentHeightChanged, this, &QQuickScrollViewPrivate::flickableContentHeightChanged); } return true; } -void QQuickScrollViewPrivate::updateContentWidth() +void QQuickScrollViewPrivate::flickableContentWidthChanged() { Q_Q(QQuickScrollView); if (!flickable || !componentComplete) @@ -237,11 +239,12 @@ void QQuickScrollViewPrivate::updateContentWidth() if (qFuzzyCompare(cw, implicitContentWidth)) return; + flickableHasExplicitContentWidth = true; implicitContentWidth = cw; emit q->implicitContentWidthChanged(); } -void QQuickScrollViewPrivate::updateContentHeight() +void QQuickScrollViewPrivate::flickableContentHeightChanged() { Q_Q(QQuickScrollView); if (!flickable || !componentComplete) @@ -251,29 +254,32 @@ void QQuickScrollViewPrivate::updateContentHeight() if (qFuzzyCompare(ch, implicitContentHeight)) return; + flickableHasExplicitContentHeight = true; implicitContentHeight = ch; emit q->implicitContentHeightChanged(); } qreal QQuickScrollViewPrivate::getContentWidth() const { - if (flickable) { - const qreal cw = flickable->contentWidth(); - if (cw > 0) - return cw; - } + if (flickable && flickableHasExplicitContentWidth) + return flickable->contentWidth(); + // The scrollview wraps a flickable created by us, and nobody searched for it and + // modified its contentWidth. In that case, since the application does not control + // this flickable, we fall back to calculate the content width based on the child + // items inside it. return QQuickPanePrivate::getContentWidth(); } qreal QQuickScrollViewPrivate::getContentHeight() const { - if (flickable) { - const qreal ch = flickable->contentHeight(); - if (ch > 0) - return ch; - } + if (flickable && flickableHasExplicitContentHeight) + return flickable->contentHeight(); + // The scrollview wraps a flickable created by us, and nobody searched for it and + // modified its contentHeight. In that case, since the application does not control + // this flickable, we fall back to calculate the content height based on the child + // items inside it. return QQuickPanePrivate::getContentHeight(); } @@ -557,7 +563,13 @@ void QQuickScrollView::componentComplete() void QQuickScrollView::contentItemChange(QQuickItem *newItem, QQuickItem *oldItem) { Q_D(QQuickScrollView); - d->setFlickable(qobject_cast<QQuickFlickable *>(newItem), false); + if (newItem != d->flickable) { + // The new flickable was not created by us. In that case, we always + // assume/require that it has an explicit content size assigned. + d->flickableHasExplicitContentWidth = true; + d->flickableHasExplicitContentHeight = true; + d->setFlickable(qobject_cast<QQuickFlickable *>(newItem), false); + } QQuickPane::contentItemChange(newItem, oldItem); } @@ -566,15 +578,15 @@ void QQuickScrollView::contentSizeChange(const QSizeF &newSize, const QSizeF &ol Q_D(QQuickScrollView); QQuickPane::contentSizeChange(newSize, oldSize); if (d->flickable) { - // Only set the content size on the flickable if we created the - // flickable ourselves. Otherwise we can end up overwriting + // Only set the content size on the flickable if the flickable doesn't + // have an explicit assignment from before. Otherwise we can end up overwriting // assignments done to those properties by the application. The // exception is if the application has assigned a content size // directly to the scrollview, which will then win even if the // application has assigned something else to the flickable. - if (d->hasContentWidth || d->ownsFlickable) + if (d->hasContentWidth || !d->flickableHasExplicitContentWidth) d->flickable->setContentWidth(newSize.width()); - if (d->hasContentHeight || d->ownsFlickable) + if (d->hasContentHeight || !d->flickableHasExplicitContentHeight) d->flickable->setContentHeight(newSize.height()); } } diff --git a/src/quicktemplates2/qquicktooltip.cpp b/src/quicktemplates2/qquicktooltip.cpp index 00090f66..ddf434a2 100644 --- a/src/quicktemplates2/qquicktooltip.cpp +++ b/src/quicktemplates2/qquicktooltip.cpp @@ -534,7 +534,8 @@ void QQuickToolTipAttached::show(const QString &text, int ms) tip->resetHeight(); tip->setParentItem(qobject_cast<QQuickItem *>(parent())); tip->setDelay(d->delay); - tip->show(text, ms >= 0 ? ms : d->timeout); + tip->setTimeout(ms >= 0 ? ms : d->timeout); + tip->show(text); } /*! diff --git a/tests/auto/controls/data/tst_scrollview.qml b/tests/auto/controls/data/tst_scrollview.qml index 4d5b1e5b..87c39509 100644 --- a/tests/auto/controls/data/tst_scrollview.qml +++ b/tests/auto/controls/data/tst_scrollview.qml @@ -146,6 +146,40 @@ TestCase { } Component { + id: scrollableFlickable + ScrollView { + Flickable { + Item { + width: 100 + height: 100 + } + } + } + } + + Component { + id: scrollableWithContentSize + ScrollView { + contentWidth: 1000 + contentHeight: 1000 + Flickable { + } + } + } + + Component { + id: scrollableAndFlicableWithContentSize + ScrollView { + contentWidth: 1000 + contentHeight: 1000 + Flickable { + contentWidth: 200 + contentHeight: 200 + } + } + } + + Component { id: scrollableTextArea ScrollView { TextArea { @@ -257,6 +291,61 @@ TestCase { compare(control.contentHeight, listview.contentHeight) } + function test_scrollableFlickable() { + // Check that if the application adds a flickable as a child of a + // scrollview, the scrollview doesn't try to calculate and change + // the flickables contentWidth/Height based on the flickables + // children, even if the flickable has an empty or negative content + // size. Some flickables (e.g ListView) sets a negative + // contentWidth on purpose, which should be respected. + var scrollview = createTemporaryObject(scrollableFlickable, testCase) + verify(scrollview) + + var flickable = scrollview.contentItem + verify(flickable.hasOwnProperty("contentX")) + verify(flickable.hasOwnProperty("contentY")) + + compare(flickable.contentWidth, -1) + compare(flickable.contentHeight, -1) + compare(scrollview.contentWidth, -1) + compare(scrollview.contentHeight, -1) + } + + function test_scrollableWithContentSize() { + // Check that if the scrollview has contentWidth/Height set, but + // not the flickable, then those values will be forwarded and used + // by the flickable (rather than trying to calculate the content size + // based on the flickables children). + var scrollview = createTemporaryObject(scrollableWithContentSize, testCase) + verify(scrollview) + + var flickable = scrollview.contentItem + verify(flickable.hasOwnProperty("contentX")) + verify(flickable.hasOwnProperty("contentY")) + + compare(flickable.contentWidth, 1000) + compare(flickable.contentHeight, 1000) + compare(scrollview.contentWidth, 1000) + compare(scrollview.contentHeight, 1000) + } + + function test_scrollableAndFlickableWithContentSize() { + // Check that if both the scrollview and the flickable has + // contentWidth/Height set (which is an inconsistency/fault + // by the app), the content size of the scrollview wins. + var scrollview = createTemporaryObject(scrollableAndFlicableWithContentSize, testCase) + verify(scrollview) + + var flickable = scrollview.contentItem + verify(flickable.hasOwnProperty("contentX")) + verify(flickable.hasOwnProperty("contentY")) + + compare(flickable.contentWidth, 1000) + compare(flickable.contentHeight, 1000) + compare(scrollview.contentWidth, 1000) + compare(scrollview.contentHeight, 1000) + } + function test_flickableWithExplicitContentSize() { var control = createTemporaryObject(emptyFlickable, testCase) verify(control) diff --git a/tests/auto/controls/data/tst_tooltip.qml b/tests/auto/controls/data/tst_tooltip.qml index 8a855ce0..18911895 100644 --- a/tests/auto/controls/data/tst_tooltip.qml +++ b/tests/auto/controls/data/tst_tooltip.qml @@ -347,4 +347,69 @@ TestCase { verify(tip.visible) tryCompare(tip, "visible", false) } + + Component { + id: timeoutButtonRowComponent + + Row { + Button { + text: "Timeout: 1" + ToolTip.text: text + ToolTip.visible: down + ToolTip.timeout: 1 + } + + Button { + text: "Timeout: -1" + ToolTip.text: text + ToolTip.visible: down + } + } + } + + // QTBUG-74226 + function test_attachedTimeout() { + var row = createTemporaryObject(timeoutButtonRowComponent, testCase) + verify(row) + + // Press the button that has no timeout; it should stay visible. + var button2 = row.children[1] + mousePress(button2) + compare(button2.down, true) + tryCompare(button2.ToolTip.toolTip, "opened", true) + + // Wait a bit to make sure that it's still visible. + wait(50) + compare(button2.ToolTip.toolTip.opened, true) + + // Release and should close. + mouseRelease(button2) + compare(button2.down, false) + tryCompare(button2.ToolTip, "visible", false) + + // Now, press the first button that does have a timeout; it should close on its own eventually. + var button1 = row.children[0] + mousePress(button1) + compare(button1.down, true) + // We use a short timeout to speed up the test, but tryCompare(...opened, true) then + // fails because the dialog has already been hidden by that point, so just check that it's + // immediately visible, which is more or less the same thing. + compare(button1.ToolTip.visible, true) + tryCompare(button1.ToolTip, "visible", false) + mouseRelease(button2) + + // Now, hover over the second button again. It should still stay visible until the mouse is released. + mousePress(button2) + compare(button2.down, true) + tryCompare(button2.ToolTip.toolTip, "opened", true) + + // Wait a bit to make sure that it's still visible. + wait(50) + compare(button2.ToolTip.toolTip.opened, true) + + // Release and should close. + mouseRelease(button2) + compare(button2.down, false) + tryCompare(button2.ToolTip, "visible", false) + } } |