Fix undo with pages. Closes #6142

packages/core/src/common/index.ts

@@ -14,9 +14,11 @@ export type DisableOptions = { fromMove?: boolean };
 
 export type LocaleOptions = { locale?: boolean };
 
+export type UndoOptions = { fromUndo?: boolean };
+
 export type WithHTMLParserOptions = { parserOptions?: HTMLParserOptions };
 
-export type RemoveOptions = Backbone.Silenceable;
+export type RemoveOptions = Backbone.Silenceable & UndoOptions;
 
 export type EventHandler = Backbone.EventHandler;
 

packages/core/src/pages/index.ts

@@ -75,7 +75,7 @@ export default class PageManager extends ItemManagerModule<PageManagerConfig, Pa
   constructor(em: EditorModel) {
     super(em, 'PageManager', new Pages([], em), PagesEvents);
     bindAll(this, '_onPageChange');
-    const model = new ModuleModel({ _undo: true } as any);
+    const model = new ModuleModel(this, { _undo: true });
     this.model = model;
     this.pages.on('reset', (coll) => coll.at(0) && this.select(coll.at(0)));
     this.pages.on('all', this.__onChange, this);

packages/core/src/pages/model/Pages.ts

@@ -1,4 +1,4 @@
-import { Collection } from '../../common';
+import { Collection, RemoveOptions } from '../../common';
 import EditorModel from '../../editor/model/Editor';
 import Page from './Page';
 
@@ -14,11 +14,13 @@ export default class Pages extends Collection<Page> {
     };
   }
 
