History merging

src/editor/selection.js

@@ -1,7 +1,7 @@
 import {
-  $createParagraphNode, $getNearestNodeFromDOMNode, $getRoot, $getSelection, $isDecoratorNode, $isElementNode,
+  $addUpdateTag, $createParagraphNode, $getNearestNodeFromDOMNode, $getRoot, $getSelection, $isDecoratorNode, $isElementNode,
   $isLineBreakNode, $isNodeSelection, $isRangeSelection, $isTextNode, $setSelection, CLICK_COMMAND, COMMAND_PRIORITY_LOW, DELETE_CHARACTER_COMMAND,
-  KEY_ARROW_DOWN_COMMAND, KEY_ARROW_LEFT_COMMAND, KEY_ARROW_RIGHT_COMMAND, KEY_ARROW_UP_COMMAND, SELECTION_CHANGE_COMMAND, isDOMNode
+  HISTORY_MERGE_TAG, KEY_ARROW_DOWN_COMMAND, KEY_ARROW_LEFT_COMMAND, KEY_ARROW_RIGHT_COMMAND, KEY_ARROW_UP_COMMAND, SELECTION_CHANGE_COMMAND, isDOMNode
 } from "lexical"
 import { $getNearestNodeOfType } from "@lexical/utils"
 import { $getListDepth, ListItemNode, ListNode } from "@lexical/list"
