Browse Source

Fix history - the 2nd installment (#1014)

* don't regenerate versionNonce on pushEntry

* fix history handling around multi-point arrows

* remove filtering from getElementMap helper
David Luzar 5 years ago
parent
commit
fda06e4fc3
4 changed files with 37 additions and 13 deletions
  1. 1 1
      src/components/App.tsx
  2. 1 1
      src/element/index.ts
  3. 1 1
      src/element/mutateElement.ts
  4. 34 10
      src/history.ts

+ 1 - 1
src/components/App.tsx

@@ -332,7 +332,7 @@ export class App extends React.Component<any, AppState> {
                       }
 
                       return elements;
-                    }, [] as any)
+                    }, [] as Mutable<typeof restoredState.elements>)
                     // add local elements that weren't deleted or on remote
                     .concat(...Object.values(localElementMap)),
                 );

+ 1 - 1
src/element/index.ts

@@ -41,7 +41,7 @@ export function getSyncableElements(elements: readonly ExcalidrawElement[]) {
 }
 
 export function getElementMap(elements: readonly ExcalidrawElement[]) {
-  return getSyncableElements(elements).reduce(
+  return elements.reduce(
     (acc: { [key: string]: ExcalidrawElement }, element: ExcalidrawElement) => {
       acc[element.id] = element;
       return acc;

+ 1 - 1
src/element/mutateElement.ts

@@ -53,8 +53,8 @@ export function newElementWith<TElement extends ExcalidrawElement>(
 ): TElement {
   return {
     ...element,
-    ...updates,
     version: element.version + 1,
     versionNonce: randomSeed(),
+    ...updates,
   };
 }

+ 34 - 10
src/history.ts

@@ -25,17 +25,41 @@ export class SceneHistory {
   ) {
     return JSON.stringify({
       appState: clearAppStatePropertiesForHistory(appState),
-      elements: elements.map(element => {
-        if (isLinearElement(element)) {
-          return newElementWith(element, {
-            points:
-              appState.multiElement && appState.multiElement.id === element.id
-                ? element.points.slice(0, -1)
-                : element.points,
-          });
+      elements: elements.reduce((elements, element) => {
+        if (
+          isLinearElement(element) &&
+          appState.multiElement &&
+          appState.multiElement.id === element.id
+        ) {
+          // don't store multi-point arrow if still has only one point
+          if (
+            appState.multiElement &&
+            appState.multiElement.id === element.id &&
+            element.points.length < 2
+          ) {
+            return elements;
+          }
+
+          elements.push(
+            newElementWith(element, {
+              // don't store last point if not committed
+              points:
+                element.lastCommittedPoint !==
+                element.points[element.points.length - 1]
+                  ? element.points.slice(0, -1)
+                  : element.points,
+              // don't regenerate versionNonce else this will short-circuit our
+              //  bail-on-no-change logic in pushEntry()
+              versionNonce: element.versionNonce,
+            }),
+          );
+        } else {
+          elements.push(
+            newElementWith(element, { versionNonce: element.versionNonce }),
+          );
         }
-        return newElementWith(element, {});
-      }),
+        return elements;
+      }, [] as Mutable<typeof elements>),
     });
   }