From 0040716006de1dddf489647e1da57894de32a122 Mon Sep 17 00:00:00 2001
From: George Gevoian <george@gevoian.com>
Date: Tue, 18 Jul 2023 22:27:53 -0400
Subject: [PATCH] (core) Disable undo/redo in detached formula editor

Summary:
Undo and redo were causing errors to be thrown while the editor was detached. In the
interest of time, we'll disable undo/redo until we have a chance to look at whether
we can support it in the editor.

Test Plan: Manual.

Reviewers: JakubSerafin

Reviewed By: JakubSerafin

Differential Revision: https://phab.getgrist.com/D3959
---
 app/client/components/UndoStack.ts  | 20 +++++++++++++++++---
 app/client/ui/TopBar.ts             | 16 ++++++++++++----
 app/client/widgets/FormulaEditor.ts | 15 ++++++++++++++-
 3 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/app/client/components/UndoStack.ts b/app/client/components/UndoStack.ts
index 259f2868..17306cc8 100644
--- a/app/client/components/UndoStack.ts
+++ b/app/client/components/UndoStack.ts
@@ -25,7 +25,7 @@ export interface IUndoState {
  * position in this stack. Undo and redo actions are generated and sent to the server here.
  */
 export class UndoStack extends dispose.Disposable {
-
+  public isDisabled: Observable<boolean>;
   public undoDisabledObs: ko.Observable<boolean>;
   public redoDisabledObs: ko.Observable<boolean>;
   private _gristDoc: GristDoc;
@@ -40,6 +40,8 @@ export class UndoStack extends dispose.Disposable {
   public create(log: MinimalActionGroup[], options: {gristDoc: GristDoc}) {
     this._gristDoc = options.gristDoc;
 
+    this.isDisabled = Observable.create(this, false);
+
     // TODO: _stack and _linkMap grow without bound within a single session.
     // The top of the stack is stack.length - 1. The pointer points above the most
     // recently applied (not undone) action.
@@ -99,15 +101,27 @@ export class UndoStack extends dispose.Disposable {
   }
 
   // Send an undo action. This should be called when the user presses 'undo'.
-  public sendUndoAction(): Promise<void> {
+  public async sendUndoAction(): Promise<void> {
+    if (this.isDisabled.get()) { return; }
+
     return this._undoChain.add(() => this._sendAction(true));
   }
 
   // Send a redo action. This should be called when the user presses 'redo'.
-  public sendRedoAction(): Promise<void> {
+  public async sendRedoAction(): Promise<void> {
+    if (this.isDisabled.get()) { return; }
+
     return this._undoChain.add(() => this._sendAction(false));
   }
 
+  public enable(): void {
+    this.isDisabled.set(false);
+  }
+
+  public disable(): void {
+    this.isDisabled.set(true);
+  }
+
   private async _sendAction(isUndo: boolean): Promise<void> {
     // Pick the action group to undo or redo.
     const ag = this._stack[isUndo ? this._pointer - 1 : this._pointer];
diff --git a/app/client/ui/TopBar.ts b/app/client/ui/TopBar.ts
index 86e4ff40..3a222d98 100644
--- a/app/client/ui/TopBar.ts
+++ b/app/client/ui/TopBar.ts
@@ -70,6 +70,14 @@ export function createTopBarDoc(owner: MultiHolder, appModel: AppModel, pageMode
       return module.SearchModelImpl.create(use.owner, gristDoc);
     });
 
+  const isUndoRedoAvailable = Computed.create(owner, use => {
+    const gristDoc = use(pageModel.gristDoc);
+    if (!gristDoc) { return false; }
+
+    const undoStack = gristDoc.getUndoStack();
+    return !use(undoStack.isDisabled);
+  });
+
   return [
     // TODO Before gristDoc is loaded, we could show doc-name without the page. For now, we delay
     // showing of breadcrumbs until gristDoc is loaded.
@@ -98,14 +106,14 @@ export function createTopBarDoc(owner: MultiHolder, appModel: AppModel, pageMode
       topBarUndoBtn('Undo',
         dom.on('click', () => state.isUndoDisabled.get() || allCommands.undo.run()),
         hoverTooltip('Undo', {key: 'topBarBtnTooltip'}),
-        cssHoverCircle.cls('-disabled', state.isUndoDisabled),
-        testId('undo')
+        cssHoverCircle.cls('-disabled', use => use(state.isUndoDisabled) || !use(isUndoRedoAvailable)),
+        testId('undo'),
       ),
       topBarUndoBtn('Redo',
         dom.on('click', () => state.isRedoDisabled.get() || allCommands.redo.run()),
         hoverTooltip('Redo', {key: 'topBarBtnTooltip'}),
-        cssHoverCircle.cls('-disabled', state.isRedoDisabled),
-        testId('redo')
+        cssHoverCircle.cls('-disabled', use => use(state.isRedoDisabled) || !use(isUndoRedoAvailable)),
+        testId('redo'),
       ),
       cssSpacer(),
     ]),
diff --git a/app/client/widgets/FormulaEditor.ts b/app/client/widgets/FormulaEditor.ts
index 7f5fa327..91b041d3 100644
--- a/app/client/widgets/FormulaEditor.ts
+++ b/app/client/widgets/FormulaEditor.ts
@@ -153,7 +153,20 @@ export class FormulaEditor extends NewBaseEditor {
     this._isEmpty.addListener(() => this._updateEditorPlaceholder());
 
     // Update the placeholder text when expanding or collapsing the editor.
-    this.isDetached.addListener(() => this._updateEditorPlaceholder());
+    this.isDetached.addListener((isDetached) => {
+      // TODO: look into whether we can support undo/redo while the editor is detached.
+      if (isDetached) {
+        options.gristDoc.getUndoStack().disable();
+      } else {
+        options.gristDoc.getUndoStack().enable();
+      }
+
+      this._updateEditorPlaceholder();
+    });
+
+    this.onDispose(() => {
+      options.gristDoc.getUndoStack().enable();
+    });
 
     this._dom = cssFormulaEditor(
       // switch border shadow