fix: group contiguous note edits into a single undo step

src/actions/index.ts

@@ -73,6 +73,7 @@ export { default as setFirstSubthought } from './setFirstSubthought'
 export { default as setImportThoughtPath } from './setImportThoughtPath'
 export { default as setIsMulticursorExecuting } from './setIsMulticursorExecuting'
 export { default as setNoteFocus } from './setNoteFocus'
+export { default as setNoteOffset } from './setNoteOffset'
 export { default as setRemoteSearch } from './setRemoteSearch'
 export { default as setResourceCache } from './setResourceCache'
 export { default as setSortPreference } from './setSortPreference'

src/actions/setNoteOffset.ts

@@ -0,0 +1,21 @@
+import State from '../@types/State'
+import Thunk from '../@types/Thunk'
+import { registerActionMetadata } from '../util/actionMetadata.registry'
+
+/** Sets the note caret offset without affecting undo history. */
+const setNoteOffset = (state: State, { value }: { value: number | null }): State => ({
+  ...state,
+  noteOffset: value,
+})
+
+/** Action-creator for setNoteOffset. */
+export const setNoteOffsetActionCreator =
+  (payload: Parameters<typeof setNoteOffset>[1]): Thunk =>
+  dispatch =>
+    dispatch({ type: 'setNoteOffset', ...payload })
+
+export default setNoteOffset
+
+registerActionMetadata('setNoteOffset', {
+  undoable: false,
+})

src/commands/__tests__/undo-redo.ts

@@ -7,6 +7,8 @@ import { indentActionCreator as indent } from '../../actions/indent'
 import { moveThoughtDownActionCreator as moveThoughtDown } from '../../actions/moveThoughtDown'
 import { newThoughtActionCreator as newThought } from '../../actions/newThought'
 import { redoActionCreator as redo } from '../../actions/redo'
+import { setNoteOffsetActionCreator as setNoteOffset } from '../../actions/setNoteOffset'
+import { toggleNoteActionCreator as toggleNote } from '../../actions/toggleNote'
 import { undoActionCreator as undo } from '../../actions/undo'
 import { executeCommandWithMulticursor } from '../../commands'
 import moveThoughtDownCommand from '../../commands/moveThoughtDown'
@@ -562,6 +564,155 @@ describe('grouping', () => {
     expect(exported).toEqual(expectedOutput)
   })
 
