Attachment interaction bugfixes

src/editor/contents.js

@@ -54,6 +54,12 @@ export default class Contents {
     this.#insertLineBelowIfLastNode(node)
   }
 
+  insertAtCursorEnsuringLinesAround(node) {
+    this.insertAtCursor(node)
+    this.#insertLineAboveIfFirstNode(node)
+    this.#insertLineBelowIfLastNode(node)
+  }
+
   insertNodeWrappingEachSelectedLine(newNodeFn) {
     this.editor.update(() => {
       const selection = $getSelection()
@@ -270,7 +276,7 @@ export default class Contents {
 
     this.editor.update(() => {
       const uploadedImageNode = new ActionTextAttachmentUploadNode({ file: file, uploadUrl: uploadUrl, blobUrlTemplate: blobUrlTemplate, editor: this.editor })
-      this.insertAtCursor(uploadedImageNode)
+      this.insertAtCursorEnsuringLinesAround(uploadedImageNode)
     }, { tag: HISTORY_MERGE_TAG })
   }
 
@@ -282,6 +288,12 @@ export default class Contents {
         const nodesToRemove = this.#selection.current.getNodes()
         if (nodesToRemove.length === 0) return
 
+        // Remove a trailing empty paragraph if it exists
+        const followingNode = nodesToRemove[nodesToRemove.length - 1].getNextSibling()
+        if ($isParagraphNode(followingNode) && this.#isElementEmpty(followingNode)) {
+          followingNode.remove()
+        }
+
         focusNode = this.#findAdjacentNodeTo(nodesToRemove)
         this.#deleteNodes(nodesToRemove)
       }
@@ -349,6 +361,17 @@ export default class Contents {
     })
   }
 
+  #insertLineAboveIfFirstNode(node) {
+    this.editor.update(() => {
+      const previousSibling = node.getPreviousSibling()
+      if (!previousSibling) {
+        const newParagraph = $createParagraphNode()
+        node.insertBefore(newParagraph)
+        newParagraph.selectStart()
+      }
+    })
+  }
+
   #unwrap(node) {
     const children = node.getChildren()
 

src/editor/selection.js

