fix inserting fragments at starting edge of blocks, closes #425

src/transforms/at-current-range.js

@@ -157,28 +157,31 @@ export function insertFragment(transform, fragment) {
 
   if (!fragment.length) return transform
 
+  const { startText, endText } = state
   const lastText = fragment.getLastText()
   const lastInline = fragment.getClosestInline(lastText.key)
-  const beforeTexts = document.getTexts()
-  const appending = selection.hasEdgeAtEndOf(document.getDescendant(selection.endKey))
+  const keys = document.getTexts().map(text => text.key)
+  const isAppending = (
+    selection.hasEdgeAtEndOf(endText) ||
+    selection.hasEdgeAtStartOf(startText)
+  )
 
   transform.unsetSelection()
   transform.insertFragmentAtRange(selection, fragment)
   state = transform.state
   document = state.document
 
-  const keys = beforeTexts.map(text => text.key)
-  const news = document.getTexts().filter(n => !keys.includes(n.key))
-  const text = appending ? news.last() : news.takeLast(2).first()
+  const newTexts = document.getTexts().filter(n => !keys.includes(n.key))
+  const newText = isAppending ? newTexts.last() : newTexts.takeLast(2).first()
   let after
 
-  if (text && lastInline) {
-    after = selection.collapseToEndOf(text)
+  if (newText && lastInline) {
+    after = selection.collapseToEndOf(newText)
   }
 
-  else if (text) {
+  else if (newText) {
     after = selection
-      .collapseToStartOf(text)
+      .collapseToStartOf(newText)
       .moveForward(lastText.length)
   }
 

src/transforms/at-range.js

@@ -313,23 +313,31 @@ export function insertBlockAtRange(transform, range, block, options = {}) {
 export function insertFragmentAtRange(transform, range, fragment, options = {}) {
   const { normalize = true } = options
 
+  // If the range is expanded, delete it first.
   if (range.isExpanded) {
     transform.deleteAtRange(range, { normalize: false })
     range = range.collapseToStart()
   }
 
+  // If the fragment is empty, there's nothing to do after deleting.
   if (!fragment.length) {
     return transform
   }
 
+  // Regenerate the keys for all of the fragments nodes, so that they're
+  // guaranteed not to collide with the existing keys in the document. Otherwise
+  // they will be rengerated automatically and we won't have an easy way to
+  // reference them.
   fragment = fragment.mapDescendants(child => child.regenerateKey())
 
+  // Calculate a few things...
   const { startKey, startOffset } = range
   let { state } = transform
   let { document } = state
   let startText = document.getDescendant(startKey)
   let startBlock = document.getClosestBlock(startText.key)
   let startChild = startBlock.getHighestChild(startText.key)
+  const isAtStart = range.isAtStartOf(startBlock)
   const parent = document.getParent(startBlock.key)
   const index = parent.nodes.indexOf(startBlock)
   const offset = startChild == startText
@@ -340,6 +348,8 @@ export function insertFragmentAtRange(transform, range, fragment, options = {})
   const firstBlock = blocks.first()
   const lastBlock = blocks.last()
 
+  // If the first and last block aren't the same, we need to insert all of the
+  // nodes after the fragment's first block at the index.
   if (firstBlock != lastBlock) {
     const lonelyParent = fragment.getFurthest(firstBlock.key, p => p.nodes.size == 1)
     const lonelyChild = lonelyParent || firstBlock
@@ -352,18 +362,23 @@ export function insertFragmentAtRange(transform, range, fragment, options = {})
     })
   }
 
+  // Check if we need to split the node.
   if (startOffset != 0) {
     transform.splitNodeByKey(startChild.key, offset, { normalize: false })
   }
 
+  // Update our variables with the new state.
   state = transform.state
   document = state.document
   startText = document.getDescendant(startKey)
   startBlock = document.getClosestBlock(startKey)
   startChild = startBlock.getHighestChild(startText.key)
 
+  // If the first and last block aren't the same, we need to move any of the
+  // starting block's children after the split into the last block of the
+  // fragment, which has already been inserted.
   if (firstBlock != lastBlock) {
-    const nextChild = startBlock.getNextSibling(startChild.key)
+    const nextChild = isAtStart ? startChild : startBlock.getNextSibling(startChild.key)
     const nextNodes = nextChild ? startBlock.nodes.skipUntil(n => n.key == nextChild.key) : List()
     const lastIndex = lastBlock.nodes.size
 
@@ -373,10 +388,16 @@ export function insertFragmentAtRange(transform, range, fragment, options = {})
     })
   }
 
+  // If the starting block is empty, we replace it entirely with the first block
+  // of the fragment, since this leads to a more expected behavior for the user.
   if (startBlock.isEmpty) {
     transform.removeNodeByKey(startBlock.key, { normalize: false })
     transform.insertNodeByKey(parent.key, index, firstBlock, { normalize: false })
-  } else {
+  }
+
+  // Otherwise, we maintain the starting block, and insert all of the first
+  // block's inline nodes into it at the split point.
+  else {
     const inlineChild = startBlock.getHighestChild(startText.key)
     const inlineIndex = startBlock.nodes.indexOf(inlineChild)
 
@@ -387,6 +408,7 @@ export function insertFragmentAtRange(transform, range, fragment, options = {})
     })
   }
 
