Fix notebook undo scope

javascript/src/api.ts

@@ -73,8 +73,11 @@ export interface ISharedBase extends IObservableDisposable {
   /**
    * Perform a transaction. While the function f is called, all changes to the shared
    * document are bundled into a single event.
+   *
+   * @param f Transaction to execute
+   * @param undoable Whether to track the change in the action history or not (default `true`)
    */
-  transact(f: () => void): void;
+  transact(f: () => void, undoable?: boolean): void;
 }
 
 /**

javascript/src/ycell.ts

@@ -431,21 +431,23 @@ export class YBaseCell<Metadata extends nbformat.IBaseCellMetadata>
       return;
     }
 
-    this._ymetadata.delete(key);
+    this.transact(() => {
+      this._ymetadata.delete(key);
 
-    const jupyter = this.getMetadata('jupyter') as any;
-    if (key === 'collapsed' && jupyter) {
-      // eslint-disable-next-line @typescript-eslint/no-unused-vars
-      const { outputs_hidden, ...others } = jupyter;
+      const jupyter = this.getMetadata('jupyter') as any;
+      if (key === 'collapsed' && jupyter) {
+        // eslint-disable-next-line @typescript-eslint/no-unused-vars
+        const { outputs_hidden, ...others } = jupyter;
 
-      if (Object.keys(others).length === 0) {
-        this._ymetadata.delete('jupyter');
-      } else {
-        this._ymetadata.set('jupyter', others);
+        if (Object.keys(others).length === 0) {
+          this._ymetadata.delete('jupyter');
+        } else {
+          this._ymetadata.set('jupyter', others);
+        }
+      } else if (key === 'jupyter') {
+        this._ymetadata.delete('collapsed');
       }
-    } else if (key === 'jupyter') {
-      this._ymetadata.delete('collapsed');
-    }
+    }, false);
   }
 
   /**
@@ -524,7 +526,7 @@ export class YBaseCell<Metadata extends nbformat.IBaseCellMetadata>
             this.deleteMetadata('collapsed');
           }
         }
-      });
+      }, false);
     } else {
       const clone = JSONExt.deepCopy(metadata) as any;
       if (clone.collapsed != null) {
@@ -538,7 +540,7 @@ export class YBaseCell<Metadata extends nbformat.IBaseCellMetadata>
           for (const [key, value] of Object.entries(clone)) {
             this._ymetadata.set(key, value);
           }
-        });
+        }, false);
       }
     }
   }
@@ -558,13 +560,16 @@ export class YBaseCell<Metadata extends nbformat.IBaseCellMetadata>
   /**
    * Perform a transaction. While the function f is called, all changes to the shared
    * document are bundled into a single event.
+   *
+   * @param f Transaction to execute
+   * @param undoable Whether to track the change in the action history or not (default `true`)
    */
   transact(f: () => void, undoable = true): void {
-    this.notebook && undoable
-      ? this.notebook.transact(f)
-      : this.ymodel.doc == null
-      ? f()
-      : this.ymodel.doc.transact(f, this);
+    !this.notebook || this.notebook.disableDocumentWideUndoRedo
+      ? this.ymodel.doc == null
+        ? f()
+        : this.ymodel.doc.transact(f, undoable ? this : null)
+      : this.notebook.transact(f, undoable);
   }
 
   /**
@@ -606,22 +611,24 @@ export class YBaseCell<Metadata extends nbformat.IBaseCellMetadata>
             });
             break;
           case 'update':
-            const newValue = this._ymetadata.get(key);
-            const oldValue = change.oldValue;
-            let equal = true;
-            if (typeof oldValue == 'object' && typeof newValue == 'object') {
-              equal = JSONExt.deepEqual(oldValue, newValue);
-            } else {
-              equal = oldValue === newValue;
-            }
-
-            if (!equal) {
-              this._metadataChanged.emit({
-                key,
-                type: 'change',
-                oldValue,
-                newValue
-              });
+            {
+              const newValue = this._ymetadata.get(key);
+              const oldValue = change.oldValue;
+              let equal = true;
+              if (typeof oldValue == 'object' && typeof newValue == 'object') {
+                equal = JSONExt.deepEqual(oldValue, newValue);
+              } else {
+                equal = oldValue === newValue;
+              }
+
+              if (!equal) {
+                this._metadataChanged.emit({
+                  key,
+                  type: 'change',
+                  oldValue,
+                  newValue
+                });
+              }
             }
             break;
         }
@@ -732,7 +739,7 @@ export class YCodeCell
     if (this.ymodel.get('execution_count') !== count) {
       this.transact(() => {
         this.ymodel.set('execution_count', count);
-      });
+      }, false);
     }
   }
 
@@ -865,7 +872,7 @@ class YAttachmentCell
       } else {
         this.ymodel.set('attachments', attachments);
       }
-    });
+    }, false);
   }
 
   /**

javascript/test/ycell.spec.ts

@@ -1,3 +1,4 @@
+/* eslint-disable camelcase */
 // Copyright (c) Jupyter Development Team.
 // Distributed under the terms of the Modified BSD License.
 
@@ -227,4 +228,89 @@ describe('@jupyter/ydoc', () => {
       }
     });
   });