@@ -27,8 +27,10 @@ export default class Selection {
 
   set current(selection) {
     if ($isNodeSelection(selection)) {
-      this._current = $getSelection()
-      this.#syncSelectedClasses()
+      this.editor.getEditorState().read(() => {
+        this._current = $getSelection()
+        this.#syncSelectedClasses()
+      })
     } else {
       this.editor.update(() => {
         this.#syncSelectedClasses()
@@ -212,8 +214,9 @@ export default class Selection {
 
     this._currentlySelectedKeys = new Set()
 
-    if (this.current) {
-      for (const node of this.current.getNodes()) {
+    const selection = $getSelection()
+    if (selection && $isNodeSelection(selection)) {
+      for (const node of selection.getNodes()) {
         this._currentlySelectedKeys.add(node.getKey())
       }
     }
@@ -389,16 +392,84 @@ export default class Selection {
     if (this.current) {
       await this.#withCurrentNode((currentNode) => currentNode.selectPrevious())
     } else {
-      this.#selectInLexical(this.topLevelNodeBeforeCursor)
+      if (this.#isCursorOnFirstLineOfBlock()) {
+        this.#selectInLexical(this.topLevelNodeBeforeCursor)
+      }
     }
   }
 
   async #selectNextTopLevelNode() {
     if (this.current) {
       await this.#withCurrentNode((currentNode) => currentNode.selectNext(0, 0))
     } else {
-      this.#selectInLexical(this.topLevelNodeAfterCursor)
+      if (this.#isCursorOnLastLineOfBlock()) {
+        this.#selectInLexical(this.topLevelNodeAfterCursor)
+      }
+    }
+  }
+
+  #isCursorOnFirstLineOfBlock() {
+    const metrics = this.#getCursorBlockMetrics()
+    if (!metrics) return true
+
+    const { cursorRect, blockRect, lineHeight } = metrics
+
+    const distanceFromTop = cursorRect.top - blockRect.top
+    return distanceFromTop < lineHeight * 0.8
+  }
+
+  #isCursorOnLastLineOfBlock() {
+    const metrics = this.#getCursorBlockMetrics()
+    if (!metrics) return true
+
+    const { cursorRect, blockRect, lineHeight } = metrics
+
+    const distanceFromBottom = blockRect.bottom - cursorRect.bottom
+    return distanceFromBottom < lineHeight * 0.8
+  }
+
+  #getCursorBlockMetrics() {
+    const nativeSelection = window.getSelection()
+    if (!nativeSelection || nativeSelection.rangeCount === 0) {
+      return null
     }
+
+    const range = nativeSelection.getRangeAt(0)
+    const cursorRect = range.getBoundingClientRect()
+
+    let blockElement = null
+    this.editor.getEditorState().read(() => {
+      const selection = $getSelection()
+      if (!$isRangeSelection(selection)) {
+        return
+      }
+
+      const anchorNode = selection.anchor.getNode()
+      const topLevelElement = anchorNode.getTopLevelElement()
+      if (topLevelElement) {
+        blockElement = this.editor.getElementByKey(topLevelElement.getKey())
+      }
+    })
+
+    if (!blockElement) {
+      return null
+    }
+
+    const blockRect = blockElement.getBoundingClientRect()
+    const lineHeight = this.#getLineHeight(blockElement)
+
+    return { cursorRect, blockRect, lineHeight }
+  }
+
+  #getLineHeight(element) {
+    const computed = window.getComputedStyle(element)
+    const lineHeight = computed.lineHeight
+
+    if (lineHeight === "normal") {
+      return parseFloat(computed.fontSize)
+    }
+
+    return parseFloat(lineHeight)
   }
 
   async #withCurrentNode(fn) {
@@ -601,8 +672,27 @@ export default class Selection {
     if (anchorNode.getNextSibling() instanceof DecoratorNode) {
       return anchorNode.getNextSibling()
     }
-    const parent = anchorNode.getParent()
-    return parent ? parent.getNextSibling() : null
+
+    // Walk up the tree to find the first ancestor with a next sibling
+    let current = anchorNode
+    while (current) {
+      const nextSibling = current.getNextSibling()
+      if (nextSibling) {
+        // If it's a DecoratorNode, return it
+        if (nextSibling instanceof DecoratorNode) {
+          return nextSibling
+        }
+        // Otherwise, try to find a DecoratorNode in its descendants
+        return this.#findFirstDecoratorDescendant(nextSibling)
+      }
+      current = current.getParent()
+      // Stop if we've reached the root
+      if (!current || current === $getRoot()) {
+        break
+      }
+    }
+
+    return null
   }
 
   #getNodeAfterElementNode(anchorNode, offset) {
@@ -623,8 +713,27 @@ export default class Selection {
     if (anchorNode.getPreviousSibling() instanceof DecoratorNode) {
       return anchorNode.getPreviousSibling()
     }
-    const parent = anchorNode.getParent()
-    return parent.getPreviousSibling()
+
+    // Walk up the tree to find the first ancestor with a previous sibling
+    let current = anchorNode
+    while (current) {
+      const prevSibling = current.getPreviousSibling()
+      if (prevSibling) {
+        // If it's a DecoratorNode, return it
+        if (prevSibling instanceof DecoratorNode) {
+          return prevSibling
+        }
+        // Otherwise, try to find a DecoratorNode in its descendants
+        return this.#findLastDecoratorDescendant(prevSibling)
+      }
+      current = current.getParent()
+      // Stop if we've reached the root
+      if (!current || current === $getRoot()) {
+        break
+      }
+    }
+
+    return null
   }
 
   #getNodeBeforeElementNode(anchorNode, offset) {
@@ -649,4 +758,36 @@ export default class Selection {
     }
     return current ? current.getPreviousSibling() : null
   }
+
+  #findFirstDecoratorDescendant(node) {
+    if (node instanceof DecoratorNode) {
+      return node
+    }
+
+    if ($isElementNode(node)) {
+      const children = node.getChildren()
+      for (const child of children) {
+        const result = this.#findFirstDecoratorDescendant(child)
+        if (result) return result
+      }
+    }
+
+    return null
+  }
+
+  #findLastDecoratorDescendant(node) {
+    if (node instanceof DecoratorNode) {
+      return node
+    }
+
+    if ($isElementNode(node)) {
+      const children = node.getChildren()
+      for (let i = children.length - 1; i >= 0; i--) {
+        const result = this.#findLastDecoratorDescendant(children[i])
+        if (result) return result
+      }
+    }
+
+    return null
+  }
 }