diff options
author | Mitch Curtis <mitch.curtis@qt.io> | 2019-02-04 16:19:55 +0100 |
---|---|---|
committer | Mitch Curtis <mitch.curtis@qt.io> | 2019-02-05 13:28:44 +0000 |
commit | 4e5601ac1c7aec6aba9ba09fe7adb7a0462da2f0 (patch) | |
tree | bdd0c234d3ebf328c2268ffdea9b1bca9d988a8b | |
parent | a59ab3ec93bc873225ccbd118ec81d023b5cb62d (diff) |
Popup: ensure that "palette" is reevaluated when enabled state changes
When changing the enabled state of a Menu after component completion,
the background would not change. The problem was that
QQuickPopup::paletteChanged() was not being emitted when the enabled
state changed, resulting in QQuickControl::palette() (which
QQuickPopup::palette() forwards calls to via its popupItem) not
being called. QQuickControl::palette() changes the palette's current
color group to QPalette::Disabled when it is disabled, so if it's not
called, the popup's colors won't change.
Fix the issue by adding a virtual enabledChange() function to
QQuickControl, which QQuickPopupItem can override to emit both
QQuickPopup::enabledChanged() (saving a connection in the process)
and QQuickPopup::paletteChanged(). This ensures that bindings to
QQuickPopup::palette are re-evaluated.
The patch adds a virtual instead of just emitting paletteChanged()
in QQuickPopup::setEnabled(), because we also want to be notified
of indirect enabled state changes, such as those coming from
parent items.
Change-Id: Ibdbd05f27b5a74fe731bda9d6698cbf6b8f81785
Fixes: QTBUG-73447
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
-rw-r--r-- | src/quicktemplates2/qquickcontrol.cpp | 5 | ||||
-rw-r--r-- | src/quicktemplates2/qquickcontrol_p.h | 1 | ||||
-rw-r--r-- | src/quicktemplates2/qquickpopup.cpp | 1 | ||||
-rw-r--r-- | src/quicktemplates2/qquickpopupitem.cpp | 13 | ||||
-rw-r--r-- | src/quicktemplates2/qquickpopupitem_p_p.h | 1 | ||||
-rw-r--r-- | tests/auto/qquickpopup/data/disabledPalette.qml | 72 | ||||
-rw-r--r-- | tests/auto/qquickpopup/tst_qquickpopup.cpp | 88 |
7 files changed, 180 insertions, 1 deletions
diff --git a/src/quicktemplates2/qquickcontrol.cpp b/src/quicktemplates2/qquickcontrol.cpp index 1fa70662..a61df3ca 100644 --- a/src/quicktemplates2/qquickcontrol.cpp +++ b/src/quicktemplates2/qquickcontrol.cpp @@ -943,6 +943,7 @@ void QQuickControl::itemChange(QQuickItem::ItemChange change, const QQuickItem:: switch (change) { case ItemEnabledHasChanged: emit paletteChanged(); + enabledChange(); break; case ItemVisibleHasChanged: #if QT_CONFIG(quicktemplates2_hover) @@ -2184,6 +2185,10 @@ void QQuickControl::geometryChanged(const QRectF &newGeometry, const QRectF &old emit availableHeightChanged(); } +void QQuickControl::enabledChange() +{ +} + void QQuickControl::fontChange(const QFont &newFont, const QFont &oldFont) { Q_UNUSED(newFont); diff --git a/src/quicktemplates2/qquickcontrol_p.h b/src/quicktemplates2/qquickcontrol_p.h index a38e34f9..3fe20f3b 100644 --- a/src/quicktemplates2/qquickcontrol_p.h +++ b/src/quicktemplates2/qquickcontrol_p.h @@ -278,6 +278,7 @@ protected: virtual void localeChange(const QLocale &newLocale, const QLocale &oldLocale); virtual void paletteChange(const QPalette &newPalette, const QPalette &oldPalette); virtual void insetChange(const QMarginsF &newInset, const QMarginsF &oldInset); + virtual void enabledChange(); #if QT_CONFIG(accessibility) virtual QAccessible::Role accessibleRole() const; diff --git a/src/quicktemplates2/qquickpopup.cpp b/src/quicktemplates2/qquickpopup.cpp index 98c2127d..72e4f042 100644 --- a/src/quicktemplates2/qquickpopup.cpp +++ b/src/quicktemplates2/qquickpopup.cpp @@ -263,7 +263,6 @@ void QQuickPopupPrivate::init() popupItem = new QQuickPopupItem(q); popupItem->setVisible(false); q->setParentItem(qobject_cast<QQuickItem *>(parent)); - QObject::connect(popupItem, &QQuickItem::enabledChanged, q, &QQuickPopup::enabledChanged); QObject::connect(popupItem, &QQuickControl::paddingChanged, q, &QQuickPopup::paddingChanged); QObject::connect(popupItem, &QQuickControl::backgroundChanged, q, &QQuickPopup::backgroundChanged); QObject::connect(popupItem, &QQuickControl::contentItemChanged, q, &QQuickPopup::contentItemChanged); diff --git a/src/quicktemplates2/qquickpopupitem.cpp b/src/quicktemplates2/qquickpopupitem.cpp index cf2fec41..16d8c4f6 100644 --- a/src/quicktemplates2/qquickpopupitem.cpp +++ b/src/quicktemplates2/qquickpopupitem.cpp @@ -366,6 +366,19 @@ void QQuickPopupItem::paletteChange(const QPalette &newPalette, const QPalette & d->popup->paletteChange(newPalette, oldPalette); } +void QQuickPopupItem::enabledChange() +{ + Q_D(QQuickPopupItem); + // Just having QQuickPopup connect our QQuickItem::enabledChanged() signal + // to its enabledChanged() signal is enough for the enabled property to work, + // but we must also ensure that its paletteChanged() signal is emitted + // so that bindings to palette are re-evaluated, because QQuickControl::palette() + // returns a different palette depending on whether or not the control is enabled. + // To save a connection, we also emit enabledChanged here. + emit d->popup->enabledChanged(); + emit d->popup->paletteChanged(); +} + QFont QQuickPopupItem::defaultFont() const { Q_D(const QQuickPopupItem); diff --git a/src/quicktemplates2/qquickpopupitem_p_p.h b/src/quicktemplates2/qquickpopupitem_p_p.h index a15aeb17..a12e43e0 100644 --- a/src/quicktemplates2/qquickpopupitem_p_p.h +++ b/src/quicktemplates2/qquickpopupitem_p_p.h @@ -95,6 +95,7 @@ protected: void itemChange(ItemChange change, const ItemChangeData &data) override; void paddingChange(const QMarginsF &newPadding, const QMarginsF &oldPadding) override; void paletteChange(const QPalette &newPalette, const QPalette &oldPalette) override; + void enabledChange() override; QFont defaultFont() const override; QPalette defaultPalette() const override; diff --git a/tests/auto/qquickpopup/data/disabledPalette.qml b/tests/auto/qquickpopup/data/disabledPalette.qml new file mode 100644 index 00000000..f080f5e8 --- /dev/null +++ b/tests/auto/qquickpopup/data/disabledPalette.qml @@ -0,0 +1,72 @@ +/**************************************************************************** +** +** Copyright (C) 2019 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of the test suite of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:BSD$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** BSD License Usage +** Alternatively, you may use this file under the terms of the BSD license +** as follows: +** +** "Redistribution and use in source and binary forms, with or without +** modification, are permitted provided that the following conditions are +** met: +** * Redistributions of source code must retain the above copyright +** notice, this list of conditions and the following disclaimer. +** * Redistributions in binary form must reproduce the above copyright +** notice, this list of conditions and the following disclaimer in +** the documentation and/or other materials provided with the +** distribution. +** * Neither the name of The Qt Company Ltd nor the names of its +** contributors may be used to endorse or promote products derived +** from this software without specific prior written permission. +** +** +** THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +** "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +** LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +** A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +** OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +** SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +** LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +** DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +** THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +** (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +** OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE." +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +import QtQuick 2.12 +import QtQuick.Controls 2.12 + +ApplicationWindow { + width: 400 + height: 400 + + property alias popup: popup + + function disableOverlay() { + popup.Overlay.overlay.enabled = false + } + + Popup { + id: popup + width: 200 + height: 200 + background: Rectangle { + color: popup.palette.base + } + } +} diff --git a/tests/auto/qquickpopup/tst_qquickpopup.cpp b/tests/auto/qquickpopup/tst_qquickpopup.cpp index 35d05244..81b2d583 100644 --- a/tests/auto/qquickpopup/tst_qquickpopup.cpp +++ b/tests/auto/qquickpopup/tst_qquickpopup.cpp @@ -86,6 +86,8 @@ private slots: void orientation_data(); void orientation(); void qquickview(); + void disabledPalette(); + void disabledParentPalette(); }; void tst_QQuickPopup::initTestCase() @@ -1083,6 +1085,92 @@ void tst_QQuickPopup::qquickview() // QTBUG-72746: shouldn't crash on application exit after closing a Dialog when using QQuickView. } +// TODO: also test it out without setting enabled directly on menu, but on a parent + +// QTBUG-73447 +void tst_QQuickPopup::disabledPalette() +{ + QQuickApplicationHelper helper(this, "disabledPalette.qml"); + + QQuickWindow *window = helper.window; + window->show(); + QVERIFY(QTest::qWaitForWindowActive(window)); + + QQuickPopup *popup = window->property("popup").value<QQuickPopup*>(); + QVERIFY(popup); + + QSignalSpy popupEnabledSpy(popup, SIGNAL(enabledChanged())); + QVERIFY(popupEnabledSpy.isValid()); + QSignalSpy popupPaletteSpy(popup, SIGNAL(paletteChanged())); + QVERIFY(popupPaletteSpy.isValid()); + + QSignalSpy popupItemEnabledSpy(popup->popupItem(), SIGNAL(enabledChanged())); + QVERIFY(popupItemEnabledSpy.isValid()); + QSignalSpy popupItemPaletteSpy(popup->popupItem(), SIGNAL(paletteChanged())); + QVERIFY(popupItemPaletteSpy.isValid()); + + QPalette palette = popup->palette(); + palette.setColor(QPalette::Active, QPalette::Base, Qt::green); + palette.setColor(QPalette::Disabled, QPalette::Base, Qt::red); + popup->setPalette(palette); + QCOMPARE(popupPaletteSpy.count(), 1); + QCOMPARE(popupItemPaletteSpy.count(), 1); + QCOMPARE(popup->background()->property("color").value<QColor>(), Qt::green); + + popup->setEnabled(false); + QCOMPARE(popupEnabledSpy.count(), 1); + QCOMPARE(popupItemEnabledSpy.count(), 1); + QCOMPARE(popupPaletteSpy.count(), 2); + QCOMPARE(popupItemPaletteSpy.count(), 2); + QCOMPARE(popup->background()->property("color").value<QColor>(), Qt::red); +} + +void tst_QQuickPopup::disabledParentPalette() +{ + QQuickApplicationHelper helper(this, "disabledPalette.qml"); + + QQuickWindow *window = helper.window; + window->show(); + QVERIFY(QTest::qWaitForWindowActive(window)); + + QQuickPopup *popup = window->property("popup").value<QQuickPopup*>(); + QVERIFY(popup); + + QSignalSpy popupEnabledSpy(popup, SIGNAL(enabledChanged())); + QVERIFY(popupEnabledSpy.isValid()); + QSignalSpy popupPaletteSpy(popup, SIGNAL(paletteChanged())); + QVERIFY(popupPaletteSpy.isValid()); + + QSignalSpy popupItemEnabledSpy(popup->popupItem(), SIGNAL(enabledChanged())); + QVERIFY(popupItemEnabledSpy.isValid()); + QSignalSpy popupItemPaletteSpy(popup->popupItem(), SIGNAL(paletteChanged())); + QVERIFY(popupItemPaletteSpy.isValid()); + + QPalette palette = popup->palette(); + palette.setColor(QPalette::Active, QPalette::Base, Qt::green); + palette.setColor(QPalette::Disabled, QPalette::Base, Qt::red); + popup->setPalette(palette); + QCOMPARE(popupPaletteSpy.count(), 1); + QCOMPARE(popupItemPaletteSpy.count(), 1); + QCOMPARE(popup->background()->property("color").value<QColor>(), Qt::green); + + // Disable the overlay (which is QQuickPopupItem's parent) to ensure that + // the palette is changed when the popup is indirectly disabled. + popup->open(); + QTRY_VERIFY(popup->isOpened()); + QVERIFY(QMetaObject::invokeMethod(window, "disableOverlay")); + QVERIFY(!popup->isEnabled()); + QVERIFY(!popup->popupItem()->isEnabled()); + QCOMPARE(popup->background()->property("color").value<QColor>(), Qt::red); + QCOMPARE(popupEnabledSpy.count(), 1); + QCOMPARE(popupItemEnabledSpy.count(), 1); + QCOMPARE(popupPaletteSpy.count(), 2); + QCOMPARE(popupItemPaletteSpy.count(), 2); + + popup->close(); + QTRY_VERIFY(!popup->isVisible()); +} + QTEST_QUICKCONTROLS_MAIN(tst_QQuickPopup) #include "tst_qquickpopup.moc" |