QmlDesigner: Fix expressions in DynamicPropertyRow and states

* States are now taken into account.
* If name is empty in expressionChanged(), then this
  is just a notifier for the QML frontend and has to be ignored.
* When exposing the expression to QML we also have to take into
  account states.
* Implementing resetting dynamic properties. We have to hardcode
  the default values for the base state
* I also implemented the callbacks in the views for PropertyChanges.

Change-Id: I08247a3c1783a52853db7bf0f7f249520f7edc1c
Reviewed-by: Miikka Heikkinen <miikka.heikkinen@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>

src/plugins/qmldesigner/components/connectioneditor/connectionview.cpp

@@ -111,6 +111,8 @@ void ConnectionView::propertiesRemoved(const QList<AbstractProperty> &propertyLi
     for (const AbstractProperty &property : propertyList) {
         if (property.isDefaultProperty())
             connectionModel()->resetModel();
+
+        dynamicPropertiesModel()->dispatchPropertyChanges(property);
     }
 }
 
@@ -138,8 +140,9 @@ void ConnectionView::variantPropertiesChanged(const QList<VariantProperty> &prop
             backendModel()->resetModel();
 
         connectionModel()->variantPropertyChanged(variantProperty);
-    }
 
+        dynamicPropertiesModel()->dispatchPropertyChanges(variantProperty);
+    }
 }
 
 void ConnectionView::bindingPropertiesChanged(const QList<BindingProperty> &propertyList,
@@ -153,6 +156,8 @@ void ConnectionView::bindingPropertiesChanged(const QList<BindingProperty> &prop
             backendModel()->resetModel();
 
         connectionModel()->bindingPropertyChanged(bindingProperty);
+
+        dynamicPropertiesModel()->dispatchPropertyChanges(bindingProperty);
     }
 }
 

src/plugins/qmldesigner/components/connectioneditor/dynamicpropertiesmodel.cpp

@@ -139,6 +139,44 @@ bool DynamicPropertiesModel::isValueType(const TypeName &type)
     return valueTypes.contains(type);
 }
 
+QVariant DynamicPropertiesModel::defaultValueForType(const TypeName &type)
+{
+    QVariant value;
+    if (type == "int")
+        value = 0;
+    else if (type == "real")
+        value = 0.0;
+    else if (type == "color")
+        value = QColor(255, 255, 255);
+    else if (type == "string")
+        value = "This is a string";
+    else if (type == "bool")
+        value = false;
+    else if (type == "url")
+        value = "";
+    else if (type == "variant")
+        value = "";
+
+    return value;
+}
+
+QString DynamicPropertiesModel::defaultExpressionForType(const TypeName &type)
+{
+    QString expression;
+    if (type == "alias")
+        expression = "null";
+    else if (type == "TextureInput")
+        expression = "null";
+    else if (type == "vector2d")
+        expression = "Qt.vector2d(0, 0)";
+    else if (type == "vector3d")
+        expression = "Qt.vector3d(0, 0, 0)";
+    else if (type == "vector4d")
+        expression = "Qt.vector4d(0, 0, 0 ,0)";
+
+    return expression;
+}
+
 DynamicPropertiesModel::DynamicPropertiesModel(bool explicitSelection, AbstractView *parent)
     : QStandardItemModel(parent)
     , m_view(parent)
@@ -239,6 +277,20 @@ void DynamicPropertiesModel::resetProperty(const PropertyName &name)
     }
 }
 