+  // Normalize if requested.
   if (normalize) {
     transform.normalizeNodeByKey(parent.key, SCHEMA)
   }

test/transforms/fixtures/at-current-range/insert-fragment/start-block-multiple-blocks/fragment.yaml

@@ -0,0 +1,17 @@
+
+nodes:
+  - kind: block
+    type: list-item
+    nodes:
+      - kind: text
+        text: fragment
+  - kind: block
+    type: list-item
+    nodes:
+      - kind: text
+        text: second fragment
+  - kind: block
+    type: list-item
+    nodes:
+      - kind: text
+        text: third fragment

test/transforms/fixtures/at-current-range/insert-fragment/start-block-multiple-blocks/index.js

@@ -0,0 +1,41 @@
+
+import assert from 'assert'
+import path from 'path'
+import readMetadata from 'read-metadata'
+import { Raw } from '../../../../../..'
+
+export default function (state) {
+  const file = path.resolve(__dirname, 'fragment.yaml')
+  const raw = readMetadata.sync(file)
+  const fragment = Raw.deserialize(raw, { terse: true }).document
+
+  const { document, selection } = state
+  const texts = document.getTexts()
+  const first = texts.first()
+  const range = selection.merge({
+    anchorKey: first.key,
+    anchorOffset: 0,
+    focusKey: first.key,
+    focusOffset: 0
+  })
+
+  const next = state
+    .transform()
+    .moveTo(range)
+    .insertFragment(fragment)
+    .apply()
+
+  const last = next.document.getTexts().last()
+
+  assert.deepEqual(
+    next.selection.toJS(),
+    range.merge({
+      anchorKey: last.key,
+      anchorOffset: fragment.getTexts().last().length,
+      focusKey: last.key,
+      focusOffset: fragment.getTexts().last().length
+    }).toJS()
+  )
+
+  return next
+}

test/transforms/fixtures/at-current-range/insert-fragment/start-block-multiple-blocks/input.yaml

@@ -0,0 +1,7 @@
+
+nodes:
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: word

test/transforms/fixtures/at-current-range/insert-fragment/start-block-multiple-blocks/output.yaml

@@ -0,0 +1,17 @@
+
+nodes:
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: fragment
+  - kind: block
+    type: list-item
+    nodes:
+      - kind: text
+        text: second fragment
+  - kind: block
+    type: list-item
+    nodes:
+      - kind: text
+        text: third fragmentword

test/transforms/fixtures/at-range/insert-fragment-at-range/start-block-multiple-blocks/fragment.yaml

@@ -0,0 +1,12 @@
+
+nodes:
+  - kind: block
+    type: list-item
+    nodes:
+      - kind: text
+        text: one
+  - kind: block
+    type: list-item
+    nodes:
+      - kind: text
+        text: two

test/transforms/fixtures/at-range/insert-fragment-at-range/start-block-multiple-blocks/index.js

@@ -0,0 +1,25 @@
+
+import path from 'path'
+import readMetadata from 'read-metadata'
+import { Raw } from '../../../../../..'
+
+export default function (state) {
+  const file = path.resolve(__dirname, 'fragment.yaml')
+  const raw = readMetadata.sync(file)
+  const fragment = Raw.deserialize(raw, { terse: true }).document
+
+  const { document, selection } = state
+  const texts = document.getTexts()
+  const first = texts.first()
+  const range = selection.merge({
+    anchorKey: first.key,
+    anchorOffset: 0,
+    focusKey: first.key,
+    focusOffset: 0
+  })
+
+  return state
+    .transform()
+    .insertFragmentAtRange(range, fragment)
+    .apply()
+}

test/transforms/fixtures/at-range/insert-fragment-at-range/start-block-multiple-blocks/input.yaml

@@ -0,0 +1,7 @@
+
+nodes:
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: word

test/transforms/fixtures/at-range/insert-fragment-at-range/start-block-multiple-blocks/output.yaml

@@ -0,0 +1,12 @@
+
+nodes:
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: one
+  - kind: block
+    type: list-item
+    nodes:
+      - kind: text
+        text: twoword