+  it('contiguous note additions should undo and redo in one step', () => {
+    store.dispatch([
+      importText({
+        text: `
+        - note-add
+          - =note
+            - x`,
+      }),
+      editThought(['note-add', '=note', 'x'], 'xy'),
+      editThought(['note-add', '=note', 'xy'], 'xyz'),
+      undo(),
+    ])
+
+    expect(exportContext(store.getState(), ['note-add'], 'text/plain')).toEqual(`- note-add
+  - =note
+    - x`)
+
+    store.dispatch(redo())
+
+    expect(exportContext(store.getState(), ['note-add'], 'text/plain')).toEqual(`- note-add
+  - =note
+    - xyz`)
+  })
+
+  it('contiguous note deletes should undo in one step', () => {
+    store.dispatch([
+      importText({
+        text: `
+        - note-delete
+          - =note
+            - xyz`,
+      }),
+      editThought(['note-delete', '=note', 'xyz'], 'xy'),
+      editThought(['note-delete', '=note', 'xy'], 'x'),
+      undo(),
+    ])
+
+    expect(exportContext(store.getState(), ['note-delete'], 'text/plain')).toEqual(`- note-delete
+  - =note
+    - xyz`)
+  })
+
+  it('note replacements should stay as separate undo steps', () => {
+    store.dispatch([
+      importText({
+        text: `
+        - note-replace
+          - =note
+            - cat`,
+      }),
+      editThought(['note-replace', '=note', 'cat'], 'bat'),
+      editThought(['note-replace', '=note', 'bat'], 'bit'),
+      undo(),
+    ])
+
+    expect(exportContext(store.getState(), ['note-replace'], 'text/plain')).toEqual(`- note-replace
+  - =note
+    - bat`)
+
+    store.dispatch(undo())
+
+    expect(exportContext(store.getState(), ['note-replace'], 'text/plain')).toEqual(`- note-replace
+  - =note
+    - cat`)
+  })
+
+  it('typed replacements should undo in one step for thoughts and notes', () => {
+    const original = 'The world of beautiful birds'
+
+    store.dispatch([
+      importText({
+        text: `
+        - ${original}`,
+      }),
+      editThought([original], 'The world of beautiful p'),
+      editThought(['The world of beautiful p'], 'The world of beautiful pe'),
+      editThought(['The world of beautiful pe'], 'The world of beautiful peo'),
+      editThought(['The world of beautiful peo'], 'The world of beautiful peop'),
+      editThought(['The world of beautiful peop'], 'The world of beautiful peopl'),
+      editThought(['The world of beautiful peopl'], 'The world of beautiful people'),
+      undo(),
+    ])
+
+    expect(exportContext(store.getState(), [HOME_TOKEN], 'text/plain')).toEqual(`- ${HOME_TOKEN}
+  - ${original}`)
+
+    store.dispatch([
+      importText({
+        text: `
+        - note-replace-word
+          - =note
+            - ${original}`,
+      }),
+      editThought(['note-replace-word', '=note', original], 'The world of beautiful p'),
+      editThought(['note-replace-word', '=note', 'The world of beautiful p'], 'The world of beautiful pe'),
+      editThought(['note-replace-word', '=note', 'The world of beautiful pe'], 'The world of beautiful peo'),
+      editThought(['note-replace-word', '=note', 'The world of beautiful peo'], 'The world of beautiful peop'),
+      editThought(['note-replace-word', '=note', 'The world of beautiful peop'], 'The world of beautiful peopl'),
+      editThought(['note-replace-word', '=note', 'The world of beautiful peopl'], 'The world of beautiful people'),
+      undo(),
+    ])
+
+    expect(exportContext(store.getState(), ['note-replace-word'], 'text/plain')).toEqual(`- note-replace-word
+  - =note
+    - ${original}`)
+  })
+
+  it('creating an empty note should undo without reverting the previous edit', () => {
+    store.dispatch([newThought({ value: 'note-new' }), setCursor(['note-new'])])
+
+    const undoPatchesBefore = store.getState().undoPatches.length
+
+    store.dispatch(toggleNote())
+
+    expect(store.getState().undoPatches.length).toBe(undoPatchesBefore + 1)
+    expect(exportContext(store.getState(), ['note-new'], 'text/plain')).toEqual(`- note-new
+  - =note
+    - `)
+
+    store.dispatch(undo())
+
+    expect(exportContext(store.getState(), [HOME_TOKEN], 'text/plain')).toEqual(`- ${HOME_TOKEN}
+  - note-new`)
+  })
+
+  it('note offset should survive undoing a note edit', () => {
+    const original = 'The world of birds'
+
+    store.dispatch([
+      importText({
+        text: `
+        - note-offset
+          - =note
+            - ${original}`,
+      }),
+      setCursor(['note-offset']),
+      toggleNote(),
+      setNoteOffset({ value: 10 }),
+      editThought(['note-offset', '=note', original], 'The cage of birds'),
+      undo(),
+    ])
+
+    expect(store.getState().noteFocus).toBe(true)
+    expect(store.getState().noteOffset).toBe(10)
+    expect(exportContext(store.getState(), ['note-offset'], 'text/plain')).toEqual(`- note-offset
+  - =note
+    - ${original}`)
+  })
+
   it('contiguous edit additions should should not be grouped with deletions', () => {
     store.dispatch([
       importText({

src/components/Note.tsx

@@ -4,22 +4,27 @@ import { useDispatch, useSelector } from 'react-redux'
 import { css, cx } from '../../styled-system/css'
 import { textNoteRecipe } from '../../styled-system/recipes'
 import Path from '../@types/Path'
+import SimplePath from '../@types/SimplePath'
 import { cursorDownActionCreator as cursorDown } from '../actions/cursorDown'
 import { deleteThoughtActionCreator as deleteThought } from '../actions/deleteThought'
+import { editThoughtActionCreator as editThought } from '../actions/editThought'
 import { keyboardOpenActionCreator as keyboardOpen } from '../actions/keyboardOpen'
 import { setCursorActionCreator as setCursor } from '../actions/setCursor'
 import { setDescendantActionCreator as setDescendant } from '../actions/setDescendant'
 import { setNoteFocusActionCreator as setNoteFocus } from '../actions/setNoteFocus'
+import { setNoteOffsetActionCreator as setNoteOffset } from '../actions/setNoteOffset'
 import { toggleNoteActionCreator as toggleNote } from '../actions/toggleNote'
 import { isTouch } from '../browser'
 import preventAutoscroll, { preventAutoscrollEnd } from '../device/preventAutoscroll'
 import * as selection from '../device/selection'
 import useFreshCallback from '../hooks/useFreshCallback'
+import { firstVisibleChild } from '../selectors/getChildren'
 import getThoughtById from '../selectors/getThoughtById'
 import noteValue from '../selectors/noteValue'
 import resolveNotePath from '../selectors/resolveNotePath'
 import store from '../stores/app'
 import batchEditingStore from '../stores/batchEditing'
+import appendToPath from '../util/appendToPath'
 import equalPathHead from '../util/equalPathHead'
 import head from '../util/head'
 import strip from '../util/strip'
@@ -45,6 +50,7 @@ const Note = React.memo(
     /** Gets the value of the note. Returns null if no note exists or if the context view is active. */
     const note = useSelector(state => noteValue(state, path))
     const noteOffset = useSelector(state => state.noteOffset)
+    const editableNonce = useSelector(state => state.editableNonce)
 
     /** Focus Handling with useFreshCallback. */
     const onFocus = useFreshCallback(() => {
@@ -66,7 +72,12 @@ const Note = React.memo(
       if (hasFocus && noteOffset !== null) {
         selection.set(noteRef.current!, { offset: noteOffset })
       }
-    }, [hasFocus, noteOffset])
+    }, [hasFocus, noteOffset, editableNonce])
+
+    /** Saves the note caret offset without creating an undo patch. */
+    const saveNoteOffset = useCallback(() => {
+      dispatch(setNoteOffset({ value: selection.offsetThought() ?? selection.offset() }))
+    }, [dispatch])
 
     /** Handles note keyboard shortcuts. */
     const onKeyDown = useCallback(
@@ -119,22 +130,36 @@ const Note = React.memo(
             // Strip <br> from beginning and end of text
             e.target.value.replace(/^<br>|<br>$/gi, '')
 
+        saveNoteOffset()
+
         // update the referenced thought directly if it exists
         dispatch((dispatch, getState) => {
           const state = getState()
 
           const targetPath = resolveNotePath(state, path) ?? path
+          const noteThought = firstVisibleChild(state, head(targetPath))
 
-          dispatch(
-            setDescendant({
-              path: targetPath,
-              values: [value],
-              mergePrev: batchEditingStore.getState(), // If batch editing is in progress, merge this edit with the previous one in the undo stack.
-            }),
-          )
+          if (noteThought) {
+            dispatch(
+              editThought({
+                path: appendToPath(targetPath, noteThought.id) as SimplePath,
+                oldValue: noteThought.value,
+                newValue: value,
+                mergePrev: batchEditingStore.getState(), // If batch editing is in progress, merge this edit with the previous one in the undo stack.
+              }),
+            )
+          } else {
+            dispatch(
+              setDescendant({
+                path: targetPath,
+                values: [value],
+                mergePrev: batchEditingStore.getState(), // If batch editing is in progress, merge this edit with the previous one in the undo stack.
+              }),
+            )
+          }
         })
       },
-      [dispatch, path, justPasted],
+      [dispatch, path, justPasted, saveNoteOffset],
     )
 
     /** Set state.noteFocus if Note lost focus and did not move to another Note. Set state.keyboardOpen if keyboard is closed. */
@@ -217,6 +242,7 @@ const Note = React.memo(
           onDrop={isTouch ? (e: React.DragEvent) => e.preventDefault() : undefined}
           onKeyDown={onKeyDown}
           onChange={onChange}
+          onSelect={saveNoteOffset}
           // Text copied from a note and pasted on a thought should not bring along the note's default color and italicization. (#3779)
           onCopy={onCopy}
           onCut={onCut}

src/device/selection.ts

@@ -78,6 +78,10 @@ const isContentEditable = (node: Element | null): boolean => node?.hasAttribute(
 /** Check the node, if it exists, for an ariaLabel set to note-editable. */
 const isNoteEditable = (node: Element | null): boolean => node?.ariaLabel === 'note-editable'
 
+/** Returns true if the node is an editable thought or note root. */
+const isEditableRoot = (node: Node | null): boolean =>
+  node instanceof Element && (isContentEditable(node) || isNoteEditable(node))
+
 /** Returns true if the node is part of a note. Defaults to using the active selection. */
 export const isNote = (node?: EventTarget | null): boolean => {
   const editable = node === undefined ? document.activeElement : getEditableCandidate(node)
@@ -194,7 +198,7 @@ export const offsetThought = (): number | null => {
         : 0
       : selection.focusOffset
   let curNode: Node | null = selection.focusNode.nodeType === Node.TEXT_NODE ? selection.focusNode : selection.focusNode
-  while (curNode && !(curNode as HTMLElement)?.hasAttribute?.('data-editable')) {
+  while (curNode && !isEditableRoot(curNode)) {
     if (curNode?.previousSibling) {
       total += curNode.previousSibling.textContent?.length || 0
       curNode = curNode.previousSibling