-  onReset(m: Page, opts?: { previousModels?: Pages }) {
-    opts?.previousModels?.map((p) => this.onRemove(p));
+  onReset(m: Page, opts?: RemoveOptions & { previousModels?: Pages }) {
+    opts?.previousModels?.map((p) => this.onRemove(p, this, opts));
   }
 
-  onRemove(removed?: Page) {
+  onRemove(removed?: Page, _p?: this, opts: RemoveOptions = {}) {
+    // Avoid removing frames if triggered from undo #6142
+    if (opts.fromUndo) return;
     removed?.onRemove();
   }
 }

packages/core/src/undo_manager/index.ts

@@ -40,6 +40,8 @@ const hasSkip = (opts: any) => opts.avoidStore || opts.noUndo;
 
 const getChanged = (obj: any) => Object.keys(obj.changedAttributes());
 
+const changedMap = new WeakMap();
+
 export default class UndoManagerModule extends Module<UndoManagerConfig & { name?: string; _disable?: boolean }> {
   beforeCache?: any;
   um: any;
@@ -69,22 +71,25 @@ export default class UndoManagerModule extends Module<UndoManagerConfig & { name
         return false;
       },
       on(object: any, v: any, opts: any) {
-        !this.beforeCache && (this.beforeCache = object.previousAttributes());
+        let before = changedMap.get(object);
+        if (!before) {
+          before = object.previousAttributes();
+          changedMap.set(object, before);
+        }
         const opt = opts || v || {};
-        opt.noUndo &&
+        if (hasSkip(opt)) {
           setTimeout(() => {
-            this.beforeCache = null;
+            changedMap.delete(object);
           });
-        if (hasSkip(opt)) {
           return;
         } else {
           const after = object.toJSON({ fromUndo });
           const result = {
             object,
-            before: this.beforeCache,
+            before,
             after,
           };
-          this.beforeCache = null;
+          changedMap.delete(object);
           // Skip undo in case of empty changes
           if (isEmpty(after)) return;
 

packages/core/test/common.ts

@@ -1,17 +1,22 @@
+import CanvasEvents from '../src/canvas/types';
 import Editor from '../src/editor';
+import { EditorConfig } from '../src/editor/config/config';
+import EditorModel from '../src/editor/model/Editor';
 
 // DocEl + Head + Wrapper
 export const DEFAULT_CMPS = 3;
 
-export function setupTestEditor() {
+export function setupTestEditor(opts?: { withCanvas?: boolean; config?: Partial<EditorConfig> }) {
+  document.body.innerHTML = '<div id="fixtures"></div> <div id="canvas-wrp"></div> <div id="editor"></div>';
   const editor = new Editor({
     mediaCondition: 'max-width',
+    el: document.body.querySelector('#editor') as HTMLElement,
     avoidInlineStyle: true,
+    ...opts?.config,
   });
   const em = editor.getModel();
   const dsm = em.DataSources;
-  document.body.innerHTML = '<div id="fixtures"></div>';
-  const { Pages, Components } = em;
+  const { Pages, Components, Canvas } = em;
   Pages.onLoad();
   const cmpRoot = Components.getWrapper()!;
   const View = Components.getType('wrapper')!.view;
@@ -22,9 +27,30 @@ export function setupTestEditor() {
   wrapperEl.render();
   const fixtures = document.body.querySelector('#fixtures')!;
   fixtures.appendChild(wrapperEl.el);
+  const canvasWrapEl = document.body.querySelector('#canvas-wrp')!;
+
+  /**
+   * When trying to render the canvas, seems like jest gets stuck in a loop of iframe.onload (FrameView.ts)
+   * and all subsequent tests containing setTimeout are not executed.
+   */
+  if (opts?.withCanvas) {
+    Canvas.postLoad();
+    canvasWrapEl.appendChild(Canvas.render());
+    editor.on(CanvasEvents.frameLoad, ({ el }) => {
+      // this seems to fix the issue of the loop
+      el.onload = null;
+    });
+    // Enable undo manager
+    editor.Pages.postLoad();
+  }
+
   return { editor, em, dsm, cmpRoot, fixtures: fixtures as HTMLElement };
 }
 
+export function waitEditorEvent(em: Editor | EditorModel, event: string) {
+  return new Promise((resolve) => em.once(event, resolve));
+}
+
 export function flattenHTML(html: string) {
   return html.replace(/>\s+|\s+</g, (m) => m.trim());
 }

packages/core/test/specs/pages/index.ts

@@ -1,8 +1,9 @@
+import CanvasEvents from '../../../src/canvas/types';
 import { ComponentDefinition } from '../../../src/dom_components/model/types';
 import Editor from '../../../src/editor';
 import EditorModel from '../../../src/editor/model/Editor';
 import { PageProperties } from '../../../src/pages/model/Page';
-import { DEFAULT_CMPS } from '../../common';
+import { DEFAULT_CMPS, setupTestEditor, waitEditorEvent } from '../../common';
 
 describe('Pages', () => {
   let editor: Editor;
@@ -281,3 +282,71 @@ describe('Managing pages', () => {
     expect(rule2.getStyle()).toEqual({ color: 'blue' });
   });
 });
+
+describe('Pages in canvas', () => {
+  let editor: Editor;
+  let canvas: Editor['Canvas'];
+  let em: EditorModel;
+  let fxt: HTMLElement;
+  let pm: Editor['Pages'];
+  const clsPageEl = 'cmp';
+  const selPageEl = `.${clsPageEl}`;
+
+  const getPageContent = () => canvas.getBody().querySelector(selPageEl)?.innerHTML;
+
+  beforeEach(async () => {
+    const testEditor = setupTestEditor({
+      withCanvas: true,
+      config: {
+        pageManager: {
+          pages: [
+            {
+              id: 'page-1',
+              component: `<div class="${clsPageEl}">Page 1</div>`,
+            },
+          ],
+        },
+      },
+    });
+    editor = testEditor.editor;
+    canvas = editor.Canvas;
+    em = testEditor.em;
+    fxt = testEditor.fixtures;
+    pm = editor.Pages;
+    await waitEditorEvent(em, 'change:readyCanvas');
+  });
+
+  afterEach(() => {
+    editor.destroy();
+  });
+
+  test('Pages are rendering properly with undo/redo', async () => {
+    const mainPage = pm.getMain();
+    expect(mainPage).toBe(pm.getSelected());
+
+    const page = pm.add(
+      {
+        id: 'page-2',
+        component: `<div class="${clsPageEl}">Page 2</div>`,
+      },
+      { select: true },
+    )!;
+
+    // Check the second page is selected and rendered properly
+    expect(page).toBe(pm.getSelected());
+    await waitEditorEvent(em, CanvasEvents.frameLoadBody);
+    expect(getPageContent()).toEqual('Page 2');
+
+    // Undo and check the main page is rendered properly
+    em.UndoManager.undo();
+    expect(mainPage).toBe(pm.getSelected());
+    await waitEditorEvent(em, CanvasEvents.frameLoadBody);
+    expect(getPageContent()).toBe('Page 1');
+
+    // Redo and check the second page is rendered properly again
+    em.UndoManager.redo();
+    expect(page).toBe(pm.getSelected());
+    await waitEditorEvent(em, CanvasEvents.frameLoadBody);
+    expect(getPageContent()).toEqual('Page 2');
+  });
+});