#6629 Make undo redo work without code pane being open (#7511)

* move useHotkey for undo/redo into App

* _editorView should be private

* get editorView should be a real get method for consistency

* resolve tsc errors

* fmt

* setView, setState are not exposed

* make undo work without editorview when kcl pane is closed

* lint

* circular deps

* resolve circular deps

* fix undo being 1 step late

* unrelated console.warn removed

* fix undo when code pane is closed during editing

* cleanup

* allow undo to get beyond when code editor has been mounted

* fix up clearHistory

* add test for testing  Undo with closed code pane

e2e/playwright/fixtures/editorFixture.ts

@@ -183,14 +183,15 @@ export class EditorFixture {
   scrollToText(text: string, placeCursor?: boolean) {
     return this.page.evaluate(
       (args: { text: string; placeCursor?: boolean }) => {
+        const editorView = window.editorManager.getEditorView()
         // error TS2339: Property 'docView' does not exist on type 'EditorView'.
         // Except it does so :shrug:
         // @ts-ignore
-        let index = window.editorManager._editorView?.docView.view.state.doc
+        const index = editorView?.docView.view.state.doc
           .toString()
           .indexOf(args.text)
-        window.editorManager._editorView?.focus()
+        editorView?.focus()
-        window.editorManager._editorView?.dispatch({
+        editorView?.dispatch({
           selection: window.EditorSelection.create([
             window.EditorSelection.cursor(index),
           ]),

e2e/playwright/sketch-tests.spec.ts

@@ -1478,6 +1478,7 @@ sketch001 = startSketchOn(XZ)
     await page.mouse.move(1200, 139)
     await page.mouse.down()
     await page.mouse.move(870, 250)
+    await page.mouse.up()
 
     await page.waitForTimeout(200)
 
@@ -1487,6 +1488,60 @@ sketch001 = startSketchOn(XZ)
     )
   })
 
+  test('Can undo with closed code pane', async ({
+    page,
+    homePage,
+    editor,
+    toolbar,
+    scene,
+    cmdBar,
+  }) => {
+    const u = await getUtils(page)
+
+    const viewportSize = { width: 1500, height: 750 }
+    await page.setBodyDimensions(viewportSize)
+
+    await page.addInitScript(async () => {
+      localStorage.setItem(
+        'persistCode',
+        `@settings(defaultLengthUnit=in)
+sketch001 = startSketchOn(XZ)
+  |> startProfile(at = [-10, -10])
+  |> line(end = [20.0, 10.0])
+  |> tangentialArc(end = [5.49, 8.37])`
+      )
+    })
+
+    await homePage.goToModelingScene()
+    await toolbar.waitForFeatureTreeToBeBuilt()
+    await scene.settled(cmdBar)
+
+    await (await toolbar.getFeatureTreeOperation('Sketch', 0)).dblclick()
+
+    await page.waitForTimeout(1000)
+
+    await page.mouse.move(1200, 139)
+    await page.mouse.down()
+    await page.mouse.move(870, 250)
+    await page.mouse.up()
+
+    await editor.expectEditor.toContain(`tangentialArc(end=[-5.85,4.32])`, {
+      shouldNormalise: true,
+    })
+
+    await u.closeKclCodePanel()
+
+    // Undo the last change
+    await page.keyboard.down('Control')
+    await page.keyboard.press('KeyZ')
+    await page.keyboard.up('Control')
+
+    await u.openKclCodePanel()
+    await editor.expectEditor.toContain(`tangentialArc(end = [5.49, 8.37])`, {
+      shouldNormalise: true,
+    })
+  })
+
   test('Can delete a single segment line with keyboard', async ({
     page,
     scene,

e2e/playwright/test-utils.ts

@@ -158,10 +158,10 @@ async function openKclCodePanel(page: Page) {
   await page.evaluate(() => {
     // editorManager is available on the window object.
     //@ts-ignore this is in an entirely different context that tsc can't see.
-    editorManager._editorView.dispatch({
+    editorManager.getEditorView().dispatch({
       selection: {
         //@ts-ignore this is in an entirely different context that tsc can't see.
-        anchor: editorManager._editorView.docView.length,
+        anchor: editorManager.getEditorView().docView.length,
       },
       scrollIntoView: true,
     })

src/App.tsx

@@ -28,6 +28,7 @@ import {
   codeManager,
   kclManager,
   settingsActor,
+  editorManager,
   getSettings,
 } from '@src/lib/singletons'
 import { maybeWriteToDisk } from '@src/lib/telemetry'
@@ -107,6 +108,16 @@ export function App() {
   useHotkeys('backspace', (e) => {
     e.preventDefault()
   })
+  // Since these already exist in the editor, we don't need to define them
+  // with the wrapper.
+  useHotkeys('mod+z', (e) => {
+    e.preventDefault()
+    editorManager.undo()
+  })
+  useHotkeys('mod+shift+z', (e) => {
+    e.preventDefault()
+    editorManager.redo()
+  })
   useHotkeyWrapper(
     [isDesktop() ? 'mod + ,' : 'shift + mod + ,'],
     () => navigate(filePath + PATHS.SETTINGS),

src/Toolbar.tsx

@@ -88,14 +88,14 @@ export function Toolbar({
       modelingState: state,
       modelingSend: send,
       sketchPathId,
-      editorHasFocus: editorManager.editorView?.hasFocus,
+      editorHasFocus: editorManager.getEditorView()?.hasFocus,
     }),
     [
       state,
       send,
       commandBarActor.send,
       sketchPathId,
-      editorManager.editorView?.hasFocus,
+      editorManager.getEditorView()?.hasFocus,
     ]
   )
 

src/clientSideScene/sceneEntities.ts

@@ -3943,7 +3943,7 @@ function isGroupStartProfileForCurrentProfile(sketchEntryNodePath: PathToNode) {
   }
 }
 
-// Returns the 2D tangent direction vector at the end of the segmentGroup if it's an arc.
+// Returns the 2D tangent direction vector at the end of the segmentGroup
 function findTangentDirection(segmentGroup: Group) {
   let tangentDirection: Coords2d | undefined
   if (segmentGroup.userData.type === TANGENTIAL_ARC_TO_SEGMENT) {
@@ -3972,11 +3972,6 @@ function findTangentDirection(segmentGroup: Group) {
     const from = segmentGroup.userData.from as Coords2d
     tangentDirection = subVec(to, from)
     tangentDirection = normalizeVec(tangentDirection)
-  } else {
-    console.warn(
-      'Unsupported segment type for tangent direction calculation: ',
-      segmentGroup.userData.type
-    )
   }
   return tangentDirection
 }

src/components/ModelingSidebar/ModelingPanes/CodeEditor.tsx

@@ -65,7 +65,7 @@ const CodeEditor = forwardRef<CodeEditorRef, CodeEditorProps>((props, ref) => {
   } = props
   const editor = useRef<HTMLDivElement>(null)
 
-  const { view, state, container } = useCodeMirror({
+  const { view, container } = useCodeMirror({
     container: editor.current,
     onCreateEditor,
     extensions,
@@ -77,8 +77,8 @@ const CodeEditor = forwardRef<CodeEditorRef, CodeEditorProps>((props, ref) => {
 
   useImperativeHandle(
     ref,
-    () => ({ editor: editor.current, view: view, state: state }),
+    () => ({ editor: editor.current, view: view, state: view?.state }),
-    [editor, container, view, state]
+    [editor, container, view]
   )
 
   return <div ref={editor}></div>
@@ -138,7 +138,7 @@ export function useCodeMirror(props: UseCodeMirror) {
           parent: container,
         })
         setView(viewCurrent)
-        onCreateEditor && onCreateEditor(viewCurrent)
+        onCreateEditor?.(viewCurrent)
       }
     }
     return () => {
@@ -156,6 +156,7 @@ export function useCodeMirror(props: UseCodeMirror) {
       if (view) {
         view.destroy()
         setView(undefined)
+        onCreateEditor?.(null)
       }
     },
     [view]
@@ -175,7 +176,7 @@ export function useCodeMirror(props: UseCodeMirror) {
     }
   }, [targetExtensions, view, isFirstRender])
 
-  return { view, setView, container, setContainer, state, setState }
+  return { view, container, setContainer, state }
 }
 
 export default CodeEditor

src/components/ModelingSidebar/ModelingPanes/FeatureTreePane.tsx

@@ -45,7 +45,7 @@ export const FeatureTreePane = () => {
       guards: {
         codePaneIsOpen: () =>
           modelingState.context.store.openPanes.includes('code') &&
-          editorManager.editorView !== null,
+          editorManager.getEditorView() !== null,
       },
       actions: {
         openCodePane: () => {

src/components/ModelingSidebar/ModelingPanes/KclEditorPane.tsx

@@ -6,6 +6,7 @@ import {
 import {
   defaultKeymap,
   history,
+  historyField,
   historyKeymap,
   indentWithTab,
 } from '@codemirror/commands'
@@ -37,13 +38,12 @@ import interact from '@replit/codemirror-interact'
 import { TEST } from '@src/env'
 import { useSelector } from '@xstate/react'
 import { useEffect, useMemo, useRef } from 'react'
-import { useHotkeys } from 'react-hotkeys-hook'
 
 import { useLspContext } from '@src/components/LspProvider'
 import CodeEditor from '@src/components/ModelingSidebar/ModelingPanes/CodeEditor'
 import { lineHighlightField } from '@src/editor/highlightextension'
 import { modelingMachineEvent } from '@src/editor/manager'
-import { codeManagerHistoryCompartment } from '@src/lang/codeManager'
+import { historyCompartment } from '@src/editor/compartments'
 import { codeManager, editorManager, kclManager } from '@src/lib/singletons'
 import { Themes, getSystemTheme } from '@src/lib/theme'
 import { onMouseDragMakeANewNumber, onMouseDragRegex } from '@src/lib/utils'
@@ -75,17 +75,6 @@ export const KclEditorPane = () => {
       : context.app.theme.current
   const { copilotLSP, kclLSP } = useLspContext()
 
-  // Since these already exist in the editor, we don't need to define them
-  // with the wrapper.
-  useHotkeys('mod+z', (e) => {
-    e.preventDefault()
-    editorManager.undo()
-  })
-  useHotkeys('mod+shift+z', (e) => {
-    e.preventDefault()
-    editorManager.redo()
-  })
-
   // When this component unmounts, we need to tell the machine that the editor
   useEffect(() => {
     return () => {
@@ -96,12 +85,13 @@ export const KclEditorPane = () => {
   }, [])
 
   useEffect(() => {
-    if (!editorIsMounted || !lastSelectionEvent || !editorManager.editorView) {
+    const editorView = editorManager.getEditorView()
+    if (!editorIsMounted || !lastSelectionEvent || !editorView) {
       return
     }
 
     try {
-      editorManager.editorView.dispatch({
+      editorView.dispatch({
         selection: lastSelectionEvent.codeMirrorSelection,
         annotations: [modelingMachineEvent, Transaction.addToHistory.of(false)],
         scrollIntoView: lastSelectionEvent.scrollIntoView,
@@ -119,13 +109,21 @@ export const KclEditorPane = () => {
   // Instead, hot load hotkeys via code mirror native.
   const codeMirrorHotkeys = codeManager.getCodemirrorHotkeys()
 
+  // When opening the editor, use the existing history in editorManager.
+  // This is needed to ensure users can undo beyond when the editor has been openeed.
+  // (Another solution would be to reuse the same state instead of creating a new one in CodeEditor.)
+  const existingHistory = editorManager.editorState.field(historyField)
+  const initialHistory = existingHistory
+    ? historyField.init(() => existingHistory)
+    : history()
+
   const editorExtensions = useMemo(() => {
     const extensions = [
       drawSelection({
         cursorBlinkRate: cursorBlinking.current ? 1200 : 0,
       }),
       lineHighlightField,
-      codeManagerHistoryCompartment.of(history()),
+      historyCompartment.of(initialHistory),
       closeBrackets(),
       codeFolding(),
       keymap.of([
@@ -206,10 +204,9 @@ export const KclEditorPane = () => {
           extensions={editorExtensions}
           theme={theme}
           onCreateEditor={(_editorView) => {
-            if (_editorView === null) return
-
             editorManager.setEditorView(_editorView)
-            kclEditorActor.send({ type: 'setKclEditorMounted', data: true })
+
+            if (!_editorView) return
 
             // Update diagnostics as they are cleared when the editor is unmounted.
             // Without this, errors would not be shown when closing and reopening the editor.

src/editor/compartments.ts

@@ -0,0 +1,3 @@
+import { Compartment } from '@codemirror/state'
+
+export const historyCompartment = new Compartment()

src/editor/manager.ts

@@ -1,10 +1,22 @@
-import { redo, undo } from '@codemirror/commands'
+import {
+  defaultKeymap,
+  history,
+  historyKeymap,
+  redo,
+  undo,
+} from '@codemirror/commands'
 import { syntaxTree } from '@codemirror/language'
 import type { Diagnostic } from '@codemirror/lint'
 import { forEachDiagnostic, setDiagnosticsEffect } from '@codemirror/lint'
-import { Annotation, EditorSelection, Transaction } from '@codemirror/state'
+import {
+  Annotation,
+  EditorSelection,
+  EditorState,
+  Transaction,
+  type TransactionSpec,
+} from '@codemirror/state'
 import type { ViewUpdate } from '@codemirror/view'
-import { EditorView } from '@codemirror/view'
+import { EditorView, keymap } from '@codemirror/view'
 import type { StateFrom } from 'xstate'
 
 import {
@@ -22,6 +34,8 @@ import type {
   ModelingMachineEvent,
   modelingMachine,
 } from '@src/machines/modelingMachine'
+import { historyCompartment } from '@src/editor/compartments'
+import type CodeManager from '@src/lang/codeManager'
 
 declare global {
   interface Window {
@@ -65,11 +79,28 @@ export default class EditorManager {
 
   private _highlightRange: Array<[number, number]> = [[0, 0]]
 
-  public _editorView: EditorView | null = null
+  private _editorState: EditorState
+  private _editorView: EditorView | null = null
   public kclManager?: KclManager
+  public codeManager?: CodeManager
 
   constructor(engineCommandManager: EngineCommandManager) {
     this.engineCommandManager = engineCommandManager
+
+    this._editorState = EditorState.create({
+      doc: '',
+      extensions: [
+        historyCompartment.of(history()),
+        keymap.of([...defaultKeymap, ...historyKeymap]),
+      ],
+    })
+  }
+
+  get editorState(): EditorState {
+    return this._editorView?.state || this._editorState
+  }
+  get state() {
+    return this.editorState
   }
 
   setCopilotEnabled(enabled: boolean) {
@@ -80,12 +111,25 @@ export default class EditorManager {
     return this._copilotEnabled
   }
 
-  setEditorView(editorView: EditorView) {
+  // Invoked when editorView is created and each time when it is updated (eg. user is sketching)..
+  setEditorView(editorView: EditorView | null) {
+    // Update editorState to the latest editorView state.
+    // This is needed because if kcl pane is closed, editorView will become null but we still want to use the last state.
+    this._editorState = editorView?.state || this._editorState
+
     this._editorView = editorView
-    kclEditorActor.send({ type: 'setKclEditorMounted', data: true })
+
+    kclEditorActor.send({
+      type: 'setKclEditorMounted',
+      data: Boolean(editorView),
+    })
     this.overrideTreeHighlighterUpdateForPerformanceTracking()
   }
 
+  getEditorView(): EditorView | null {
+    return this._editorView
+  }
+
   overrideTreeHighlighterUpdateForPerformanceTracking() {
     // @ts-ignore
     this._editorView?.plugins.forEach((e) => {
@@ -132,10 +176,6 @@ export default class EditorManager {
     return this._isAllTextSelected
   }
 
-  get editorView(): EditorView | null {
-    return this._editorView
-  }
-
   get isShiftDown(): boolean {
     return this._isShiftDown
   }
@@ -287,12 +327,39 @@ export default class EditorManager {
   undo() {
     if (this._editorView) {
       undo(this._editorView)
+    } else if (this._editorState) {
+      const undoPerformed = undo(this) // invokes dispatch which updates this._editorState
+      if (undoPerformed) {
+        const newState = this._editorState
+        // Update the code, this is similar to kcl/index.ts / update, updateDoc,
+        // needed to update the code, so sketch segments can update themselves.
+        // In the editorView case this happens within the kcl plugin's update method being called during updates.
+        this.codeManager!.code = newState.doc.toString()
+        void this.kclManager!.executeCode()
+      }
     }
   }
 
   redo() {
     if (this._editorView) {
       redo(this._editorView)
+    } else if (this._editorState) {
+      const redoPerformed = redo(this)
+      if (redoPerformed) {
+        const newState = this._editorState
+        this.codeManager!.code = newState.doc.toString()
+        void this.kclManager!.executeCode()
+      }
+    }
+  }
+
+  // Invoked by codeMirror during undo/redo.
+  // Call with incorrect "this" so it needs to be an arrow function.
+  dispatch = (spec: TransactionSpec) => {
+    if (this._editorView) {
+      this._editorView.dispatch(spec)
+    } else if (this._editorState) {
+      this._editorState = this._editorState.update(spec).state
     }
   }
 

src/hooks/useQueryParamEffects.ts

@@ -50,8 +50,6 @@ export function useQueryParamEffects() {
     searchParams.has(CMD_NAME_QUERY_PARAM) &&
     searchParams.has(CMD_GROUP_QUERY_PARAM)
 
-  console.log(window.location.href)
-
   /**
    * Watches for legacy `?create-file` hook, which share links currently use.
    */

src/lang/codeManager.ts

@@ -2,10 +2,11 @@
 // NOT updating the code state when we don't need to.
 // This prevents re-renders of the codemirror editor, when typing.
 import { history } from '@codemirror/commands'
-import { Annotation, Compartment, Transaction } from '@codemirror/state'
+import { Annotation, Transaction } from '@codemirror/state'
-import type { EditorView, KeyBinding } from '@codemirror/view'
+import type { KeyBinding } from '@codemirror/view'
 import toast from 'react-hot-toast'
 
+import { historyCompartment } from '@src/editor/compartments'
 import type { Program } from '@src/lang/wasm'
 import { parse, recast } from '@src/lang/wasm'
 import { bracket } from '@src/lib/exampleKcl'
@@ -17,7 +18,6 @@ const PERSIST_CODE_KEY = 'persistCode'
 
 const codeManagerUpdateAnnotation = Annotation.define<boolean>()
 export const codeManagerUpdateEvent = codeManagerUpdateAnnotation.of(true)
-export const codeManagerHistoryCompartment = new Compartment()
 
 export default class CodeManager {
   private _code: string = bracket
@@ -103,25 +103,24 @@ export default class CodeManager {
 
   /**
    * Update the code in the editor.
+   * This is invoked when a segment is being dragged on the canvas, among other things.
    */
   updateCodeEditor(code: string, clearHistory?: boolean): void {
     this.code = code
-    if (editorManager.editorView) {
+    if (clearHistory) {
-      if (clearHistory) {
+      clearCodeMirrorHistory()
-        clearCodeMirrorHistory(editorManager.editorView)
-      }
-      editorManager.editorView.dispatch({
-        changes: {
-          from: 0,
-          to: editorManager.editorView.state.doc.length,
-          insert: code,
-        },
-        annotations: [
-          codeManagerUpdateEvent,
-          Transaction.addToHistory.of(!clearHistory),
-        ],
-      })
     }
+    editorManager.dispatch({
+      changes: {
+        from: 0,
+        to: editorManager.editorState?.doc.length || 0,
+        insert: code,
+      },
+      annotations: [
+        codeManagerUpdateEvent,
+        Transaction.addToHistory.of(!clearHistory),
+      ],
+    })
   }
 
   /**
@@ -213,16 +212,16 @@ function safeLSSetItem(key: string, value: string) {
   localStorage?.setItem(key, value)
 }
 
-function clearCodeMirrorHistory(view: EditorView) {
+function clearCodeMirrorHistory() {
   // Clear history
-  view.dispatch({
+  editorManager.dispatch({
-    effects: [codeManagerHistoryCompartment.reconfigure([])],
+    effects: [historyCompartment.reconfigure([])],
     annotations: [codeManagerUpdateEvent],
   })
 
   // Add history back
-  view.dispatch({
+  editorManager.dispatch({
-    effects: [codeManagerHistoryCompartment.reconfigure([history()])],
+    effects: [historyCompartment.reconfigure([history()])],
     annotations: [codeManagerUpdateEvent],
   })
 }

src/lib/promptToEdit.tsx

@@ -509,7 +509,7 @@ export async function promptToEditFlow({
     const ranges: SelectionRange[] = diff.insertRanges.map((range) =>
       EditorSelection.range(range[0], range[1])
     )
-    editorManager?.editorView?.dispatch({
+    editorManager?.getEditorView()?.dispatch({
       selection: EditorSelection.create(
         ranges,
         selections.graphSelections.length - 1

src/lib/singletons.ts

@@ -69,6 +69,7 @@ export const kclManager = new KclManager(engineCommandManager, {
 // method requires it for the current ast.
 // CYCLIC REF
 editorManager.kclManager = kclManager
+editorManager.codeManager = codeManager
 
 // These are all late binding because of their circular dependency.
 // TODO: proper dependency injection.

src/menu/channels.ts

@@ -7,6 +7,8 @@ export type MenuLabels =
   | 'Help.Command Palette...'
   | 'Help.Report a bug'
   | 'Help.Replay onboarding tutorial'
+  | 'Edit.Undo'
+  | 'Edit.Redo'
   | 'Edit.Rename project'
   | 'Edit.Delete project'
   | 'Edit.Change project directory'

src/menu/editRole.ts

@@ -149,8 +149,24 @@ export const modelingEditRole = (
         },
       },
       { type: 'separator' },
-      { role: 'undo' },
+      {
-      { role: 'redo' },
+        label: 'Undo',
+        accelerator: 'CmdOrCtrl+Z',
+        click: () => {
+          typeSafeWebContentsSend(mainWindow, 'menu-action-clicked', {
+            menuLabel: 'Edit.Undo',
+          })
+        },
+      },
+      {
+        label: 'Redo',
+        accelerator: 'Shift+CmdOrCtrl+Z',
+        click: () => {
+          typeSafeWebContentsSend(mainWindow, 'menu-action-clicked', {
+            menuLabel: 'Edit.Redo',
+          })
+        },
+      },
       { type: 'separator' },
       { role: 'cut' },
       { role: 'copy' },

src/menu/register.ts

@@ -5,8 +5,12 @@ import type { SettingsType } from '@src/lib/settings/initialSettings'
 import { engineCommandManager, sceneInfra } from '@src/lib/singletons'
 import { reportRejection } from '@src/lib/trap'
 import { uuidv4 } from '@src/lib/utils'
-import { authActor, settingsActor } from '@src/lib/singletons'
+import {
-import { commandBarActor } from '@src/lib/singletons'
+  authActor,
+  commandBarActor,
+  editorManager,
+  settingsActor,
+} from '@src/lib/singletons'
 import type { WebContentSendPayload } from '@src/menu/channels'
 import type { NavigateFunction } from 'react-router-dom'
 
@@ -119,6 +123,10 @@ export function modelingMenuCallbackMostActions(
         type: 'Find and select command',
         data: { name: 'format-code', groupId: 'code' },
       })
+    } else if (data.menuLabel === 'Edit.Undo') {
+      editorManager.undo()
+    } else if (data.menuLabel === 'Edit.Redo') {
+      editorManager.redo()
     } else if (data.menuLabel === 'View.Orthographic view') {
       settingsActor.send({
         type: 'set.modeling.cameraProjection',

src/menu/roles.ts

@@ -33,6 +33,8 @@ type EditRoleLabel =
   | 'Rename project'
   | 'Delete project'
   | 'Change project directory'
+  | 'Undo'
+  | 'Redo'
   | 'Speech'
   | 'Edit parameter'
   | 'Modify with Zoo Text-To-CAD'