Fix color selection issues

- Color dialog selection only affects the selected property
- Timebar color setting is now actually committed
- Canceling color dialog when setting color property value in action
  palette now restores the old color

Task-number: QT3DS-2080
Change-Id: I67f6869f59c1840d76edcbaff5f9026cb3e00194
Reviewed-by: Mahmoud Badri <mahmoud.badri@qt.io>
Reviewed-by: Tomi Korpipää <tomi.korpipaa@qt.io>

7 files changed, 29 insertions, 40 deletions

diff --git a/src/Authoring/Studio/Palettes/Action/ActionView.cpp b/src/Authoring/Studio/Palettes/Action/ActionView.cpp
index 14d42d3a..1cb5ab2d 100644
--- a/src/Authoring/Studio/Palettes/Action/ActionView.cpp
+++ b/src/Authoring/Studio/Palettes/Action/ActionView.cpp
@@ -1074,18 +1074,12 @@ QColor ActionView::showColorDialog(const QColor &color)
 {
     m_currentColor = color;
     CDialogs *dialogs = g_StudioApp.GetDialogs();
-    connect(dialogs, &CDialogs::onColorChanged, this, &ActionView::changeColor);
+    connect(dialogs, &CDialogs::onColorChanged, this, &ActionView::dialogCurrentColorChanged);
     QColor currentColor = dialogs->displayColorDialog(color);
-    disconnect(dialogs, &CDialogs::onColorChanged, this, &ActionView::changeColor);
+    disconnect(dialogs, &CDialogs::onColorChanged, this, &ActionView::dialogCurrentColorChanged);
     return currentColor;
 }
 
