QmlDesigner: Generate qml id from image file name

When dragging an image file to create a Image or Texture item,
base the id on the file name rather than setting it to generic name
based on type.

This includes cleanup of AbstractView::generateNewId to remove
redundant and unused code as well as an option to provide a fallback
id prefix.

Change-Id: I563f1760ffbace9c5c2145477ec8736836f36c45
Fixes: QDS-2328
Reviewed-by: Thomas Hartmann <thomas.hartmann@qt.io>
Reviewed-by: Mahmoud Badri <mahmoud.badri@qt.io>

4 files changed, 26 insertions, 23 deletions

diff --git a/src/plugins/qmldesigner/components/navigator/navigatortreemodel.cpp b/src/plugins/qmldesigner/components/navigator/navigatortreemodel.cpp
index f86b377933..e2cc2970b8 100644
--- a/src/plugins/qmldesigner/components/navigator/navigatortreemodel.cpp
+++ b/src/plugins/qmldesigner/components/navigator/navigatortreemodel.cpp
@@ -51,6 +51,7 @@
 #include <QApplication>
 #include <QPointF>
 #include <QDir>
+#include <QFileInfo>
 
 #include <coreplugin/messagebox.h>
 
@@ -649,6 +650,10 @@ void NavigatorTreeModel::handleItemLibraryImageDrop(const QMimeData *mimeData, i
 
                 // create a texture
                 newModelNode = QmlItemNode::createQmlObjectNode(m_view, itemLibraryEntry, {}, targetProp, false);
+
+                // Rename the node based on source image
+                QFileInfo fi(imagePath);
+                newModelNode.setIdWithoutRefactoring(m_view->generateNewId(fi.baseName(), "textureImage"));
                 return newModelNode.isValid();
             }
             return false;
diff --git a/src/plugins/qmldesigner/designercore/include/abstractview.h b/src/plugins/qmldesigner/designercore/include/abstractview.h
index d0dcddb2ee..2412ed9700 100644
--- a/src/plugins/qmldesigner/designercore/include/abstractview.h
+++ b/src/plugins/qmldesigner/designercore/include/abstractview.h
@@ -160,6 +160,7 @@ public:
     ModelNode modelNodeForId(const QString &id);
     bool hasId(const QString &id) const;
     QString generateNewId(const QString &prefixName) const;
+    QString generateNewId(const QString &prefixName, const QString &fallbackPrefix) const;
 
     ModelNode modelNodeForInternalId(qint32 internalId) const;
     bool hasModelNodeForInternalId(qint32 internalId) const;
diff --git a/src/plugins/qmldesigner/designercore/model/abstractview.cpp b/src/plugins/qmldesigner/designercore/model/abstractview.cpp
index f5ef94f621..d1a7222877 100644
--- a/src/plugins/qmldesigner/designercore/model/abstractview.cpp
+++ b/src/plugins/qmldesigner/designercore/model/abstractview.cpp
@@ -504,40 +504,36 @@ QString firstCharToLower(const QString &string)
     return resultString;
 }
 
-QString AbstractView::generateNewId(const QString &prefixName) const
+QString AbstractView::generateNewId(const QString &prefixName, const QString &fallbackPrefix) const
 {
-    QString fixedPrefix = firstCharToLower(prefixName);
-    fixedPrefix.remove(' ');
-
-    bool forceSuffix = false;
-
-    if (!ModelNode::isValidId(fixedPrefix))
-        forceSuffix = true;
+    // First try just the prefixName without number as postfix, then continue with 2 and further
+    // as postfix until id does not already exist.
+    // Properties of the root node are not allowed for ids, because they are available in the
+    // complete context without qualification.
 
     int counter = 0;
 
-    /* First try just the prefixName without number as postfix, then continue with 2 and further as postfix
-     * until id does not already exist.
-     * Properties of the root node are not allowed for ids, because they are available in the complete context
-     * without qualification.
-     * The id "item" is explicitly not allowed, because it is too likely to clash.
-    */
+    QString newBaseId = QString(QStringLiteral("%1")).arg(firstCharToLower(prefixName));
+    newBaseId.remove(QRegExp(QStringLiteral("[^a-zA-Z0-9_]")));
 
-    QString newId = QString(QStringLiteral("%1")).arg(firstCharToLower(prefixName));
-    if (forceSuffix)
-        QString(QStringLiteral("%1%2")).arg(firstCharToLower(prefixName)).arg(1);
+    if (newBaseId.isEmpty())
+        newBaseId = fallbackPrefix;
 
-    newId.remove(QRegExp(QStringLiteral("[^a-zA-Z0-9_]")));
+    QString newId = newBaseId;
 
-    while (!ModelNode::isValidId(newId) || hasId(newId) || rootModelNode().hasProperty(newId.toUtf8()) || newId == "item") {
-        counter += 1;
-        newId = QString(QStringLiteral("%1%2")).arg(firstCharToLower(prefixName)).arg(counter);
-        newId.remove(QRegExp(QStringLiteral("[^a-zA-Z0-9_]")));
+    while (!ModelNode::isValidId(newId) || hasId(newId) || rootModelNode().hasProperty(newId.toUtf8())) {
+        ++counter;
+        newId = QString(QStringLiteral("%1%2")).arg(firstCharToLower(newBaseId)).arg(counter);
     }
 
     return newId;
 }
 
+QString AbstractView::generateNewId(const QString &prefixName) const
+{
+    return generateNewId(prefixName, QStringLiteral("element"));
+}
+
 ModelNode AbstractView::modelNodeForInternalId(qint32 internalId) const
 {
      return ModelNode(model()->d->nodeForInternalId(internalId), model(), this);
diff --git a/src/plugins/qmldesigner/designercore/model/qmlitemnode.cpp b/src/plugins/qmldesigner/designercore/model/qmlitemnode.cpp
index fe42f2b7e0..886e524b29 100644
--- a/src/plugins/qmldesigner/designercore/model/qmlitemnode.cpp
+++ b/src/plugins/qmldesigner/designercore/model/qmlitemnode.cpp
@@ -106,7 +106,8 @@ QmlItemNode QmlItemNode::createQmlItemNodeFromImage(AbstractView *view, const QS
             newQmlItemNode = QmlItemNode(view->createModelNode("QtQuick.Image", metaInfo.majorVersion(), metaInfo.minorVersion(), propertyPairList));
             parentproperty.reparentHere(newQmlItemNode);
 
-            newQmlItemNode.setId(view->generateNewId(QLatin1String("image")));
+            QFileInfo fi(relativeImageName);
+            newQmlItemNode.setId(view->generateNewId(fi.baseName(), "image"));
 
             newQmlItemNode.modelNode().variantProperty("fillMode").setEnumeration("Image.PreserveAspectFit");