+void DynamicPropertiesModel::dispatchPropertyChanges(const AbstractProperty &abstractProperty)
+{
+    if (abstractProperty.parentModelNode().simplifiedTypeName() == "PropertyChanges") {
+        QmlPropertyChanges changes(abstractProperty.parentModelNode());
+        if (changes.target().isValid()) {
+            const ModelNode target = changes.target();
+            const PropertyName propertyName = abstractProperty.name();
+            const AbstractProperty targetProperty = target.variantProperty(propertyName);
+            if (target.hasProperty(propertyName) && targetProperty.isDynamic())
+                abstractPropertyChanged(targetProperty);
+        }
+    }
+}
+
 void DynamicPropertiesModel::bindingPropertyChanged(const BindingProperty &bindingProperty)
 {
     if (!bindingProperty.isDynamic())
@@ -262,6 +314,27 @@ void DynamicPropertiesModel::bindingPropertyChanged(const BindingProperty &bindi
     m_handleDataChanged = true;
 }
 
+void DynamicPropertiesModel::abstractPropertyChanged(const AbstractProperty &property)
+{
+    if (!property.isDynamic())
+        return;
+
+    m_handleDataChanged = false;
+
+    const QList<ModelNode> nodes = selectedNodes();
+    if (!nodes.contains(property.parentModelNode()))
+        return;
+    int rowNumber = findRowForProperty(property);
+    if (rowNumber > -1) {
+        if (property.isVariantProperty())
+            updateVariantProperty(rowNumber);
+        else
+            updateBindingProperty(rowNumber);
+    }
+
+    m_handleDataChanged = true;
+}
+
 void DynamicPropertiesModel::variantPropertyChanged(const VariantProperty &variantProperty)
 {
     if (!variantProperty.isDynamic())
@@ -334,12 +407,13 @@ void DynamicPropertiesModel::setSelectedNode(const ModelNode &node)
 AbstractProperty DynamicPropertiesModel::abstractPropertyForRow(int rowNumber) const
 {
     const int internalId = data(index(rowNumber, TargetModelNodeRow), Qt::UserRole + 1).toInt();
-    const QString targetPropertyName = data(index(rowNumber, TargetModelNodeRow), Qt::UserRole + 2).toString();
+    const QString targetPropertyName = data(index(rowNumber, TargetModelNodeRow), Qt::UserRole + 2)
+                                           .toString();
 
     if (!m_view->isAttached())
         return AbstractProperty();
 
-    ModelNode  modelNode = m_view->modelNodeForInternalId(internalId);
+    ModelNode modelNode = m_view->modelNodeForInternalId(internalId);
 
     if (modelNode.isValid())
         return modelNode.property(targetPropertyName.toUtf8());
@@ -545,6 +619,12 @@ void DynamicPropertiesModel::updateBindingProperty(int rowNumber)
         QString type = QString::fromUtf8(bindingProperty.dynamicTypeName());
         updateDisplayRole(rowNumber, PropertyTypeRow, type);
         updateDisplayRole(rowNumber, PropertyValueRow, value);
+
+        const QmlObjectNode objectNode = QmlObjectNode(bindingProperty.parentModelNode());
+        if (objectNode.isValid() && !objectNode.view()->currentState().isBaseState())
+            value = objectNode.expression(bindingProperty.name());
+
+        updateDisplayRole(rowNumber, PropertyValueRow, value);
     }
 }
 
@@ -558,6 +638,10 @@ void DynamicPropertiesModel::updateVariantProperty(int rowNumber)
         QVariant value = variantProperty.value();
         QString type = QString::fromUtf8(variantProperty.dynamicTypeName());
         updateDisplayRole(rowNumber, PropertyTypeRow, type);
+        const QmlObjectNode objectNode = QmlObjectNode(variantProperty.parentModelNode());
+        if (objectNode.isValid() && !objectNode.view()->currentState().isBaseState())
+            value = objectNode.modelValue(variantProperty.name());
+
 
         updateDisplayRoleFromVariant(rowNumber, PropertyValueRow, value);
     }
@@ -761,6 +845,16 @@ int DynamicPropertiesModel::findRowForVariantProperty(const VariantProperty &var
     return -1;
 }
 
+int DynamicPropertiesModel::findRowForProperty(const AbstractProperty &abstractProperty) const
+{
+    for (int i = 0; i < rowCount(); i++) {
+        if ((abstractPropertyForRow(i).name() == abstractProperty.name()))
+            return i;
+    }
+    //not found
+    return -1;
+}
+
 bool DynamicPropertiesModel::getExpressionStrings(const BindingProperty &bindingProperty, QString *sourceNode, QString *sourceProperty)
 {
     //### todo we assume no expressions yet

src/plugins/qmldesigner/components/connectioneditor/dynamicpropertiesmodel.h

@@ -50,6 +50,7 @@ public:
     };
     DynamicPropertiesModel(bool explicitSelection, AbstractView *parent);
     void bindingPropertyChanged(const BindingProperty &bindingProperty);
+    void abstractPropertyChanged(const AbstractProperty &bindingProperty);
     void variantPropertyChanged(const VariantProperty &variantProperty);
     void bindingRemoved(const BindingProperty &bindingProperty);
     void variantRemoved(const VariantProperty &variantProperty);
