Allow setting a keyframe's time in the Edit keyframe dialog

- rename keyframe context menu item "Edit Value for keyframe..." to
"Edit Keyframe..."
- add a field to set the frame, this allows accurately setting a
keyframe's frame.
- allow showing the dialog by double clicking a keyframe.
- several relavant tweaks.

Task-number: QDS-1072
Change-Id: I3c6a490c335911b2f6fefd3e788468686e827969
Reviewed-by: Thomas Hartmann <thomas.hartmann@qt.io>

src/plugins/qmldesigner/components/timelineeditor/setframevaluedialog.cpp

@@ -26,14 +26,24 @@
 #include "setframevaluedialog.h"
 #include "ui_setframevaluedialog.h"
 
+#include <QIntValidator>
+
 namespace QmlDesigner {
 
-SetFrameValueDialog::SetFrameValueDialog(QWidget *parent)
+SetFrameValueDialog::SetFrameValueDialog(qreal frame, const QVariant &value,
+                                         const QString &propertyName, QWidget *parent)
     : QDialog(parent)
     , ui(new Ui::SetFrameValueDialog)
 {
     setWindowFlags(windowFlags() & ~Qt::WindowContextHelpButtonHint);
+    setWindowTitle(tr("Edit Keyframe"));
     ui->setupUi(this);
+
+    ui->lineEditFrame->setValidator(new QIntValidator(0, 99999, this));
+
+    ui->lineEditFrame->setText(QString::number(frame));
+    ui->lineEditValue->setText(value.toString());
+    ui->labelValue->setText(propertyName);
 }
 
 SetFrameValueDialog::~SetFrameValueDialog()
@@ -41,15 +51,14 @@ SetFrameValueDialog::~SetFrameValueDialog()
     delete ui;
 }
 
-QLineEdit *SetFrameValueDialog::lineEdit() const
+qreal SetFrameValueDialog::frame() const
 {
-    return ui->lineEdit;
+    return ui->lineEditFrame->text().toDouble();
 }
 
-void SetFrameValueDialog::setPropertName(const QString &name)
+QVariant SetFrameValueDialog::value() const
 {
-    setWindowTitle(tr("Change %1").arg(name));
+    return QVariant(ui->lineEditValue->text());
-    ui->label->setText(name);
 }
 
 } // namespace QmlDesigner

src/plugins/qmldesigner/components/timelineeditor/setframevaluedialog.h

@@ -40,12 +40,12 @@ class SetFrameValueDialog : public QDialog
     Q_OBJECT
 
 public:
-    explicit SetFrameValueDialog(QWidget *parent = nullptr);
+    explicit SetFrameValueDialog(qreal frame, const QVariant &value, const QString &propertyName,
+                                 QWidget *parent = nullptr);
     ~SetFrameValueDialog() override;
 
-    QLineEdit *lineEdit() const;
+    qreal frame() const;
-
+    QVariant value() const;
-    void setPropertName(const QString &name);
 
 private:
     Ui::SetFrameValueDialog *ui;

src/plugins/qmldesigner/components/timelineeditor/setframevaluedialog.ui

@@ -7,7 +7,7 @@
     <x>0</x>
     <y>0</y>
     <width>184</width>
-    <height>79</height>
+    <height>93</height>
    </rect>
   </property>
   <property name="windowTitle">
@@ -15,16 +15,13 @@
   </property>
   <layout class="QGridLayout" name="gridLayout">
    <item row="0" column="0">
-    <widget class="QLabel" name="label">
+    <widget class="QLabel" name="labelFrame">
      <property name="text">
-      <string>Value</string>
+      <string>Frame</string>
      </property>
     </widget>
    </item>
-   <item row="0" column="1">
+   <item row="2" column="0" colspan="2">
-    <widget class="QLineEdit" name="lineEdit"/>
-   </item>
-   <item row="1" column="0" colspan="2">
     <widget class="QDialogButtonBox" name="buttonBox">
      <property name="orientation">
       <enum>Qt::Horizontal</enum>
@@ -34,6 +31,19 @@
      </property>
     </widget>
    </item>