@@ -32,7 +32,7 @@ export default class Selection {
   }
 
   set current(selection) {
-    this.editor.update(() => {
+    this.editor.getEditorState().read(() => {
       this.#syncSelectedClasses()
     })
   }
@@ -554,6 +554,7 @@ export default class Selection {
 
   #selectInLexical(node) {
     if ($isDecoratorNode(node)) {
+      $addUpdateTag(HISTORY_MERGE_TAG)
       const selection = $createNodeSelectionWith(node)
       $setSelection(selection)
       return selection

src/extensions/provisional_paragraph_extension.js

@@ -1,4 +1,4 @@
-import { $getRoot, COMMAND_PRIORITY_HIGH, RootNode, SELECTION_CHANGE_COMMAND, defineExtension } from "lexical"
+import { $addUpdateTag, $getRoot, COMMAND_PRIORITY_HIGH, HISTORY_MERGE_TAG, RootNode, SELECTION_CHANGE_COMMAND, defineExtension } from "lexical"
 import { $descendantsMatching, $firstToLastIterator, $insertFirst, mergeRegister } from "@lexical/utils"
 import { $isProvisionalParagraphNode, ProvisionalParagraphNode } from "../nodes/provisional_paragraph_node"
 import LexxyExtension from "./lexxy_extension"
@@ -45,6 +45,9 @@ function $removeUnneededProvisionalParagraphs(rootNode) {
 }
 
 function $markAllProvisionalParagraphsDirty() {
+  // Selection-driven visibility updates must not become standalone undo steps.
+  $addUpdateTag(HISTORY_MERGE_TAG)
+
   for (const provisionalParagraph of $getAllProvisionalParagraphs()) {
     provisionalParagraph.markDirty()
   }

src/nodes/action_text_attachment_upload_node.js

@@ -1,13 +1,17 @@
-import { $getSelection, $isRangeSelection, $isRootOrShadowRoot, SKIP_DOM_SELECTION_TAG } from "lexical"
+import {
+  $createParagraphNode, $getNodeByKey, $getRoot, $getSelection, $isRangeSelection, $isRootOrShadowRoot, HISTORIC_TAG, HISTORY_PUSH_TAG, SKIP_DOM_SELECTION_TAG,
+  SKIP_SCROLL_INTO_VIEW_TAG
+} from "lexical"
 import Lexxy from "../config/lexxy"
-import { SILENT_UPDATE_TAGS } from "../helpers/lexical_helper"
 import { ActionTextAttachmentNode } from "./action_text_attachment_node"
 import { $isProvisionalParagraphNode } from "./provisional_paragraph_node"
 import { createElement, dispatch } from "../helpers/html_helper"
 import { loadFileIntoImage } from "../helpers/upload_helper"
 import { bytesToHumanSize } from "../helpers/storage_helper"
 
 export class ActionTextAttachmentUploadNode extends ActionTextAttachmentNode {
+  static #activeUploads = new WeakSet()
+
   static getType() {
     return "action_text_attachment_upload"
   }
@@ -135,7 +139,7 @@ export class ActionTextAttachmentUploadNode extends ActionTextAttachmentNode {
       const writable = this.getWritable()
       writable.width = width
       writable.height = height
-    }, { tag: this.#backgroundUpdateTags })
+    }, { tag: this.#transientUpdateTags })
   }
 
   get #hasDimensions() {
@@ -145,7 +149,10 @@ export class ActionTextAttachmentUploadNode extends ActionTextAttachmentNode {
   async #startUploadIfNeeded() {
     if (this.#uploadStarted) return
     if (!this.uploadUrl) return // Bridge-managed upload — skip DirectUpload
+    if (ActionTextAttachmentUploadNode.#activeUploads.has(this.file)) return
 
+    ActionTextAttachmentUploadNode.#activeUploads.add(this.file)
+    const uploadNodeKey = this.getKey()
     this.#setUploadStarted()
 
     const { DirectUpload } = await import("@rails/activestorage")
@@ -162,12 +169,79 @@ export class ActionTextAttachmentUploadNode extends ActionTextAttachmentNode {
       } else {
         this.#dispatchEvent("lexxy:upload-end", { file: this.file, error: null })
         this.editor.update(() => {
-          this.showUploadedAttachment(blob)
-        }, { tag: this.#backgroundUpdateTags })
+          const uploadNode = $getNodeByKey(uploadNodeKey)
+          if (!(uploadNode instanceof ActionTextAttachmentUploadNode)) return
+
+          uploadNode.showUploadedAttachment(blob)
+        }, {
+          tag: this.#backgroundUpdateTags,
+          onUpdate: () => requestAnimationFrame(() => this.#collapseUploadHistory(uploadNodeKey))
+        })
       }
     })
   }
 
+  // Upload completion uses HISTORY_PUSH_TAG so Lexical creates the undo boundary.
+  // Then we rewrite any upload-node snapshots to their clean (node-stripped) form
+  // and collapse duplicates to avoid no-op undo steps.
+  #collapseUploadHistory(uploadNodeKey) {
+    const historyState = this.#historyState
+    if (!historyState) return
+
+    const currentSignature = historyState.current && this.#contentSignatureFor(historyState.current.editorState)
+    const rewrittenStack = []
+    let prevSignature = null
+
+    for (const entry of historyState.undoStack) {
+      const hasUploadNode = entry.editorState.read(() =>
+        $getNodeByKey(uploadNodeKey) instanceof ActionTextAttachmentUploadNode
+      )
+
+      const editorState = hasUploadNode
+        ? this.#editorStateStrippingUploadNodes(entry.editorState)
+        : entry.editorState
+
+      const signature = this.#contentSignatureFor(editorState)
+      if (signature !== prevSignature && signature !== currentSignature) {
+        rewrittenStack.push(hasUploadNode ? { editor: entry.editor, editorState } : entry)
+      }
+      prevSignature = signature
+    }
+
+    historyState.undoStack = rewrittenStack
+    historyState.redoStack = []
+
+    // Force listeners (toolbar undo/redo button states) to observe the stack rewrite.
+    this.editor.update(() => {}, { tag: HISTORIC_TAG })
+  }
+
+  // Upload nodes can't survive a JSON round-trip (File isn't serializable),
+  // so strip them from the serialized state before parseEditorState calls importJSON.
+  #editorStateStrippingUploadNodes(editorState) {
+    const json = editorState.toJSON()
+    this.#stripNodesFromJSON(json.root, n => n.type === "action_text_attachment_upload")
+    return this.editor.parseEditorState(json, () => {
+      if ($getRoot().getChildrenSize() === 0) $getRoot().append($createParagraphNode())
+    })
+  }
+
+  #stripNodesFromJSON(node, predicate) {
+    if (!node.children) return
+    node.children = node.children.filter(child => {
+      if (predicate(child)) return false
+      this.#stripNodesFromJSON(child, predicate)
+      return true
+    })
+  }
+
+  #contentSignatureFor(editorState) {
+    return JSON.stringify(editorState.toJSON().root)
+  }
+
+  get #historyState() {
+    return this.editor.getRootElement()?.closest("lexxy-editor")?.historyState
+  }
+
   #createUploadDelegate() {
     const shouldAuthenticateUploads = Lexxy.global.get("authenticatedUploads")
 
@@ -197,14 +271,14 @@ export class ActionTextAttachmentUploadNode extends ActionTextAttachmentNode {
   #setProgress(progress) {
     this.editor.update(() => {
       this.getWritable().progress = progress
-    }, { tag: this.#backgroundUpdateTags })
+    }, { tag: this.#transientUpdateTags })
   }
 
   #handleUploadError(error) {
     console.warn(`Upload error for ${this.file?.name ?? "file"}: ${error}`)
     this.editor.update(() => {
       this.getWritable().uploadError = true
-    }, { tag: this.#backgroundUpdateTags })
+    }, { tag: this.#transientUpdateTags })
   }
 
   showUploadedAttachment(blob) {
@@ -221,15 +295,24 @@ export class ActionTextAttachmentUploadNode extends ActionTextAttachmentNode {
     return replacementNode.getKey()
   }
 
-  // Upload lifecycle methods (progress, completion, errors) run asynchronously and may
-  // fire while the user is focused on another element (e.g., a title field). Without
-  // SKIP_DOM_SELECTION_TAG, Lexical's reconciler would move the DOM selection back into
-  // the editor, stealing focus from wherever the user is currently typing.
+  // Transient updates (progress, dimensions, errors) are completely invisible to
+  // the undo history via HISTORIC_TAG.
+  get #transientUpdateTags() {
+    if (this.#editorHasFocus) {
+      return [ HISTORIC_TAG ]
+    } else {
+      return [ HISTORIC_TAG, SKIP_DOM_SELECTION_TAG ]
+    }
+  }
+
+  // Use HISTORY_PUSH_TAG to force a stable undo boundary at upload completion.
+  // SKIP_SCROLL_INTO_VIEW_TAG avoids scroll jumps, and SKIP_DOM_SELECTION_TAG
+  // prevents focus theft when the editor is not active.
   get #backgroundUpdateTags() {
     if (this.#editorHasFocus) {
-      return SILENT_UPDATE_TAGS
+      return [ HISTORY_PUSH_TAG, SKIP_SCROLL_INTO_VIEW_TAG ]
     } else {
-      return [ ...SILENT_UPDATE_TAGS, SKIP_DOM_SELECTION_TAG ]
+      return [ HISTORY_PUSH_TAG, SKIP_SCROLL_INTO_VIEW_TAG, SKIP_DOM_SELECTION_TAG ]
     }
   }
 

test/browser/helpers/active_storage_mock.js

@@ -7,7 +7,7 @@ const TRANSPARENT_PNG = Buffer.from(
   "base64"
 )
 
-export async function mockActiveStorageUploads(page, { delayBlobResponses = false, delayDirectUploadResponse = false } = {}) {
+export async function mockActiveStorageUploads(page, { delayBlobResponses = false, delayDirectUploadResponse = false, uploadDelayMs = 0 } = {}) {
   let blobCounter = 0
   const calls = { blobCreations: [], fileUploads: [] }
   const pendingBlobRoutes = []
@@ -115,6 +115,10 @@ export async function mockActiveStorageUploads(page, { delayBlobResponses = fals
       contentType: request.headers()["content-type"],
     })
 
+    if (uploadDelayMs > 0) {
+      await page.waitForTimeout(uploadDelayMs)
+    }
+
     await route.fulfill({ status: 204 })
   })
 

test/browser/tests/attachments/attachment_drag_and_drop.test.js

@@ -462,6 +462,9 @@ async function simulateDragByIndex(page, sourceIndex, targetIndex, position) {
 }
 
 async function dragLocatorToLocator(page, sourceLocator, targetLocator, position = "onto") {
+  await sourceLocator.scrollIntoViewIfNeeded()
+  await targetLocator.scrollIntoViewIfNeeded()
+
   const sourceHandle = await sourceLocator.elementHandle()
   const targetHandle = await targetLocator.elementHandle()
 

test/browser/tests/attachments/attachments.test.js

@@ -251,6 +251,109 @@ test.describe("Attachments", () => {
     await expect.poll(() => editor.plainTextValue()).toContain("hello world")
   })
 
+  test("undo after uploading into empty editor restores empty state", async ({ page, editor }) => {
+    await mockActiveStorageUploads(page)
+    await editor.uploadFile("test/fixtures/files/example.png")
+
+    const figure = page.locator("figure.attachment")
+    await expect(figure).toBeVisible({ timeout: 10_000 })
+    await editor.flush()
+
+    // Wait for the history collapse to complete (runs in requestAnimationFrame)
+    await page.evaluate(() => new Promise(resolve => requestAnimationFrame(resolve)))
+
+    // Undo until the undo button is disabled — no stale upload node should remain
+    const undoButton = page.getByRole("button", { name: "Undo" })
+    while (await undoButton.evaluate((el) => !el.disabled)) {
+      await undoButton.click()
+      await editor.flush()
+    }
+
+    await expect(figure).toHaveCount(0)
+    await expect(editor.content.locator("progress")).toHaveCount(0)
+  })
+
+  test("undo preserves edits made during upload", async ({ page, editor }) => {
+    await mockActiveStorageUploads(page, { uploadDelayMs: 1_000 })
+
+    await editor.uploadFile("test/fixtures/files/example.png")
+
+    const uploadProgress = page.locator("figure.attachment progress")
+    await expect(uploadProgress).toBeVisible({ timeout: 10_000 })
+
+    // Type while the upload node is still in-flight.
+    await editor.send("hello world")
+    await expect(uploadProgress).toBeVisible()
+
+    const figure = page.locator("figure.attachment")
+    await expect(figure.locator("img")).toHaveAttribute(
+      "src",
+      /\/rails\/active_storage\/blobs\/mock-signed-id-\d+\/example\.png/,
+      { timeout: 10_000 },
+    )
+    await editor.flush()
+
+    // Wait for the history collapse to complete
+    await page.evaluate(() => new Promise(resolve => requestAnimationFrame(resolve)))
+
+    // Undo should remove the attachment but preserve the typed text
+    const undoButton = page.getByRole("button", { name: "Undo" })
+    await undoButton.click()
+    await editor.flush()
+
+    await expect(figure).toHaveCount(0)
+    await expect(editor.content.locator("progress")).toHaveCount(0)
+    await expect(editor.content).toContainText("hello world")
+  })
+
+  test("node selection does not create an extra undo step", async ({ page, editor }) => {
+    await mockActiveStorageUploads(page)
+    await editor.send("hello")
+    await editor.uploadFile("test/fixtures/files/example.png")
+
+    const figure = page.locator("figure.attachment[data-content-type='image/png']")
+    await expect(figure).toBeVisible({ timeout: 10_000 })
+    await editor.flush()
+    await page.evaluate(() => new Promise(resolve => requestAnimationFrame(resolve)))
+
+    // Click to create a node selection, then undo should still remove the attachment.
+    await figure.locator("img").click()
+    await editor.flush()
+
+    await page.getByRole("button", { name: "Undo" }).click()
+    await editor.flush()
+
+    await expect(figure).toHaveCount(0)
+    await expect(editor.content).toContainText("hello")
+  })
+
+  test("upload completion clears redo after undo during upload", async ({ page, editor }) => {
+    await mockActiveStorageUploads(page, { uploadDelayMs: 500 })
+    await editor.send("hello")
+    await editor.uploadFile("test/fixtures/files/example.png")
+
+    const undoButton = page.getByRole("button", { name: "Undo" })
+    const redoButton = page.getByRole("button", { name: "Redo" })
+    await expect(page.locator("figure.attachment progress")).toBeVisible({ timeout: 10_000 })
+
+    await undoButton.click()
+    await editor.flush()
+    await expect(redoButton).toBeEnabled()
+    await expect.poll(() => page.evaluate(() => {
+      return document.querySelector("lexxy-editor").historyState.redoStack.length
+    })).toBeGreaterThan(0)
+
+    await expect.poll(() => page.evaluate(() => {
+      return document.querySelector("lexxy-editor").historyState.redoStack.length
+    }), { timeout: 5_000 }).toBe(0)
+
+    // Redo should be a no-op once completion collapses and clears redo history.
+    await redoButton.click()
+    await editor.flush()
+    await expect(page.locator("figure.attachment")).toHaveCount(0)
+    await expect(editor.content).toContainText("hello")
+  })
+
   test("Ctrl+C in caption copies text without losing focus", async ({ page, editor }) => {
     await mockActiveStorageUploads(page)
     await editor.uploadFile("test/fixtures/files/example.png")