From 2ea6d40ad0ec9f2b5edadf18c681e9dabb1587d7 Mon Sep 17 00:00:00 2001
From: Charley DAVID <Calyhre@users.noreply.github.com>
Date: Thu, 22 Aug 2019 04:39:38 +0200
Subject: [PATCH] Fix improper selection after insertFragment (#2976)

* Fix typo & remove useless check

* No need to check the end of the selection, as this part is called after the
  `deleteExpanded` call. Selection is collapsed at this point.

* Fix insertFragment selection not place properly
---
 packages/slate/src/commands/at-range.js       |  2 +-
 packages/slate/src/commands/with-intent.js    | 59 +++++++++----------
 .../fragment-multiple-marks.js                | 41 +++++++++++++
 .../insert-fragment/fragment-nested-blocks.js |  8 +--
 .../insert-fragment/fragment-single-inline.js | 41 +++++++++++++
 .../insert-fragment/fragment-single-mark.js   | 41 +++++++++++++
 .../middle-inline-fragment-inline.js          |  5 +-
 .../nested-block-fragment-nested-blocks.js    |  2 +-
 8 files changed, 159 insertions(+), 40 deletions(-)
 create mode 100644 packages/slate/test/commands/at-current-range/insert-fragment/fragment-multiple-marks.js
 create mode 100644 packages/slate/test/commands/at-current-range/insert-fragment/fragment-single-inline.js
 create mode 100644 packages/slate/test/commands/at-current-range/insert-fragment/fragment-single-mark.js

diff --git a/packages/slate/src/commands/at-range.js b/packages/slate/src/commands/at-range.js
index 1a13fa9a44..e26f8c032e 100644
--- a/packages/slate/src/commands/at-range.js
+++ b/packages/slate/src/commands/at-range.js
@@ -735,7 +735,7 @@ Commands.insertFragmentAtRange = (editor, range, fragment) => {
 
     // 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
+    // they will be regenerated automatically and we won't have an easy way to
     // reference them.
     fragment = fragment.mapDescendants(child => child.regenerateKey())
 
diff --git a/packages/slate/src/commands/with-intent.js b/packages/slate/src/commands/with-intent.js
index 911e11f77c..bcfcae2f31 100644
--- a/packages/slate/src/commands/with-intent.js
+++ b/packages/slate/src/commands/with-intent.js
@@ -252,46 +252,41 @@ Commands.insertFragment = (editor, fragment) => {
 
   let { value } = editor
   let { document, selection } = value
-  const { start, end } = selection
-  const { startText, endText, startInline } = value
-  const lastText = fragment.getLastText()
-  const lastInline = fragment.getClosestInline(lastText.key)
-  const lastBlock = fragment.getClosestBlock(lastText.key)
-  const firstChild = fragment.nodes.first()
-  const lastChild = fragment.nodes.last()
+  const { start } = selection
   const keys = Array.from(document.texts(), ([text]) => text.key)
-  const isAppending =
-    !startInline ||
-    (start.isAtStartOfNode(startText) || end.isAtStartOfNode(startText)) ||
-    (start.isAtEndOfNode(endText) || end.isAtEndOfNode(endText))
-
-  const isInserting =
-    firstChild.hasBlockChildren() || lastChild.hasBlockChildren()
 
   editor.insertFragmentAtRange(selection, fragment)
   value = editor.value
   document = value.document
 
   const newTexts = document.getTexts().filter(n => !keys.includes(n.key))
-  const newText = isAppending ? newTexts.last() : newTexts.takeLast(2).first()
-
-  if (newText && (lastInline || isInserting)) {
-    editor.moveToEndOfNode(newText)
-  } else if (newText) {
-    // The position within the last text node needs to be calculated. This is the length
-    // of all text nodes within the last block, but if the last block contains inline nodes,
-    // these have to be skipped.
-    const { nodes } = lastBlock
-    const lastIndex = nodes.findLastIndex(
-      node => node && node.object === 'inline'
-    )
-    const remainingTexts = nodes.takeLast(nodes.size - lastIndex - 1)
-    const remainingTextLength = remainingTexts.reduce(
-      (acc, val) => acc + val.text.length,
-      0
-    )
-    editor.moveToStartOfNode(newText).moveForward(remainingTextLength)
+  if (newTexts.size === 0) return
+  const fragmentLength = fragment.text.length
+
+  // Either startText is still here, or we want the first un-previously known text
+  const startText = document.getNode(start.key) || newTexts.first()
+  // We want the last un-previously known text
+  let endText = newTexts.last() || startText
+
+  if (startText === endText) {
+    editor.moveTo(endText.key, fragmentLength)
+    return
   }
+
+  // Everything will be calculated relative to the first common ancestor to optimize speed
+  const parent = document.getCommonAncestor(startText.key, endText.key)
+
+  const startOffset =
+    parent.getOffset(startText.key) +
+    (start.key === startText.key ? start.offset : 0)
+
+  // endText might not be the last un-previously known text node, so we precisely pick it by offset
+  endText = parent.getTextAtOffset(startOffset + fragmentLength - 1) || endText
+
+  editor.moveTo(
+    endText.key,
+    startOffset + fragmentLength - parent.getOffset(endText.key)
+  )
 }
 
 /**
diff --git a/packages/slate/test/commands/at-current-range/insert-fragment/fragment-multiple-marks.js b/packages/slate/test/commands/at-current-range/insert-fragment/fragment-multiple-marks.js
new file mode 100644
index 0000000000..f1679d26b1
--- /dev/null
+++ b/packages/slate/test/commands/at-current-range/insert-fragment/fragment-multiple-marks.js
@@ -0,0 +1,41 @@
+/** @jsx h */
+
+import h from '../../../helpers/h'
+
+export default function(editor) {
+  editor.insertFragment(
+    <document>
+      <paragraph>
+        <b>b</b>
+        <u>u</u>
+        <i>i</i>
+      </paragraph>
+    </document>
+  )
+}
+
+export const input = (
+  <value>
+    <document>
+      <paragraph>
+        wo<cursor />rd
+      </paragraph>
+    </document>
+  </value>
+)
+
+export const output = (
+  <value>
+    <document>
+      <paragraph>
+        wo
+        <b>b</b>
+        <u>u</u>
+        <i>
+          i<cursor />
+        </i>
+        rd
+      </paragraph>
+    </document>
+  </value>
+)
diff --git a/packages/slate/test/commands/at-current-range/insert-fragment/fragment-nested-blocks.js b/packages/slate/test/commands/at-current-range/insert-fragment/fragment-nested-blocks.js
index 872c722c1d..1c0b4b1346 100644
--- a/packages/slate/test/commands/at-current-range/insert-fragment/fragment-nested-blocks.js
+++ b/packages/slate/test/commands/at-current-range/insert-fragment/fragment-nested-blocks.js
@@ -29,11 +29,11 @@ export const output = (
       <paragraph>wo</paragraph>
       <quote>
         <quote>one</quote>
-        <quote>two</quote>
+        <quote>
+          two<cursor />
+        </quote>
       </quote>
-      <paragraph>
-        rd<cursor />
-      </paragraph>
+      <paragraph>rd</paragraph>
     </document>
   </value>
 )
diff --git a/packages/slate/test/commands/at-current-range/insert-fragment/fragment-single-inline.js b/packages/slate/test/commands/at-current-range/insert-fragment/fragment-single-inline.js
new file mode 100644
index 0000000000..6e23dd4828
--- /dev/null
+++ b/packages/slate/test/commands/at-current-range/insert-fragment/fragment-single-inline.js
@@ -0,0 +1,41 @@
+/** @jsx h */
+
+import h from '../../../helpers/h'
+
+export default function(editor) {
+  editor.insertFragment(
+    <document>
+      <paragraph>
+        <inline type="link">bar</inline>
+      </paragraph>
+    </document>
+  )
+}
+
+export const input = (
+  <value>
+    <document>
+      <code>
+        <paragraph>
+          Foo<cursor />baz
+        </paragraph>
+      </code>
+    </document>
+  </value>
+)
+
+export const output = (
+  <value>
+    <document>
+      <code>
+        <paragraph>
+          Foo
+          <inline type="link">
+            bar<cursor />
+          </inline>
+          baz
+        </paragraph>
+      </code>
+    </document>
+  </value>
+)
diff --git a/packages/slate/test/commands/at-current-range/insert-fragment/fragment-single-mark.js b/packages/slate/test/commands/at-current-range/insert-fragment/fragment-single-mark.js
new file mode 100644
index 0000000000..bb239535e1
--- /dev/null
+++ b/packages/slate/test/commands/at-current-range/insert-fragment/fragment-single-mark.js
@@ -0,0 +1,41 @@
+/** @jsx h */
+
+import h from '../../../helpers/h'
+
+export default function(editor) {
+  editor.insertFragment(
+    <document>
+      <paragraph>
+        <b>bar</b>
+      </paragraph>
+    </document>
+  )
+}
+
+export const input = (
+  <value>
+    <document>
+      <code>
+        <paragraph>
+          Foo<cursor />baz
+        </paragraph>
+      </code>
+    </document>
+  </value>
+)
+
+export const output = (
+  <value>
+    <document>
+      <code>
+        <paragraph>
+          Foo
+          <b>
+            bar<cursor />
+          </b>
+          baz
+        </paragraph>
+      </code>
+    </document>
+  </value>
+)
diff --git a/packages/slate/test/commands/at-current-range/insert-fragment/middle-inline-fragment-inline.js b/packages/slate/test/commands/at-current-range/insert-fragment/middle-inline-fragment-inline.js
index c4723fb980..629276bbc9 100644
--- a/packages/slate/test/commands/at-current-range/insert-fragment/middle-inline-fragment-inline.js
+++ b/packages/slate/test/commands/at-current-range/insert-fragment/middle-inline-fragment-inline.js
@@ -29,8 +29,9 @@ export const output = (
     <document>
       <paragraph>
         <link>wo</link>
-        <hashtag>fragment</hashtag>
-        <cursor />
+        <hashtag>
+          fragment<cursor />
+        </hashtag>
         <link>rd</link>
       </paragraph>
     </document>
diff --git a/packages/slate/test/commands/at-current-range/insert-fragment/nested-block-fragment-nested-blocks.js b/packages/slate/test/commands/at-current-range/insert-fragment/nested-block-fragment-nested-blocks.js
index 728a5794f4..da4bdb5a31 100644
--- a/packages/slate/test/commands/at-current-range/insert-fragment/nested-block-fragment-nested-blocks.js
+++ b/packages/slate/test/commands/at-current-range/insert-fragment/nested-block-fragment-nested-blocks.js
@@ -31,7 +31,7 @@ export const output = (
       <quote>
         <paragraph>woone</paragraph>
         <quote>
-          tword<cursor />
+          two<cursor />rd
         </quote>
       </quote>
     </document>