+
+  describe('#undo', () => {
+    test('should undo source change', () => {
+      const codeCell = YCodeCell.create();
+      codeCell.setSource('test');
+      codeCell.undoManager?.stopCapturing();
+      codeCell.updateSource(0, 0, 'hello');
+
+      codeCell.undo();
+
+      expect(codeCell.getSource()).toEqual('test');
+    });
+
+    test('should not undo execution count change', () => {
+      const codeCell = YCodeCell.create();
+      codeCell.setSource('test');
+      codeCell.undoManager?.stopCapturing();
+      codeCell.execution_count = 22;
+      codeCell.undoManager?.stopCapturing();
+      codeCell.execution_count = 42;
+
+      codeCell.undo();
+
+      expect(codeCell.execution_count).toEqual(42);
+    });
+
+    test('should not undo output change', () => {
+      const codeCell = YCodeCell.create();
+      codeCell.setSource('test');
+      codeCell.undoManager?.stopCapturing();
+      const outputs = [
+        {
+          data: {
+            'application/geo+json': {
+              geometry: {
+                coordinates: [-118.4563712, 34.0163116],
+                type: 'Point'
+              },
+              type: 'Feature'
+            },
+            'text/plain': ['<IPython.display.GeoJSON object>']
+          },
+          metadata: {
+            'application/geo+json': {
+              expanded: false
+            }
+          },
+          output_type: 'display_data'
+        },
+        {
+          data: {
+            'application/vnd.jupyter.widget-view+json': {
+              model_id: '4619c172d65e496baa5d1230894b535a',
+              version_major: 2,
+              version_minor: 0
+            },
+            'text/plain': [
+              "HBox(children=(Text(value='text input', layout=Layout(border='1px dashed red', width='80px')), Button(descript…"
+            ]
+          },
+          metadata: {},
+          output_type: 'display_data'
+        }
+      ];
+      codeCell.setOutputs(outputs);
+      codeCell.undoManager?.stopCapturing();
+
+      codeCell.undo();
+
+      expect(codeCell.getOutputs()).toEqual(outputs);
+    });
+
+    test('should not undo metadata change', () => {
+      const codeCell = YCodeCell.create();
+      codeCell.setSource('test');
+      codeCell.undoManager?.stopCapturing();
+      codeCell.setMetadata({ collapsed: false });
+      codeCell.undoManager?.stopCapturing();
+      codeCell.setMetadata({ collapsed: true });
+
+      codeCell.undo();
+
+      expect(codeCell.getMetadata('collapsed')).toEqual(true);
+    });
+  });
 });

javascript/test/ynotebook.spec.ts

@@ -482,5 +482,103 @@ describe('@jupyter/ydoc', () => {
         notebook.dispose();
       });
     });
+
+    describe('#undo', () => {
+      describe('globally', () => {
+        test('should undo cell addition', () => {
+          const notebook = YNotebook.create();
+          notebook.addCell({ cell_type: 'code' });
+          notebook.undoManager.stopCapturing();
+          notebook.addCell({ cell_type: 'markdown' });
+
+          expect(notebook.cells.length).toEqual(2);
+
+          notebook.undo();
+
+          expect(notebook.cells.length).toEqual(1);
+        });
+
+        test('should undo cell source update', () => {
+          const notebook = YNotebook.create();
+          const codeCell = notebook.addCell({ cell_type: 'code' });
+          notebook.undoManager.stopCapturing();
+          notebook.addCell({ cell_type: 'markdown' });
+          notebook.undoManager.stopCapturing();
+          codeCell.updateSource(0, 0, 'print(hello);');
+
+          notebook.undo();
+
+          expect(notebook.cells.length).toEqual(2);
+          expect(notebook.getCell(0).getSource()).toEqual('');
+        });
+
+        test('should undo at global level when called locally', () => {
+          const notebook = YNotebook.create();
+          const codeCell = notebook.addCell({ cell_type: 'code' });
+          notebook.undoManager.stopCapturing();
+          const markdownCell = notebook.addCell({ cell_type: 'markdown' });
+          notebook.undoManager.stopCapturing();
+          codeCell.updateSource(0, 0, 'print(hello);');
+          notebook.undoManager.stopCapturing();
+          markdownCell.updateSource(0, 0, '# Title');
+
+          codeCell.undo();
+
+          expect(notebook.cells.length).toEqual(2);
+          expect(notebook.getCell(0).getSource()).toEqual('print(hello);');
+          expect(notebook.getCell(1).getSource()).toEqual('');
+        });
+      });
+
+      describe('per cells', () => {
+        test('should undo cell addition', () => {
+          const notebook = YNotebook.create({
+            disableDocumentWideUndoRedo: true
+          });
+          notebook.addCell({ cell_type: 'code' });
+          notebook.undoManager.stopCapturing();
+          notebook.addCell({ cell_type: 'markdown' });
+
+          expect(notebook.cells.length).toEqual(2);
+
+          notebook.undo();
+
+          expect(notebook.cells.length).toEqual(1);
+        });
+
+        test('should not undo cell source update', () => {
+          const notebook = YNotebook.create({
+            disableDocumentWideUndoRedo: true
+          });
+          const codeCell = notebook.addCell({ cell_type: 'code' });
+          notebook.undoManager.stopCapturing();
+          notebook.addCell({ cell_type: 'markdown' });
+
+          codeCell.updateSource(0, 0, 'print(hello);');
+
+          notebook.undo();
+
+          expect(notebook.cells.length).toEqual(1);
+          expect(notebook.getCell(0).getSource()).toEqual('print(hello);');
+        });
+
+        test('should only undo cell source update', () => {
+          const notebook = YNotebook.create({
+            disableDocumentWideUndoRedo: true
+          });
+          const codeCell = notebook.addCell({ cell_type: 'code' });
+          notebook.undoManager.stopCapturing();
+          const markdownCell = notebook.addCell({ cell_type: 'markdown' });
+          codeCell.updateSource(0, 0, 'print(hello);');
+          markdownCell.updateSource(0, 0, '# Title');
+
+          codeCell.undo();
+
+          expect(notebook.cells.length).toEqual(2);
+          expect(notebook.getCell(0).getSource()).toEqual('');
+          expect(notebook.getCell(1).getSource()).toEqual('# Title');
+        });
+      });
+    });
   });
 });