@@ -73,9 +74,13 @@ public:
     BindingProperty replaceVariantWithBinding(const PropertyName &name, bool copyValue = false);
     void resetProperty(const PropertyName &name);
 
+    void dispatchPropertyChanges(const AbstractProperty &abstractProperty);
+
     QmlDesigner::PropertyName unusedProperty(const QmlDesigner::ModelNode &modelNode);
 
     static bool isValueType(const TypeName &type);
+    static QVariant defaultValueForType(const TypeName &type);
+    static QString defaultExpressionForType(const TypeName &type);
 
 protected:
     void addProperty(const QVariant &propertyValue,
@@ -94,6 +99,7 @@ protected:
     void updateCustomData(int row, const AbstractProperty &property);
     int findRowForBindingProperty(const BindingProperty &bindingProperty) const;
     int findRowForVariantProperty(const VariantProperty &variantProperty) const;
+    int findRowForProperty(const AbstractProperty &abstractProperty) const;
 
     bool getExpressionStrings(const BindingProperty &bindingProperty, QString *sourceNode, QString *sourceProperty);
 

src/plugins/qmldesigner/components/materialeditor/materialeditorview.cpp

@@ -804,6 +804,8 @@ void MaterialEditorView::propertiesRemoved(const QList<AbstractProperty> &proper
             setValue(m_selectedMaterial, property.name(), QmlObjectNode(m_selectedMaterial).instanceValue(property.name()));
             changed = true;
         }
+
+        dynamicPropertiesModel()->dispatchPropertyChanges(property);
     }
     if (changed)
         requestPreviewRender();
@@ -827,6 +829,8 @@ void MaterialEditorView::variantPropertiesChanged(const QList<VariantProperty> &
 
             changed = true;
         }
+
+        dynamicPropertiesModel()->dispatchPropertyChanges(property);
     }
     if (changed)
         requestPreviewRender();
@@ -854,6 +858,8 @@ void MaterialEditorView::bindingPropertiesChanged(const QList<BindingProperty> &
 
             changed = true;
         }
+
+        dynamicPropertiesModel()->dispatchPropertyChanges(property);
     }
     if (changed)
         requestPreviewRender();

src/plugins/qmldesigner/components/propertyeditor/dynamicpropertiesproxymodel.cpp

@@ -106,8 +106,10 @@ QVariant DynamicPropertiesProxyModel::data(const QModelIndex &index, int role) c
             QmlObjectNode objectNode = property.parentQmlObjectNode();
             return objectNode.modelValue(property.name());
         } else if (role == propertyBindingRole) {
-            if (property.isBindingProperty())
+            if (property.isBindingProperty()) {
-                return property.toBindingProperty().expression();
+                QmlObjectNode objectNode = property.parentQmlObjectNode();
+                return objectNode.expression(property.name());
+            }
             return QVariant();
         }
         qWarning() << Q_FUNC_INFO << "invalid role";
