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>

5 files changed, 20 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;