Merge pull request musescore#24543 from cbjeukendrup/cleanup_undoredo…

…model [techdebt] Clean up UndoRedoModel
cbjeukendrup · Oct 25, 2024 · 902cbae · 902cbae
2 parents c95bb47 + 8afb0f6
commit 902cbae
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 37 deletions.
diff --git a/src/notation/view/internal/undoredomodel.cpp b/src/notation/view/internal/undoredomodel.cpp
@@ -25,53 +25,69 @@
 #include "uicomponents/view/menuitem.h"
 
 using namespace mu::notation;
+using namespace muse;
 using namespace muse::uicomponents;
 
 UndoRedoModel::UndoRedoModel(QObject* parent)
-    : QObject(parent), muse::Injectable(muse::iocCtxForQmlObject(this))
+    : QObject(parent), Injectable(iocCtxForQmlObject(this))
 {
 }
 
-QVariant UndoRedoModel::makeUndoItem()
+void UndoRedoModel::load()
 {
-    MenuItem* item = new MenuItem(actionsRegister()->action("undo"), this);
+    context()->currentNotationChanged().onNotify(this, [this]() {
+        updateItems();
 
-    muse::ui::UiActionState state;
-    state.enabled = undoStack() ? undoStack()->canUndo() : false;
-    item->setState(state);
+        auto stack = undoStack();
+        if (stack) {
+            stack->stackChanged().onNotify(this, [this]() {
+                updateItems();
+            });
+        }
+    });
 
-    return QVariant::fromValue(item);
-}
+    auto stack = undoStack();
+    if (stack) {
+        stack->stackChanged().onNotify(this, [this]() {
+            updateItems();
+        });
+    }
 
-QVariant UndoRedoModel::makeRedoItem()
-{
-    MenuItem* item = new MenuItem(actionsRegister()->action("redo"), this);
+    m_undoItem = new MenuItem(actionsRegister()->action("undo"), this);
+    m_redoItem = new MenuItem(actionsRegister()->action("redo"), this);
 
-    muse::ui::UiActionState state;
-    state.enabled = undoStack() ? undoStack()->canRedo() : false;
-    item->setState(state);
+    updateItems();
 
-    return QVariant::fromValue(item);
+    // Only to let QML know that `m_undoItem` and `m_redoItem` have been initialised;
+    // changes to their properties will be communicated via those MenuItems' own signals.
+    emit itemsChanged();
 }
 
-void UndoRedoModel::load()
+MenuItem* UndoRedoModel::undoItem() const
 {
-    context()->currentNotationChanged().onNotify(this, [this]() {
-        if (!undoStack()) {
-            emit stackChanged();
-            return;
-        }
+    return m_undoItem;
+}
 
-        undoStack()->stackChanged().onNotify(this, [this]() {
-            emit stackChanged();
-        });
-    });
+MenuItem* UndoRedoModel::redoItem() const
+{
+    return m_redoItem;
+}
 
-    context()->currentProjectChanged().onNotify(this, [this]() {
-        emit stackChanged();
-    });
+void UndoRedoModel::updateItems()
+{
+    auto stack = undoStack();
 
-    emit stackChanged();
+    if (m_undoItem) {
+        ui::UiActionState state;
+        state.enabled = stack ? stack->canUndo() : false;
+        m_undoItem->setState(state);
+    }
+
+    if (m_redoItem) {
+        ui::UiActionState state;
+        state.enabled = stack ? stack->canRedo() : false;
+        m_redoItem->setState(state);
+    }
 }
 
 void UndoRedoModel::redo()

diff --git a/src/notation/view/internal/undoredomodel.h b/src/notation/view/internal/undoredomodel.h
@@ -24,37 +24,47 @@
 
 #include <QObject>
 
+#include "async/asyncable.h"
 #include "context/iglobalcontext.h"
-#include "ui/iuiactionsregister.h"
 #include "modularity/ioc.h"
-#include "async/asyncable.h"
+#include "ui/iuiactionsregister.h"
+
+namespace muse::uicomponents {
+class MenuItem;
+}
 
 namespace mu::notation {
 class UndoRedoModel : public QObject, public muse::Injectable, public muse::async::Asyncable
 {
     Q_OBJECT
 
-    Q_PROPERTY(QVariant undoItem READ makeUndoItem NOTIFY stackChanged)
-    Q_PROPERTY(QVariant redoItem READ makeRedoItem NOTIFY stackChanged)
+    Q_PROPERTY(muse::uicomponents::MenuItem * undoItem READ undoItem NOTIFY itemsChanged)
+    Q_PROPERTY(muse::uicomponents::MenuItem * redoItem READ redoItem NOTIFY itemsChanged)
 
     muse::Inject<context::IGlobalContext> context = { this };
     muse::Inject<muse::ui::IUiActionsRegister> actionsRegister = { this };
 
 public:
     explicit UndoRedoModel(QObject* parent = nullptr);
 
-    QVariant makeUndoItem();
-    QVariant makeRedoItem();
-
     Q_INVOKABLE void load();
+
+    muse::uicomponents::MenuItem* undoItem() const;
+    muse::uicomponents::MenuItem* redoItem() const;
+
     Q_INVOKABLE void undo();
     Q_INVOKABLE void redo();
 
 signals:
-    void stackChanged();
+    void itemsChanged();
 
 private:
     INotationUndoStackPtr undoStack() const;
+
+    void updateItems();
+
+    muse::uicomponents::MenuItem* m_undoItem = nullptr;
+    muse::uicomponents::MenuItem* m_redoItem = nullptr;
 };
 }