History improvements (#337)

* Simplified redoOnce.

* Help mental model.

* Move clear redo stack where it belongs.

* Not needed anymore as we check for same state.

src/history.ts

@@ -22,13 +22,14 @@ class SceneHistory {
       // If the last entry is the same as this one, ignore it
       return;
     }
+
     this.stateHistory.push(newEntry);
+
+    // As a new entry was pushed, we invalidate the redo stack
+    this.clearRedoStack();
   }
 
   restoreEntry(entry: string) {
-    // When restoring, we shouldn't add an history entry otherwise we'll be stuck with it and can't go back
-    this.skipRecording();
-
     try {
       return JSON.parse(entry);
     } catch {
@@ -40,11 +41,15 @@ class SceneHistory {
     this.redoStack.splice(0, this.redoStack.length);
   }
 
-  redoOnce(elements: readonly ExcalidrawElement[]) {
+  redoOnce() {
-    const currentEntry = this.generateCurrentEntry(elements);
+    if (this.redoStack.length === 0) {
+      return null;
+    }
+
     const entryToRestore = this.redoStack.pop();
+
     if (entryToRestore !== undefined) {
-      this.stateHistory.push(currentEntry);
+      this.stateHistory.push(entryToRestore);
       return this.restoreEntry(entryToRestore);
     }
 

src/index.tsx

@@ -291,7 +291,7 @@ export class App extends React.Component<{}, AppState> {
     } else if (event[META_KEY] && event.code === "KeyZ") {
       if (event.shiftKey) {
         // Redo action
-        const data = history.redoOnce(elements);
+        const data = history.redoOnce();
         if (data !== null) {
           elements = data;
         }
@@ -1026,7 +1026,6 @@ export class App extends React.Component<{}, AppState> {
     this.saveDebounced();
     if (history.isRecording()) {
       history.pushEntry(history.generateCurrentEntry(elements));
-      history.clearRedoStack();
     }
   }
 }