+   <item row="0" column="1">
+    <widget class="QLineEdit" name="lineEditFrame"/>
+   </item>
+   <item row="1" column="1">
+    <widget class="QLineEdit" name="lineEditValue"/>
+   </item>
+   <item row="1" column="0">
+    <widget class="QLabel" name="labelValue">
+     <property name="text">
+      <string>Value</string>
+     </property>
+    </widget>
+   </item>
   </layout>
  </widget>
  <resources/>

src/plugins/qmldesigner/components/timelineeditor/timelinemovableabstractitem.cpp

@@ -46,6 +46,11 @@ void TimelineMovableAbstractItem::itemMoved(const QPointF & /*start*/, const QPo
     setPositionInteractive(end);
 }
 
+void TimelineMovableAbstractItem::itemDoubleClicked()
+{
+    // to be overridden by child classes if needed
+}
+
 int TimelineMovableAbstractItem::scrollOffset() const
 {
     return timelineScene()->scrollOffset();

src/plugins/qmldesigner/components/timelineeditor/timelinemovableabstractitem.h

@@ -57,6 +57,7 @@ public:
     virtual void setPositionInteractive(const QPointF &point);
     virtual void commitPosition(const QPointF &point);
     virtual void itemMoved(const QPointF &start, const QPointF &end);
+    virtual void itemDoubleClicked();
 
     int xPosScrollOffset(int x) const;
 

src/plugins/qmldesigner/components/timelineeditor/timelinepropertyitem.cpp

@@ -106,31 +106,40 @@ static void setEasingCurve(TimelineGraphicsScene *scene, const QList<ModelNode>
     EasingCurveDialog::runDialog(keys);
 }
 
-static void editValue(const ModelNode &frame, const QString &propertyName)
+// display and handle the edit keyframe dialog
+static void editValue(const ModelNode &frameNode, const std::pair<qreal, qreal> &timelineRange,
+                      const QString &propertyName)
 {
-    const QVariant value = frame.variantProperty("value").value();
+    const qreal frame = frameNode.variantProperty("frame").value().toReal();
-    auto dialog = new SetFrameValueDialog(Core::ICore::dialogParent());
+    const QVariant value = frameNode.variantProperty("value").value();
-
+    auto dialog = new SetFrameValueDialog(frame, value, propertyName,
-    dialog->lineEdit()->setText(value.toString());
+                                          Core::ICore::dialogParent());
-    dialog->setPropertName(propertyName);
 
     QObject::connect(dialog, &SetFrameValueDialog::rejected, [dialog]() { dialog->deleteLater(); });
 
-    QObject::connect(dialog, &SetFrameValueDialog::accepted, [dialog, frame, value]() {
+    QObject::connect(dialog, &SetFrameValueDialog::accepted, [dialog, frameNode, frame, value,
+                                                              timelineRange]() {
         dialog->deleteLater();
-        int userType = value.userType();
-        const QVariant result = dialog->lineEdit()->text();
 
-        if (result.canConvert(userType)) {
+        qreal newFrame = qBound(timelineRange.first, dialog->frame(), timelineRange.second);
-            QVariant newValue = result;
+        if (newFrame != frame)
-            newValue.convert(userType);
+            frameNode.variantProperty("frame").setValue(newFrame);
-            // canConvert gives true in case if the result is a double but the usertype was interger
+
-            // try to fix that with a workaround to convert it to double if convertion resulted in isNull
+        int userType = value.userType();
-            if (newValue.isNull()) {
+        QVariant newValue = dialog->value();
-                newValue = result;
+
-                newValue.convert(QMetaType::Double);
+        if (newValue.canConvert(userType)) {
+            QVariant newValueConverted = newValue;
+            bool converted = newValueConverted.convert(userType);
+
+            if (!converted) {
+                // convert() fails for int to double, so we try this combination
+                newValueConverted = newValue;
+                converted = newValueConverted.convert(QMetaType::Double);
             }
-            frame.variantProperty("value").setValue(result);
+
+            if (converted)
+                frameNode.variantProperty("value").setValue(newValueConverted);
         }
     });
 
@@ -431,9 +440,12 @@ void TimelinePropertyItem::contextMenuEvent(QGraphicsSceneContextMenuEvent *even
             setEasingCurve(timelineScene(), {currentFrameNode});
         });
 
-        QAction *editValueAction = mainMenu.addAction(tr("Edit Value for Keyframe..."));
+        QAction *editValueAction = mainMenu.addAction(tr("Edit Keyframe..."));
         QObject::connect(editValueAction, &QAction::triggered, [this, currentFrameNode]() {
-            editValue(currentFrameNode, propertyName());
+            std::pair<qreal, qreal> timelineRange
+                    = {timelineScene()->currentTimeline().startKeyframe(),
+                       timelineScene()->currentTimeline().endKeyframe()};
+            editValue(currentFrameNode, timelineRange, propertyName());
         });
 
         const bool hasKeyframe = currentFrameNode.isValid();
@@ -541,6 +553,13 @@ void TimelineKeyframeItem::commitPosition(const QPointF &point)
     enableUpdates();
 }
 
+void TimelineKeyframeItem::itemDoubleClicked()
+{
+    std::pair<qreal, qreal> timelineRange = {timelineScene()->currentTimeline().startKeyframe(),
+                                             timelineScene()->currentTimeline().endKeyframe()};
+    editValue(m_frame, timelineRange, propertyItem()->propertyName());
+}
+
 TimelineKeyframeItem *TimelineKeyframeItem::asTimelineKeyframeItem()
 {
     return this;
@@ -630,9 +649,11 @@ void TimelineKeyframeItem::contextMenuEvent(QGraphicsSceneContextMenuEvent *even
         setEasingCurve(timelineScene(), keys);
     });
 
-    QAction *editValueAction = mainMenu.addAction(tr("Edit Value for Keyframe..."));
+    QAction *editValueAction = mainMenu.addAction(tr("Edit Keyframe..."));
     QObject::connect(editValueAction, &QAction::triggered, [this]() {
-        editValue(m_frame, propertyItem()->propertyName());
+        std::pair<qreal, qreal> timelineRange = {timelineScene()->currentTimeline().startKeyframe(),
+                                                 timelineScene()->currentTimeline().endKeyframe()};
+        editValue(m_frame, timelineRange, propertyItem()->propertyName());
     });
 
     mainMenu.exec(event->screenPos());

src/plugins/qmldesigner/components/timelineeditor/timelinepropertyitem.h

@@ -63,6 +63,8 @@ public:
 
     void commitPosition(const QPointF &point) override;
 
+    void itemDoubleClicked() override;
+
     TimelineKeyframeItem *asTimelineKeyframeItem() override;
 
 protected:

src/plugins/qmldesigner/components/timelineeditor/timelineselectiontool.cpp

@@ -98,7 +98,6 @@ void TimelineSelectionTool::mouseReleaseEvent(TimelineMovableAbstractItem *item,
                                               QGraphicsSceneMouseEvent *event)
 {
     Q_UNUSED(item)
-    Q_UNUSED(event)
 
     commitSelection(selectionMode(event));
 
@@ -108,9 +107,11 @@ void TimelineSelectionTool::mouseReleaseEvent(TimelineMovableAbstractItem *item,
 void TimelineSelectionTool::mouseDoubleClickEvent(TimelineMovableAbstractItem *item,
                                                   QGraphicsSceneMouseEvent *event)
 {
-    Q_UNUSED(item)
     Q_UNUSED(event)
 
+    if (item)
+        item->itemDoubleClicked();
+
     reset();
 }
 

src/plugins/qmldesigner/components/timelineeditor/timelinetooldelegate.cpp

@@ -92,8 +92,10 @@ void TimelineToolDelegate::mouseReleaseEvent(TimelineMovableAbstractItem *item,
 void TimelineToolDelegate::mouseDoubleClickEvent(TimelineMovableAbstractItem *item,
                                                  QGraphicsSceneMouseEvent *event)
 {
-    if (m_currentTool)
+    if (hitCanvas(event)) {
+        m_currentTool = m_selectTool.get();
         m_currentTool->mouseDoubleClickEvent(item, event);
+    }
 
     reset();
 }