fix #4153 -- Jupyter: focus cell on undo/redo

Author: williamstein

src/packages/frontend/frame-editors/jupyter-editor/actions.ts

@@ -190,14 +190,23 @@ export class JupyterEditorActions extends BaseActions<JupyterEditorState> {
 
   // per-session sync-aware undo
   undo(id: string): void {
-    id = id; // not used yet, since only one thing that can be undone.
-    this.jupyter_actions.undo();
+    const actions = this.get_frame_actions(id);
+    if (actions != null) {
+      // this properly moves the selection, so prefer if available
+      actions.undo();
+    } else {
+      this.jupyter_actions.undo();
+    }
   }
 
   // per-session sync-aware redo
   redo(id: string): void {
-    id = id; // not used yet
-    this.jupyter_actions.redo();
+    const actions = this.get_frame_actions(id);
+    if (actions != null) {
+      actions.redo();
+    } else {
+      this.jupyter_actions.redo();
+    }
   }
 
   cut(id: string): void {

src/packages/frontend/frame-editors/jupyter-editor/cell-notebook/actions.ts

@@ -1095,5 +1095,50 @@ export class NotebookFrameActions {
     }
     this.jupyter_actions.syncdb.commit();
   };
+
+  private focusFirstChangedCell = (before) => {
+    const store = this.jupyter_actions.store;
+    const after = store.get("cells");
+    const ids = store.get_cell_list();
+    const id = firstChangedCell({ before, after, ids });
+    if (id) {
+      this.set_cur_id(id);
+      this.scroll("cell visible");
+      setTimeout(() => this.scroll("cell visible"), 1);
+    }
+  };
+
+  undo = () => {
+    const before = this.jupyter_actions.store.get("cells");
+    this.jupyter_actions.syncdb.undo();
+    setTimeout(() => this.focusFirstChangedCell(before), 1);
+  };
+
+  redo = () => {
+    const before = this.jupyter_actions.store.get("cells");
+    this.jupyter_actions.syncdb.redo();
+    setTimeout(() => this.focusFirstChangedCell(before), 1);
+  };
 }
 
+// This function returns the id of the first (minimal position)
+// cell that changed going from before to after, or
+// null if no cells changed.  An annoying subtlety is that if
+// you reorder cells then *all* positions may change (to keep them
+// separated and sane) and then all cells are different.  It's
+// an edge case, and handling it in a more expected way would be much
+// more difficult and slower, so we don't.
+function firstChangedCell({ before, after, ids }): string | null {
+  // before and after are immutablejs cells maps, from
+  // cell id to cell description.
+  if (before.equals(after)) {
+    // obviously, nothing changed.
+    return null;
+  }
+  for (const id of ids) {
+    if (!before.get(id)?.equals(after.get(id))) {
+      return id;
+    }
+  }
+  return null;
+}

src/packages/frontend/jupyter/codemirror-editor.tsx

@@ -411,14 +411,23 @@ export const CodeMirrorEditor: React.FC<CodeMirrorEditorProps> = ({
     ) {
       cm_save();
     }
-    actions.undo();
+    if (frameActions.current) {
+      // prefer this, because can update selection
+      frameActions.current.undo();
+    } else {
+      actions.undo();
+    }
   }
 
   function cm_redo(): void {
     if (cm.current == null || actions == null) {
       return;
     }
-    actions.redo();
+    if (frameActions.current) {
+      frameActions.current.redo();
+    } else {
+      actions.redo();
+    }
   }
 
   function shift_tab_key(): void {

src/packages/frontend/jupyter/commands.ts

@@ -389,7 +389,7 @@ export function commands(actions: AllActions): {
         { alt: true, mode: "escape", which: 90 },
         { ctrl: true, mode: "escape", which: 90 },
       ],
-      f: () => actions.jupyter_actions?.undo(),
+      f: () => actions.frame_actions?.undo(),
     },
 
     "global redo": {
@@ -402,7 +402,7 @@ export function commands(actions: AllActions): {
         { alt: true, mode: "escape", which: 89 },
         { ctrl: true, mode: "escape", which: 89 },
       ],
-      f: () => actions.jupyter_actions?.redo(),
+      f: () => actions.frame_actions?.redo(),
     },
 
     "hide all line numbers": {