From fda06e4fc3ca32abb26cd1564316f354783fff7a Mon Sep 17 00:00:00 2001 From: David Luzar Date: Thu, 19 Mar 2020 19:41:32 +0100 Subject: [PATCH] Fix history - the 2nd installment (#1014) * don't regenerate versionNonce on pushEntry * fix history handling around multi-point arrows * remove filtering from getElementMap helper --- src/components/App.tsx | 2 +- src/element/index.ts | 2 +- src/element/mutateElement.ts | 2 +- src/history.ts | 44 ++++++++++++++++++++++++++++-------- 4 files changed, 37 insertions(+), 13 deletions(-) diff --git a/src/components/App.tsx b/src/components/App.tsx index 228d0afe..40587128 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -332,7 +332,7 @@ export class App extends React.Component { } return elements; - }, [] as any) + }, [] as Mutable) // add local elements that weren't deleted or on remote .concat(...Object.values(localElementMap)), ); diff --git a/src/element/index.ts b/src/element/index.ts index d640d698..c50552e7 100644 --- a/src/element/index.ts +++ b/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; diff --git a/src/element/mutateElement.ts b/src/element/mutateElement.ts index bc27ecbc..45ba62cd 100644 --- a/src/element/mutateElement.ts +++ b/src/element/mutateElement.ts @@ -53,8 +53,8 @@ export function newElementWith( ): TElement { return { ...element, - ...updates, version: element.version + 1, versionNonce: randomSeed(), + ...updates, }; } diff --git a/src/history.ts b/src/history.ts index f12f5231..a714a936 100644 --- a/src/history.ts +++ b/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), }); }