Close active chooser dialog when it goes out of context

A chooser is considered out of context when the property the chooser
was opened for no longer has visible control for it.

This change should ensure that user can never interact with
invalid properties via the chooser dialogs.

Task-number: QT3DS-2836
Change-Id: I6fca811cfccda0b0b4fbc0feb17935c3949f4f9f
Reviewed-by: Tomi Korpipää <tomi.korpipaa@qt.io>
Reviewed-by: Mahmoud Badri <mahmoud.badri@qt.io>
Reviewed-by: Miikka Heikkinen <miikka.heikkinen@qt.io>

6 files changed, 83 insertions, 9 deletions

diff --git a/src/Authoring/Studio/Palettes/Action/ActionView.cpp b/src/Authoring/Studio/Palettes/Action/ActionView.cpp
index eac0e0e7..3fb6d1d4 100644
--- a/src/Authoring/Studio/Palettes/Action/ActionView.cpp
+++ b/src/Authoring/Studio/Palettes/Action/ActionView.cpp
@@ -186,6 +186,11 @@ bool ActionView::event(QEvent *event)
 
 void ActionView::setItem(const qt3dsdm::Qt3DSDMInstanceHandle &handle)
 {
+    if (!m_activeBrowser.isNull() && m_activeBrowser->isVisible()) {
+        m_activeBrowser->close();
+        m_activeBrowser.clear();
+    }
+
     m_objRefHelper = GetDoc()->GetDataModelObjectReferenceHelper();
     m_itemHandle = handle;
     m_actionsModel->setInstanceHandle(handle);
@@ -425,6 +430,7 @@ QObject *ActionView::showTriggerObjectBrowser(const QPoint &point)
     m_triggerObjectBrowser->disconnect();
     m_triggerObjectBrowser->selectAndExpand(instanceHandle, actionInfo.m_Owner);
     CDialogs::showWidgetBrowser(this, m_triggerObjectBrowser, point);
+    m_activeBrowser = m_triggerObjectBrowser;
 
     connect(m_triggerObjectBrowser, &ObjectBrowserView::selectionChanged,
             this, &ActionView::OnTriggerSelectionChanged);
@@ -456,6 +462,7 @@ QObject *ActionView::showTargetObjectBrowser(const QPoint &point)
     m_targetObjectBrowser->disconnect();
     m_targetObjectBrowser->selectAndExpand(instanceHandle, actionInfo.m_Owner);
     CDialogs::showWidgetBrowser(this, m_targetObjectBrowser, point);
+    m_activeBrowser = m_targetObjectBrowser;
 
     connect(m_targetObjectBrowser, &ObjectBrowserView::selectionChanged,
             this, &ActionView::OnTargetSelectionChanged);
@@ -518,6 +525,7 @@ QObject *ActionView::showEventBrowser(const QPoint &point)
     m_eventsBrowser->disconnect();
     m_eventsBrowser->selectAndExpand(QString::fromStdWString(actionInfo.m_Event));
     CDialogs::showWidgetBrowser(this, m_eventsBrowser, point);
+    m_activeBrowser = m_eventsBrowser;
 
     connect(m_eventsBrowser, &EventsBrowserView::selectionChanged,
             this, [this] {
@@ -554,6 +562,7 @@ QObject *ActionView::showHandlerBrowser(const QPoint &point)
     m_handlerBrowser->disconnect();
     m_handlerBrowser->selectAndExpand(QString::fromStdWString(actionInfo.m_Handler));
     CDialogs::showWidgetBrowser(this, m_handlerBrowser, point);
+    m_activeBrowser = m_handlerBrowser;
 
     connect(m_handlerBrowser, &EventsBrowserView::selectionChanged,
             this, [this] {
@@ -602,6 +611,7 @@ QObject *ActionView::showEventBrowserForArgument(int handle, const QPoint &point
     m_fireEventsBrowser->disconnect();
     m_fireEventsBrowser->selectAndExpand(eventName);
     CDialogs::showWidgetBrowser(this, m_fireEventsBrowser, point);
+    m_activeBrowser = m_fireEventsBrowser;
 
     connect(m_fireEventsBrowser, &EventsBrowserView::selectionChanged,
             this, [this, handle] {
@@ -749,6 +759,11 @@ void ActionView::OnActionAdded(qt3dsdm::Qt3DSDMActionHandle inAction,
     qt3dsdm::Qt3DSDMSlideHandle theMasterOfCurrentSlide =
             theStudioSystem->GetSlideSystem()->GetMasterSlide(theCurrentSlide);
 
+    if (!m_activeBrowser.isNull() && m_activeBrowser->isVisible()) {
+        m_activeBrowser->close();
+        m_activeBrowser.clear();
+    }
+
     if (inOwner == m_itemHandle  // the action is added to current viewed instance
             && (theCurrentSlide == inSlide // and is added to the current viewed slide
                 || (theMasterSlideOfAction == inSlide
@@ -765,6 +780,10 @@ void ActionView::OnActionDeleted(qt3dsdm::Qt3DSDMActionHandle inAction,
     Q_UNUSED(inSlide);
     Q_UNUSED(inOwner);
 
+    if (!m_activeBrowser.isNull() && m_activeBrowser->isVisible()) {
+        m_activeBrowser->close();
+        m_activeBrowser.clear();
+    }
     m_actionsModel->removeAction(inAction);
 }
 
@@ -784,6 +803,17 @@ void ActionView::OnHandlerArgumentModified(qt3dsdm::Qt3DSDMHandlerArgHandle inHa
     if (!m_itemHandle.Valid())
         return;
 
+    // m_fireEventsBrowser needs to be closed if another type of target handler is chosen.
+    // Other browsers will remain valid always as long as the action is selected.
+    if (!m_fireEventsBrowser.isNull() && m_activeBrowser == m_fireEventsBrowser
+            && m_activeBrowser->isVisible()) {
+        const auto actionInfo = m_actionsModel->actionInfoAt(m_currentActionIndex);
+        if (actionInfo.m_Handler != L"Fire Event") {
+            m_activeBrowser->close();
+            m_activeBrowser.clear();
+        }
+
+    }
     emitActionChanged();
 }
 
diff --git a/src/Authoring/Studio/Palettes/Action/ActionView.h b/src/Authoring/Studio/Palettes/Action/ActionView.h
index 6b3c2807..0b0345fe 100644
--- a/src/Authoring/Studio/Palettes/Action/ActionView.h
+++ b/src/Authoring/Studio/Palettes/Action/ActionView.h
@@ -224,6 +224,7 @@ private:
     QAction *m_actionPaste;
     bool m_propertyValueInvalid = true;
     QColor m_currentColor;
+    QPointer<QWidget> m_activeBrowser = nullptr;
 };
 
 #endif // ACTIONVIEW_H
diff --git a/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.cpp b/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.cpp
index 3ee2a01c..2335ba2d 100644
--- a/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.cpp
+++ b/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.cpp
@@ -201,18 +201,18 @@ void InspectorControlModel::notifyInstancePropertyValue(qt3dsdm::Qt3DSDMInstance
         Q_EMIT dataChanged(index(0), index(rowCount() - 1));
 }
 
-QVariant InspectorControlModel::getPropertyValue(long instance, int handle)
+bool InspectorControlModel::hasInstanceProperty(long instance, int handle)
 {
-    for (int row = 0; row < m_groupElements.count(); ++row) {
-        auto group = m_groupElements[row];
-        for (int p = 0; p < group.controlElements.count(); ++p) {
-            QVariant& element = group.controlElements[p];
+    for (const auto &group : qAsConst(m_groupElements)) {
+        for (const auto &element : qAsConst(group.controlElements)) {
             InspectorControlBase *property = element.value<InspectorControlBase *>();
-            if (property->m_property == qt3dsdm::CDataModelHandle(handle))
-                return property->m_value;
+            if (property->m_property == qt3dsdm::CDataModelHandle(handle)
+                    && property->m_instance == qt3dsdm::CDataModelHandle(instance)) {
+                return true;
+            }
         }
     }
-    return {};
+    return false;
 }
 
 bool InspectorControlModel::isInsideMaterialContainer() const
diff --git a/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.h b/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.h
index a3140b3e..630689ae 100644
--- a/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.h
+++ b/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.h
@@ -130,7 +130,7 @@ public:
     void refresh();
     void saveIfMaterial(qt3dsdm::Qt3DSDMInstanceHandle instance);
 
-    QVariant getPropertyValue(long instance, int handle);
+    bool hasInstanceProperty(long instance, int handle);
 
     qt3dsdm::SValue currentPropertyValue(long instance, int handle) const;
     QString currentControllerValue(long instance, int handle) const;
diff --git a/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.cpp b/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.cpp
index 10d79c02..e427c2de 100644
--- a/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.cpp
+++ b/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.cpp
@@ -362,6 +362,7 @@ void InspectorControlView::updateInspectable(CInspectableBase *inInspectable)
 void InspectorControlView::setInspectable(CInspectableBase *inInspectable)
 {
     if (m_inspectableBase != inInspectable) {
+        m_activeBrowser.clear();
         m_inspectableBase = inInspectable;
         m_inspectorControlModel->setInspectable(inInspectable);
 
@@ -469,6 +470,7 @@ QObject *InspectorControlView::showImageChooser(int handle, int instance, const
     m_imageChooserView->setInstance(instance);
 
     CDialogs::showWidgetBrowser(this, m_imageChooserView, point);
+    m_activeBrowser.setData(m_imageChooserView, handle, instance);
 
     return m_imageChooserView;
 }
@@ -487,6 +489,7 @@ QObject *InspectorControlView::showFilesChooser(int handle, int instance, const
     m_fileChooserView->setInstance(instance);
 
     CDialogs::showWidgetBrowser(this, m_fileChooserView, point);
+    m_activeBrowser.setData(m_fileChooserView, handle, instance);
 
     return m_fileChooserView;
 }
@@ -510,6 +513,7 @@ QObject *InspectorControlView::showMeshChooser(int handle, int instance, const Q
     m_meshChooserView->setInstance(instance);
 
     CDialogs::showWidgetBrowser(this, m_meshChooserView, point);
+    m_activeBrowser.setData(m_meshChooserView, handle, instance);
 
     return m_meshChooserView;
 }
@@ -529,6 +533,7 @@ QObject *InspectorControlView::showTextureChooser(int handle, int instance, cons
     m_textureChooserView->setInstance(instance);
 
     CDialogs::showWidgetBrowser(this, m_textureChooserView, point);
+    m_activeBrowser.setData(m_textureChooserView, handle, instance);
 
     return m_textureChooserView;
 }
@@ -570,6 +575,7 @@ QObject *InspectorControlView::showObjectReference(int handle, int instance, con
     }
 
     CDialogs::showWidgetBrowser(this, m_objectReferenceView, point);
+    m_activeBrowser.setData(m_objectReferenceView, handle, instance);
 
     connect(m_objectReferenceView, &ObjectBrowserView::selectionChanged,
             this, [this, doc, handle, instance] {
@@ -606,6 +612,7 @@ QObject *InspectorControlView::showMaterialReference(int handle, int instance, c
     CDialogs::showWidgetBrowser(this, m_matRefListWidget, point,
                                 CDialogs::WidgetBrowserAlign::ComboBox,
                                 QSize(CStudioPreferences::valueWidth(), popupHeight));
+    m_activeBrowser.setData(m_matRefListWidget, handle, instance);
 
     connect(m_matRefListWidget, &QListWidget::itemClicked, this,
             [doc, propertySystem, instance, handle](QListWidgetItem *item) {
@@ -660,6 +667,7 @@ void InspectorControlView::showDataInputChooser(int handle, int instance, const
                     handle, instance);
     CDialogs::showWidgetBrowser(this, m_dataInputChooserView, point,
                                 CDialogs::WidgetBrowserAlign::ToolButton);
+    m_activeBrowser.setData(m_dataInputChooserView, handle, instance);
 }
 
 QColor InspectorControlView::showColorDialog(const QColor &color)
@@ -702,6 +710,12 @@ void InspectorControlView::OnEndDataModelNotifications()
     if (inspectable && !inspectable->IsValid())
         OnSelectionSet(Q3DStudio::SSelectedValue());
     m_inspectorControlModel->refresh();
+
+    // Check if the instance/handle pair still has an active UI control. If not, close browser.
+    if (m_activeBrowser.isActive() && !m_inspectorControlModel->hasInstanceProperty(
+                m_activeBrowser.m_instance, m_activeBrowser.m_handle)) {
+        m_activeBrowser.clear();
+    }
 }
 
 void InspectorControlView::OnImmediateRefreshInstanceSingle(qt3dsdm::Qt3DSDMInstanceHandle inInstance)
diff --git a/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.h b/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.h
index a5661d74..400a26b4 100644
--- a/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.h
+++ b/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.h
@@ -144,6 +144,35 @@ private:
 
     QSize m_preferredSize;
     QColor m_currentColor;
+
+    class ActiveBrowserData
+    {
+    public:
+        void setData(QWidget *browser, int handle, int instance)
+        {
+            m_activeBrowser = browser;
+            m_handle = handle;
+            m_instance = instance;
+        }
+        void clear()
+        {
+            if (isActive())
+                m_activeBrowser->close();
+            m_activeBrowser.clear();
+            m_handle = -1;
+            m_instance = -1;
+        }
+        bool isActive() const
+        {
+            return !m_activeBrowser.isNull() && m_activeBrowser->isVisible();
+        }
+
+        QPointer<QWidget> m_activeBrowser = nullptr;
+        int m_handle = -1;
+        int m_instance = -1;
+    };
+
+    ActiveBrowserData m_activeBrowser;
 };
 
 #endif // INSPECTORCONTROLVIEW_H