-void ActionView::changeColor(const QColor &color)
-{
-    m_currentColor = color;
-    Q_EMIT currentColorChanged();
-}
-
 void ActionView::updateActionStates()
 {
     bool hasValidAction = (m_currentActionIndex != -1) && m_itemHandle.Valid();
diff --git a/src/Authoring/Studio/Palettes/Action/ActionView.h b/src/Authoring/Studio/Palettes/Action/ActionView.h
index 4fdc83a9..6b3c2807 100644
--- a/src/Authoring/Studio/Palettes/Action/ActionView.h
+++ b/src/Authoring/Studio/Palettes/Action/ActionView.h
@@ -95,7 +95,6 @@ class ActionView : public QQuickWidget,
     Q_PROPERTY(PropertyInfo property READ property NOTIFY propertyChanged FINAL)
     Q_PROPERTY(QString firedEvent MEMBER m_firedEvent NOTIFY firedEventChanged FINAL)
     Q_PROPERTY(bool propertyValueInvalid READ isPropertyValueInvalid NOTIFY propertyValueInvalidChanged FINAL)
-    Q_PROPERTY(QColor currentColor READ currentColor NOTIFY currentColorChanged FINAL)
 
 public:
     ActionView(const QSize &preferredSize, QWidget *parent = nullptr);
@@ -116,7 +115,6 @@ public:
     QVariantList handlerArguments() const;
     PropertyInfo property() const;
     bool isPropertyValueInvalid() const;
-    QColor currentColor() const { return m_currentColor; }
 
     Q_INVOKABLE void setCurrentActionIndex(int index);
     Q_INVOKABLE void setCurrentPropertyIndex(int handle, int index);
@@ -167,13 +165,12 @@ Q_SIGNALS:
     void firedEventChanged();
     void hasItemChanged();
     void propertyValueInvalidChanged();
-    void currentColorChanged();
+    void dialogCurrentColorChanged(const QColor &newColor);
 
 private Q_SLOTS:
     void copyAction();
     void cutAction();
     void pasteAction();
-    void changeColor(const QColor &color);
 
 private:
     void setTriggerObject(const qt3dsdm::SObjectRefType &object);
diff --git a/src/Authoring/Studio/Palettes/Action/HandlerGenericBaseColor.qml b/src/Authoring/Studio/Palettes/Action/HandlerGenericBaseColor.qml
index 22347afd..2d2c04f3 100644
--- a/src/Authoring/Studio/Palettes/Action/HandlerGenericBaseColor.qml
+++ b/src/Authoring/Studio/Palettes/Action/HandlerGenericBaseColor.qml
@@ -36,11 +36,21 @@ RowLayout {
 
     property alias color: rect.color
     property color selectedColor: "black"
-    property color currentColor: _parentView.currentColor
+    property bool listenToColorChanges: false
 
     signal colorSelected()
     signal previewColorSelected()
 
+    Connections {
+        target: _parentView
+        onDialogCurrentColorChanged: {
+            if (root.listenToColorChanges) {
+                root.selectedColor = newColor;
+                root.previewColorSelected();
+            }
+        }
+    }
+
     Rectangle {
         id: rect
 
@@ -57,8 +67,9 @@ RowLayout {
 
             anchors.fill: parent
             onClicked: {
-                selectedColor = rect.color;
-                selectedColor = _parentView.showColorDialog(selectedColor);
+                root.listenToColorChanges = true;
+                root.selectedColor = _parentView.showColorDialog(rect.color);
+                root.listenToColorChanges = false;
                 root.colorSelected();
             }
         }
@@ -75,9 +86,4 @@ RowLayout {
     Item {
         Layout.fillWidth: true
     }
-
-    onCurrentColorChanged: {
-        selectedColor = currentColor;
-        root.previewColorSelected();
-    }
 }
diff --git a/src/Authoring/Studio/Palettes/Action/HandlerProperty.qml b/src/Authoring/Studio/Palettes/Action/HandlerProperty.qml
index 6c903561..cf3cc79d 100644
--- a/src/Authoring/Studio/Palettes/Action/HandlerProperty.qml
+++ b/src/Authoring/Studio/Palettes/Action/HandlerProperty.qml
@@ -178,6 +178,7 @@ ColumnLayout {
             label: parent ? parent.label : ""
             color:  "black"
             onColorSelected: {
+                color = selectedColor;
                 _parentView.setArgumentValue(propertyModel.valueHandle, selectedColor);
             }
             onPreviewColorSelected: color = selectedColor
diff --git a/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.cpp b/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.cpp
index fbe799b2..5ab6cb91 100644
--- a/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.cpp
+++ b/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.cpp
@@ -533,18 +533,14 @@ QColor InspectorControlView::showColorDialog(const QColor &color)
 {
     m_currentColor = color;
     CDialogs *dialogs = g_StudioApp.GetDialogs();
-    connect(dialogs, &CDialogs::onColorChanged, this, &InspectorControlView::changeColor);
+    connect(dialogs, &CDialogs::onColorChanged,
+            this, &InspectorControlView::dialogCurrentColorChanged);
     QColor currentColor = dialogs->displayColorDialog(color);
-    disconnect(dialogs, &CDialogs::onColorChanged, this, &InspectorControlView::changeColor);
+    disconnect(dialogs, &CDialogs::onColorChanged,
+               this, &InspectorControlView::dialogCurrentColorChanged);
     return currentColor;
 }
 
-void InspectorControlView::changeColor(const QColor &color)
-{
-    m_currentColor = color;
-    Q_EMIT currentColorChanged();
-}
-
 bool InspectorControlView::toolTipsEnabled()
 {
     return CStudioPreferences::ShouldShowTooltips();
diff --git a/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.h b/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.h
index bbd0dd14..2d812c2a 100644
--- a/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.h
+++ b/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.h
@@ -58,7 +58,6 @@ class InspectorControlView : public QQuickWidget,
     Q_OBJECT
     Q_PROPERTY(QString titleText READ titleText NOTIFY titleChanged FINAL)
     Q_PROPERTY(QString titleIcon READ titleIcon NOTIFY titleChanged FINAL)
-    Q_PROPERTY(QColor currentColor READ currentColor NOTIFY currentColorChanged FINAL)
 public:
     explicit InspectorControlView(const QSize &preferredSize, QWidget *parent = nullptr);
     ~InspectorControlView() override;
@@ -69,7 +68,6 @@ public:
     QString titleText() const;
     Q_INVOKABLE QColor titleColor(int instance = 0, int handle = 0) const;
     QString titleIcon() const;
-    QColor currentColor() const { return m_currentColor; }
 
     Q_INVOKABLE void showContextMenu(int x, int y, int handle, int instance);
     Q_INVOKABLE QObject *showImageChooser(int handle, int instance, const QPoint &point);
@@ -93,11 +91,10 @@ Q_SIGNALS:
     void titleChanged();
     void controlsChanged();
     void imageSelected(const QString &name);
-    void currentColorChanged();
+    void dialogCurrentColorChanged(const QColor &newColor);
 
 public Q_SLOTS:
     void toggleMasterLink();
-    void changeColor(const QColor &color);
 
 protected:
     QSize sizeHint() const override;
diff --git a/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineWidget.cpp b/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineWidget.cpp
index 68b54599..cd74854d 100644
--- a/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineWidget.cpp
+++ b/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineWidget.cpp
@@ -1149,8 +1149,8 @@ void TimelineWidget::openBarColorDialog()
     CDialogs *dialogs = g_StudioApp.GetDialogs();
     connect(dialogs, &CDialogs::onColorChanged, this, &TimelineWidget::onTimeBarColorChanged);
     QColor selectedColor = dialogs->displayColorDialog(previousColor);
-    setSelectedTimeBarsColor(selectedColor, selectedColor == previousColor);
     disconnect(dialogs, &CDialogs::onColorChanged, this, &TimelineWidget::onTimeBarColorChanged);
+    setSelectedTimeBarsColor(selectedColor, selectedColor == previousColor);
 }
 
 void TimelineWidget::onTimeBarColorChanged(const QColor &color)
@@ -1162,18 +1162,16 @@ void TimelineWidget::onTimeBarColorChanged(const QColor &color)
 // When preview, only set the UI without property changes.
 void TimelineWidget::setSelectedTimeBarsColor(const QColor &color, bool preview)
 {
+    using namespace Q3DStudio; // Needed for SCOPED_DOCUMENT_EDITOR macro
     auto rows = selectedRows();
     for (RowTree *row : qAsConst(rows)) {
         row->rowTimeline()->setBarColor(color);
         if (!preview) {
-            // Get editable handle into document editor without undo transactions
-            CDoc *theDoc = g_StudioApp.GetCore()->GetDoc();
-            Q3DStudio::IDocumentEditor *editor =
-                    dynamic_cast<Q3DStudio::IDocumentEditor*>(&theDoc->GetDocumentReader());
-
             Qt3DSDMTimelineItemBinding *timelineItemBinding =
-                static_cast<Qt3DSDMTimelineItemBinding *>(row->getBinding());
-            editor->SetTimebarColor(timelineItemBinding->GetInstanceHandle(), color);
+                    static_cast<Qt3DSDMTimelineItemBinding *>(row->getBinding());
+            SCOPED_DOCUMENT_EDITOR(*g_StudioApp.GetCore()->GetDoc(),
+                                   QObject::tr("Set Timebar Color"))
+                ->SetTimebarColor(timelineItemBinding->GetInstanceHandle(), color);
         }
     }
 }