From 965fa9450c3e3424e5e9e4a1bec3612a66594059 Mon Sep 17 00:00:00 2001 From: Samuel Ghinet Date: Thu, 15 Dec 2022 16:38:38 +0200 Subject: [PATCH] QmlDesigner: Fix cancel button not reverting background color In the 3D view, when the user experiments with different background colors, but then decides to cancel, upon pressing the Cancel button and closing the dialog, the last color picked remained set. This happened on Qt6.4 and not on Qt6.3. The reason for this was the usage of Array.isArray() on an object exposed from C++ to QML of type QVariant that could hold either a QList or a a QColor. It looks like there is no supported way from qml/js to see if a variable is a list / sequence or a scalar. So the best way would be to always work with QList even though in most cases only one single QColor is being used. Task-number: QDS-8143 Change-Id: Ia7e8e1facad24439ad244305c213bb12e286105b Reviewed-by: Miikka Heikkinen Reviewed-by: Mahmoud Badri --- .../edit3d/backgroundcolorselection.cpp | 8 +++---- .../components/edit3d/edit3dview.cpp | 8 +++---- .../components/edit3d/edit3dviewconfig.h | 21 ++++-------------- .../qml2puppet/mockfiles/qt6/EditView3D.qml | 22 +++++++------------ .../qt5informationnodeinstanceserver.cpp | 7 +++--- 5 files changed, 24 insertions(+), 42 deletions(-) diff --git a/src/plugins/qmldesigner/components/edit3d/backgroundcolorselection.cpp b/src/plugins/qmldesigner/components/edit3d/backgroundcolorselection.cpp index 07d65cb3c33..42f5bc45dd5 100644 --- a/src/plugins/qmldesigner/components/edit3d/backgroundcolorselection.cpp +++ b/src/plugins/qmldesigner/components/edit3d/backgroundcolorselection.cpp @@ -44,7 +44,7 @@ QColorDialog *BackgroundColorSelection::createColorDialog(QWidget *parent, QObject::connect(dialog, &QColorDialog::currentColorChanged, dialog, [actionType, view](const QColor &color) { - Edit3DViewConfig::setColor(view, actionType, color); + Edit3DViewConfig::setColors(view, actionType, {color}); }); QObject::connect(dialog, &QColorDialog::colorSelected, dialog, @@ -52,13 +52,13 @@ QColorDialog *BackgroundColorSelection::createColorDialog(QWidget *parent, if (colorSelected) colorSelected(); - Edit3DViewConfig::saveColor(key, color); + Edit3DViewConfig::saveColors(key, {color}); }); - if (Edit3DViewConfig::isColorValid(oldColorConfig)) { + if (Edit3DViewConfig::colorsValid(oldColorConfig)) { QObject::connect(dialog, &QColorDialog::rejected, dialog, [actionType, oldColorConfig, view]() { - Edit3DViewConfig::setColor(view, actionType, oldColorConfig); + Edit3DViewConfig::setColors(view, actionType, oldColorConfig); }); } diff --git a/src/plugins/qmldesigner/components/edit3d/edit3dview.cpp b/src/plugins/qmldesigner/components/edit3d/edit3dview.cpp index b7efef60d6a..75c2138ffc0 100644 --- a/src/plugins/qmldesigner/components/edit3d/edit3dview.cpp +++ b/src/plugins/qmldesigner/components/edit3d/edit3dview.cpp @@ -389,12 +389,12 @@ Edit3DAction *Edit3DView::createResetColorAction(QAction *syncBackgroundColorAct auto operation = [this, syncBackgroundColorAction](const SelectionContext &) { QList bgColors = {QRgb(0x222222), QRgb(0x999999)}; - Edit3DViewConfig::setColor(this, View3DActionType::SelectBackgroundColor, bgColors); - Edit3DViewConfig::saveColor(DesignerSettingsKey::EDIT3DVIEW_BACKGROUND_COLOR, bgColors); + Edit3DViewConfig::setColors(this, View3DActionType::SelectBackgroundColor, bgColors); + Edit3DViewConfig::saveColors(DesignerSettingsKey::EDIT3DVIEW_BACKGROUND_COLOR, bgColors); QColor gridColor{0xaaaaaa}; - Edit3DViewConfig::setColor(this, View3DActionType::SelectGridColor, gridColor); - Edit3DViewConfig::saveColor(DesignerSettingsKey::EDIT3DVIEW_GRID_COLOR, gridColor); + Edit3DViewConfig::setColors(this, View3DActionType::SelectGridColor, {gridColor}); + Edit3DViewConfig::saveColors(DesignerSettingsKey::EDIT3DVIEW_GRID_COLOR, {gridColor}); if (syncBackgroundColorAction->isChecked()) { Edit3DViewConfig::set(this, View3DActionType::SyncBackgroundColor, false); diff --git a/src/plugins/qmldesigner/components/edit3d/edit3dviewconfig.h b/src/plugins/qmldesigner/components/edit3d/edit3dviewconfig.h index 267e27042c9..e0eba1115c9 100644 --- a/src/plugins/qmldesigner/components/edit3d/edit3dviewconfig.h +++ b/src/plugins/qmldesigner/components/edit3d/edit3dviewconfig.h @@ -28,17 +28,9 @@ public: }); } - static void setColor(AbstractView *view, View3DActionType type, const QList &colorConfig) + static void setColors(AbstractView *view, View3DActionType type, const QList &colorConfig) { - if (colorConfig.size() == 1) - setColor(view, type, colorConfig.at(0)); - else - setVariant(view, type, QVariant::fromValue(colorConfig)); - } - - static void setColor(AbstractView *view, View3DActionType type, const QColor &color) - { - setVariant(view, type, QVariant::fromValue(color)); + setVariant(view, type, QVariant::fromValue(colorConfig)); } template @@ -47,7 +39,7 @@ public: setVariant(view, type, QVariant::fromValue(value)); } - static void saveColor(const QByteArray &key, const QList &colorConfig) + static void saveColors(const QByteArray &key, const QList &colorConfig) { QStringList colorNames = Utils::transform(colorConfig, [](const QColor &color) { return color.name(); @@ -56,12 +48,7 @@ public: saveVariant(key, QVariant::fromValue(colorNames)); } - static void saveColor(const QByteArray &key, const QColor &color) - { - saveVariant(key, QVariant::fromValue(color.name())); - } - - static bool isColorValid(const QList &colorConfig) { return !colorConfig.isEmpty(); } + static bool colorsValid(const QList &colorConfig) { return !colorConfig.isEmpty(); } private: static void setVariant(AbstractView *view, View3DActionType type, const QVariant &colorConfig) diff --git a/src/tools/qml2puppet/mockfiles/qt6/EditView3D.qml b/src/tools/qml2puppet/mockfiles/qt6/EditView3D.qml index 819f1e2be90..7f7b148460b 100644 --- a/src/tools/qml2puppet/mockfiles/qt6/EditView3D.qml +++ b/src/tools/qml2puppet/mockfiles/qt6/EditView3D.qml @@ -194,24 +194,18 @@ Item { function updateViewStates(viewStates) { if ("selectBackgroundColor" in viewStates) { - if (Array.isArray(viewStates.selectBackgroundColor)) { - var colors = viewStates.selectBackgroundColor - if (colors.length === 1) { - backgroundGradientColorStart = colors[0]; - backgroundGradientColorEnd = colors[0]; - } else { - backgroundGradientColorStart = colors[0]; - backgroundGradientColorEnd = colors[1]; - } + var colors = viewStates.selectBackgroundColor + if (colors.length === 1) { + backgroundGradientColorStart = colors[0]; + backgroundGradientColorEnd = colors[0]; } else { - var color = viewStates.selectBackgroundColor - backgroundGradientColorStart = color; - backgroundGradientColorEnd = color; + backgroundGradientColorStart = colors[0]; + backgroundGradientColorEnd = colors[1]; } } - if ("selectGridColor" in viewStates) - viewRoot.gridColor = viewStates.selectGridColor + if ("selectGridColor" in viewStates && viewStates.selectGridColor.length === 1) + viewRoot.gridColor = viewStates.selectGridColor[0] } // If resetToDefault is true, tool states not specifically set to anything will be reset to diff --git a/src/tools/qml2puppet/qml2puppet/instances/qt5informationnodeinstanceserver.cpp b/src/tools/qml2puppet/qml2puppet/instances/qt5informationnodeinstanceserver.cpp index dbbe90c2f35..c2ef586c031 100644 --- a/src/tools/qml2puppet/qml2puppet/instances/qt5informationnodeinstanceserver.cpp +++ b/src/tools/qml2puppet/qml2puppet/instances/qt5informationnodeinstanceserver.cpp @@ -921,9 +921,9 @@ void Qt5InformationNodeInstanceServer::updateActiveSceneToEditView3D([[maybe_unu if (toolStates.contains("syncBackgroundColor")) { bool sync = toolStates["syncBackgroundColor"].toBool(); if (sync) { - QColor color = helper->sceneEnvironmentColor(sceneId); + QList colors = {helper->sceneEnvironmentColor(sceneId)}; View3DActionCommand cmd(View3DActionType::SelectBackgroundColor, - QVariant::fromValue(color)); + QVariant::fromValue(colors)); view3DAction(cmd); } } @@ -2274,9 +2274,10 @@ void Qt5InformationNodeInstanceServer::setSceneEnvironmentColor(const PropertyVa if (toolStates.contains("syncBackgroundColor")) { bool sync = toolStates["syncBackgroundColor"].toBool(); + QList colors = {color}; if (sync) { View3DActionCommand cmd(View3DActionType::SelectBackgroundColor, - QVariant::fromValue(color)); + QVariant::fromValue(colors)); view3DAction(cmd); } }