@@ -142,45 +144,23 @@ void DynamicPropertiesProxyModel::createProperty(const QString &name, const QStr
 {
     QmlDesignerPlugin::emitUsageStatistics(Constants::EVENT_PROPERTY_ADDED);
 
+    TypeName typeName = type.toUtf8();
+
     const auto selectedNodes = dynamicPropertiesModel()->selectedNodes();
     if (selectedNodes.count() == 1) {
         const ModelNode modelNode = selectedNodes.constFirst();
         if (modelNode.isValid()) {
             try {
-                if (Internal::DynamicPropertiesModel::isValueType(type.toUtf8())) {
+                if (Internal::DynamicPropertiesModel::isValueType(typeName)) {
-                    QVariant value;
+                    QVariant value = Internal::DynamicPropertiesModel::defaultValueForType(typeName);
-                    if (type == "int")
-                        value = 0;
-                    else if (type == "real")
-                        value = 0.0;
-                    else if (type == "color")
-                        value = QColor(255, 255, 255);
-                    else if (type == "string")
-                        value = "";
-                    else if (type == "bool")
-                        value = false;
-                    else if (type == "url")
-                        value = "";
-                    else if (type == "variant")
-                        value = "";
-
                     modelNode.variantProperty(name.toUtf8())
-                        .setDynamicTypeNameAndValue(type.toUtf8(), value);
+                        .setDynamicTypeNameAndValue(typeName, value);
                 } else {
-                    QString expression;
+                    QString expression = Internal::DynamicPropertiesModel::defaultExpressionForType(
-                    if (type == "alias")
+                        typeName);
-                        expression = "null";
-                    else if (type == "TextureInput")
-                        expression = "null";
-                    else if (type == "vector2d")
-                        expression = "Qt.vector2d(0, 0)";
-                    else if (type == "vector3d")
-                        expression = "Qt.vector3d(0, 0, 0)";
-                    else if (type == "vector4d")
-                        expression = "Qt.vector4d(0, 0, 0 ,0)";
 
                     modelNode.bindingProperty(name.toUtf8())
-                        .setDynamicTypeNameAndExpression(type.toUtf8(), expression);
+                        .setDynamicTypeNameAndExpression(typeName, expression);
                 }
             } catch (Exception &e) {
                 e.showException();
@@ -203,7 +183,12 @@ DynamicPropertyRow::DynamicPropertyRow(QObject *parent)
     QObject::connect(m_backendValue,
                      &PropertyEditorValue::expressionChanged,
                      this,
-                     [this](const QString &) { commitExpression(m_backendValue->expression()); });
+                     [this](const QString &name) {
+                         if (!name.isEmpty()) //If name is empty the notifer is only for QML
+                             commitExpression(m_backendValue->expression());
+                         else if (m_backendValue->expression().isEmpty())
+                             resetValue();
+                     });
 }
 
 DynamicPropertyRow::~DynamicPropertyRow()
@@ -301,13 +286,16 @@ void DynamicPropertyRow::setupBackendValue()
         m_backendValue->setModelNode(node);
 
     QVariant modelValue = property.parentQmlObjectNode().modelValue(property.name());
+
+    const bool isBound = property.parentQmlObjectNode().hasBindingProperty(property.name());
+
     if (modelValue != m_backendValue->value()) {
         m_backendValue->setValue({});
         m_backendValue->setValue(modelValue);
     }
 
-    if (property.isBindingProperty()) {
+    if (isBound) {
-        QString expression = property.toBindingProperty().expression();
+        QString expression = property.parentQmlObjectNode().expression(property.name());
         if (m_backendValue->expression() != expression)
             m_backendValue->setExpression(expression);
     }
@@ -323,6 +311,9 @@ void DynamicPropertyRow::commitValue(const QVariant &value)
     if (m_row < 0)
         return;
 
+    if (!value.isValid())
+        return;
+
     auto propertiesModel = m_model->dynamicPropertiesModel();
     VariantProperty variantProperty = propertiesModel->variantPropertyForRow(m_row);
 
@@ -336,11 +327,11 @@ void DynamicPropertyRow::commitValue(const QVariant &value)
     RewriterTransaction transaction = view->beginRewriterTransaction(
         QByteArrayLiteral("DynamicPropertiesModel::commitValue"));
     try {
-        if (view->currentState().isBaseState()) {
+        QmlObjectNode objectNode = variantProperty.parentQmlObjectNode();
+        if (view->currentState().isBaseState() && !objectNode.timelineIsActive()) {
             if (variantProperty.value() != value)
                 variantProperty.setDynamicTypeNameAndValue(variantProperty.dynamicTypeName(), value);
         } else {
-            QmlObjectNode objectNode = variantProperty.parentQmlObjectNode();
             QTC_CHECK(objectNode.isValid());
             PropertyName name = variantProperty.name();
             if (objectNode.isValid() && objectNode.modelValue(name) != value)
@@ -360,24 +351,43 @@ void DynamicPropertyRow::commitExpression(const QString &expression)
     if (m_row < 0)
         return;
 
+    auto propertiesModel = m_model->dynamicPropertiesModel();
+    AbstractProperty property = propertiesModel->abstractPropertyForRow(m_row);
+
+    BindingProperty bindingProperty = property.parentModelNode().bindingProperty(property.name());
+
+    const QVariant literal = BindingProperty::convertToLiteral(bindingProperty.dynamicTypeName(),
+                                                               expression);
+
+    if (literal.isValid()) { //If the string can be converted to a literal we set it as a literal/value
+        commitValue(literal);
+        return;
+    }
+
     m_lock = true;
     auto unlock = qScopeGuard([this] { m_lock = false; });
 
-    auto propertiesModel = m_model->dynamicPropertiesModel();
-    BindingProperty bindingProperty = propertiesModel->bindingPropertyForRow(m_row);
-
     auto view = propertiesModel->view();
     RewriterTransaction transaction = view->beginRewriterTransaction(
-        QByteArrayLiteral("DynamicPropertiesModel::commitExpression"));
+        QByteArrayLiteral("DynamicPropertyRow::commitExpression"));
     try {
         QString theExpression = expression;
         if (theExpression.isEmpty())
             theExpression = "null";
 
-        if (bindingProperty.expression() != theExpression) {
+        if (view->currentState().isBaseState()) {
-            bindingProperty.setDynamicTypeNameAndExpression(bindingProperty.dynamicTypeName(),
+            if (bindingProperty.expression() != theExpression) {
-                                                            theExpression);
+                bindingProperty.setDynamicTypeNameAndExpression(bindingProperty.dynamicTypeName(),
+                                                                theExpression);
+            }
+        } else {
+            QmlObjectNode objectNode = bindingProperty.parentQmlObjectNode();
+            QTC_CHECK(objectNode.isValid());
+            PropertyName name = bindingProperty.name();
+            if (objectNode.isValid() && objectNode.expression(name) != theExpression)
+                objectNode.setBindingProperty(name, theExpression);
         }
+
         transaction.commit(); //committing in the try block
     } catch (Exception &e) {
         e.showException();
@@ -390,3 +400,46 @@ void DynamicPropertyRow::handleDataChanged(const QModelIndex &topLeft, const QMo
     if (topLeft.row() == m_row)
         setupBackendValue();
 }
+
+void DynamicPropertyRow::resetValue()
+{
+    if (m_lock)
+        return;
+
+    if (m_row < 0)
+        return;
+
+    auto propertiesModel = m_model->dynamicPropertiesModel();
+    auto view = propertiesModel->view();
+
+    AbstractProperty property = propertiesModel->abstractPropertyForRow(m_row);
+    TypeName typeName = property.dynamicTypeName();
+
+    if (view->currentState().isBaseState()) {
+        if (Internal::DynamicPropertiesModel::isValueType(typeName)) {
+            QVariant value = Internal::DynamicPropertiesModel::defaultValueForType(typeName);
+            commitValue(value);
+        } else {
+            QString expression = Internal::DynamicPropertiesModel::defaultExpressionForType(
+                typeName);
+            commitExpression(expression);
+        }
+    } else {
+        m_lock = true;
+        auto unlock = qScopeGuard([this] { m_lock = false; });
+
+        RewriterTransaction transaction = view->beginRewriterTransaction(
+            QByteArrayLiteral("DynamicPropertyRow::resetValue"));
+        try {
+            QmlObjectNode objectNode = property.parentQmlObjectNode();
+            QTC_CHECK(objectNode.isValid());
+            PropertyName name = property.name();
+            if (objectNode.isValid() && objectNode.propertyAffectedByCurrentState(name))
+                objectNode.removeProperty(name);
+
+            transaction.commit(); //committing in the try block
+        } catch (Exception &e) {
+            e.showException();
+        }
+    }
+}

src/plugins/qmldesigner/components/propertyeditor/dynamicpropertiesproxymodel.h

@@ -100,6 +100,7 @@ private:
     void commitValue(const QVariant &value);
     void commitExpression(const QString &expression);
     void handleDataChanged(const QModelIndex &topLeft, const QModelIndex &, const QList<int> &);
+    void resetValue();
 
     int m_row = -1;
     PropertyEditorValue *m_backendValue = nullptr;

src/plugins/qmldesigner/components/propertyeditor/propertyeditorvalue.cpp

@@ -193,7 +193,7 @@ void PropertyEditorValue::setExpressionWithEmit(const QString &expression)
     if ( m_expression != expression) {
         setExpression(expression);
         m_value.clear();
-        emit expressionChanged(nameAsQString());
+        emit expressionChanged(nameAsQString()); //Note that we set the name in this case
     }
 }
 

src/plugins/qmldesigner/components/propertyeditor/propertyeditorvalue.h

@@ -157,7 +157,11 @@ signals:
     void valueChanged(const QString &name, const QVariant&);
     void valueChangedQml();
 
-    void expressionChanged(const QString &name);
+    void expressionChanged(const QString &name); //HACK - We use the same notifer
+                                                 //for the backend and frontend.
+                                                 //If name is empty the signal is
+                                                 //used for QML.
+
     void exportPropertyAsAliasRequested(const QString &name);
     void removeAliasExportRequested(const QString &name);