From 697a7710147c2da231a301f3c3f6cad6bd301622 Mon Sep 17 00:00:00 2001 From: Per-Kristian Nordnes Date: Thu, 7 Sep 2017 18:15:25 +0200 Subject: [PATCH] Fix bugs in deleteAtRange (#1072) * Fix bug in deleteAtRange when selecting part of non-void block followed by void block * Fix bug in deleteAtRange when selecting from an inline void node * Fix bug with deleteAtRange if a whole startBlock was selected, fixes #1021 --- src/changes/at-range.js | 37 ++++++++++++++---- .../index.js | 37 ++++++++++++++++++ .../input.yaml | 15 ++++++++ .../output.yaml | 15 ++++++++ .../index.js | 37 ++++++++++++++++++ .../input.yaml | 15 ++++++++ .../output.yaml | 12 ++++++ .../index.js | 36 ++++++++++++++++++ .../input.yaml | 15 ++++++++ .../output.yaml | 15 ++++++++ .../index.js | 37 ++++++++++++++++++ .../input.yaml | 20 ++++++++++ .../output.yaml | 7 ++++ .../index.js | 38 +++++++++++++++++++ .../input.yaml | 12 ++++++ .../output.yaml | 7 ++++ 16 files changed, 347 insertions(+), 8 deletions(-) create mode 100644 test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-up-to-start-of-void/index.js create mode 100644 test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-up-to-start-of-void/input.yaml create mode 100644 test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-up-to-start-of-void/output.yaml create mode 100644 test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-void/index.js create mode 100644 test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-void/input.yaml create mode 100644 test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-void/output.yaml create mode 100644 test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void/index.js create mode 100644 test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void/input.yaml create mode 100644 test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void/output.yaml create mode 100644 test/changes/fixtures/at-current-range/delete/void-inline-as-first-with-non-void-block-next/index.js create mode 100644 test/changes/fixtures/at-current-range/delete/void-inline-as-first-with-non-void-block-next/input.yaml create mode 100644 test/changes/fixtures/at-current-range/delete/void-inline-as-first-with-non-void-block-next/output.yaml create mode 100644 test/changes/fixtures/at-current-range/delete/whole-block-with-other-block-type-below/index.js create mode 100644 test/changes/fixtures/at-current-range/delete/whole-block-with-other-block-type-below/input.yaml create mode 100644 test/changes/fixtures/at-current-range/delete/whole-block-with-other-block-type-below/output.yaml diff --git a/src/changes/at-range.js b/src/changes/at-range.js index 2969b2646..52a1f30db 100644 --- a/src/changes/at-range.js +++ b/src/changes/at-range.js @@ -135,6 +135,16 @@ Changes.deleteAtRange = (change, range, options = {}) => { } } + // If the selection starts at an inline void, remove that void inline first + const startInline = document.getClosestInline(startKey) + if (startInline && startInline.isVoid && + startInline.getTexts().first().key == startKey) { + const nextText = document.getNextText(startInline.getTexts().first().key) + change.removeNodeByKey(startInline.key, OPTS) + startKey = nextText.key + startOffset = 0 + } + // If the start and end key are the same, we can just remove it. if (startKey == endKey) { // If it is a void node, remove the whole node @@ -178,10 +188,12 @@ Changes.deleteAtRange = (change, range, options = {}) => { ancestor = document.getCommonAncestor(startKey, endKey) startChild = ancestor.getFurthestAncestor(startKey) endChild = ancestor.getFurthestAncestor(endKey) + + + const nextText = document.getNextText(endKey) const startIndex = ancestor.nodes.indexOf(startChild) const endIndex = ancestor.nodes.indexOf(endChild) const middles = ancestor.nodes.slice(startIndex + 1, endIndex + 1) - const next = document.getNextText(endKey) // Remove all of the middle nodes, between the splits. if (middles.size) { @@ -190,16 +202,25 @@ Changes.deleteAtRange = (change, range, options = {}) => { }) } - // If the start and end block are different, move all of the nodes from the - // end block into the start block. + // Refresh variables state = change.state document = state.document - const startBlock = document.getClosestBlock(startKey) - const endBlock = document.getClosestBlock(next.key) - // If the endBlock is void, just remove the startBlock - if (endBlock.isVoid) { - change.removeNodeByKey(startBlock.key) + const startBlock = document.getClosestBlock(startKey) + const endBlock = document.getClosestBlock(nextText.key) + + // If the whole startBlock is selected but the endBlock is different, just remove the startBlock + if (startBlock.key !== endBlock.key && startChild.text.length === endOffset && startOffset === 0) { + document = change.removeNodeByKey(startBlock.key, OPTS).state.document + return + } + + // If the endBlock is void, remove what is selected of the start block + if (endBlock.isVoid && endOffset === 0) { + // If part of the startBlock is selected, split it and remove the unwanted part + document = change.splitNodeByKey(startChild.key, startOffset, OPTS).state.document + const toBeRemoved = document.nodes.get(startIndex + 1) + change.removeNodeByKey(toBeRemoved.key, OPTS) return } diff --git a/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-up-to-start-of-void/index.js b/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-up-to-start-of-void/index.js new file mode 100644 index 000000000..16739c37a --- /dev/null +++ b/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-up-to-start-of-void/index.js @@ -0,0 +1,37 @@ + +import assert from 'assert' + +export default function (state) { + const { document, selection } = state + const texts = document.getTexts() + const first = texts.first() + const second = texts.get(1) + const range = selection.merge({ + anchorKey: first.key, + anchorOffset: 4, + focusKey: second.key, + focusOffset: 0 + }) + + const next = state + .change() + .select(range) + .delete() + .state + + const anchorAndFocusKey = next.document.getTexts().first() + assert.deepEqual( + next.selection.toJS(), + { + anchorKey: anchorAndFocusKey.key, + anchorOffset: anchorAndFocusKey.characters.size, + focusKey: anchorAndFocusKey.key, + focusOffset: anchorAndFocusKey.characters.size, + isBackward: false, + isFocused: false, + marks: null + } + ) + + return next +} diff --git a/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-up-to-start-of-void/input.yaml b/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-up-to-start-of-void/input.yaml new file mode 100644 index 000000000..0442b49fd --- /dev/null +++ b/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-up-to-start-of-void/input.yaml @@ -0,0 +1,15 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: one two + - kind: block + type: image + isVoid: true + - kind: block + type: paragraph + nodes: + - kind: text + text: three diff --git a/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-up-to-start-of-void/output.yaml b/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-up-to-start-of-void/output.yaml new file mode 100644 index 000000000..46f2bc709 --- /dev/null +++ b/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-up-to-start-of-void/output.yaml @@ -0,0 +1,15 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: "one " + - kind: block + type: image + isVoid: true + - kind: block + type: paragraph + nodes: + - kind: text + text: three diff --git a/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-void/index.js b/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-void/index.js new file mode 100644 index 000000000..7edebe90e --- /dev/null +++ b/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-void/index.js @@ -0,0 +1,37 @@ + +import assert from 'assert' + +export default function (state) { + const { document, selection } = state + const texts = document.getTexts() + const first = texts.first() + const second = texts.get(1) + const range = selection.merge({ + anchorKey: first.key, + anchorOffset: 4, + focusKey: second.key, + focusOffset: 1 + }) + + const next = state + .change() + .select(range) + .delete() + .state + + const anchorAndFocusKey = next.document.getTexts().first() + assert.deepEqual( + next.selection.toJS(), + { + anchorKey: anchorAndFocusKey.key, + anchorOffset: anchorAndFocusKey.characters.size, + focusKey: anchorAndFocusKey.key, + focusOffset: anchorAndFocusKey.characters.size, + isBackward: false, + isFocused: false, + marks: null + } + ) + + return next +} diff --git a/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-void/input.yaml b/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-void/input.yaml new file mode 100644 index 000000000..0442b49fd --- /dev/null +++ b/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-void/input.yaml @@ -0,0 +1,15 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: one two + - kind: block + type: image + isVoid: true + - kind: block + type: paragraph + nodes: + - kind: text + text: three diff --git a/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-void/output.yaml b/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-void/output.yaml new file mode 100644 index 000000000..d2eb3e7ab --- /dev/null +++ b/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-void/output.yaml @@ -0,0 +1,12 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: "one " + - kind: block + type: paragraph + nodes: + - kind: text + text: three diff --git a/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void/index.js b/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void/index.js new file mode 100644 index 000000000..9c0e57914 --- /dev/null +++ b/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void/index.js @@ -0,0 +1,36 @@ + +import assert from 'assert' + +export default function (state) { + const { document, selection } = state + const texts = document.getTexts() + const first = texts.first() + const range = selection.merge({ + anchorKey: first.key, + anchorOffset: 4, + focusKey: first.key, + focusOffset: 7 + }) + + const next = state + .change() + .select(range) + .delete() + .state + + const anchorAndFocusKey = next.document.getTexts().first() + assert.deepEqual( + next.selection.toJS(), + { + anchorKey: anchorAndFocusKey.key, + anchorOffset: anchorAndFocusKey.characters.size, + focusKey: anchorAndFocusKey.key, + focusOffset: anchorAndFocusKey.characters.size, + isBackward: false, + isFocused: false, + marks: null + } + ) + + return next +} diff --git a/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void/input.yaml b/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void/input.yaml new file mode 100644 index 000000000..0442b49fd --- /dev/null +++ b/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void/input.yaml @@ -0,0 +1,15 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: one two + - kind: block + type: image + isVoid: true + - kind: block + type: paragraph + nodes: + - kind: text + text: three diff --git a/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void/output.yaml b/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void/output.yaml new file mode 100644 index 000000000..46f2bc709 --- /dev/null +++ b/test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void/output.yaml @@ -0,0 +1,15 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: "one " + - kind: block + type: image + isVoid: true + - kind: block + type: paragraph + nodes: + - kind: text + text: three diff --git a/test/changes/fixtures/at-current-range/delete/void-inline-as-first-with-non-void-block-next/index.js b/test/changes/fixtures/at-current-range/delete/void-inline-as-first-with-non-void-block-next/index.js new file mode 100644 index 000000000..b25382a2d --- /dev/null +++ b/test/changes/fixtures/at-current-range/delete/void-inline-as-first-with-non-void-block-next/index.js @@ -0,0 +1,37 @@ + +import assert from 'assert' + +export default function (state) { + const { document, selection } = state + const texts = document.getTexts() + const firstText = texts.first() + const inlineText = texts.get(1) + const lastBlockText = texts.get(3) + const range = selection.merge({ + anchorKey: inlineText.key, + anchorOffset: 0, + focusKey: lastBlockText.key, + focusOffset: 0 + }) + + const next = state + .change() + .select(range) + .delete() + .state + const newFirstText = next.document.getTexts().first() + assert.deepEqual( + next.selection.toJS(), + { + anchorKey: newFirstText.key, + anchorOffset: firstText.text.length, + focusKey: newFirstText.key, + focusOffset: firstText.text.length, + isBackward: false, + isFocused: false, + marks: null + } + ) + + return next +} diff --git a/test/changes/fixtures/at-current-range/delete/void-inline-as-first-with-non-void-block-next/input.yaml b/test/changes/fixtures/at-current-range/delete/void-inline-as-first-with-non-void-block-next/input.yaml new file mode 100644 index 000000000..7f50b9ad2 --- /dev/null +++ b/test/changes/fixtures/at-current-range/delete/void-inline-as-first-with-non-void-block-next/input.yaml @@ -0,0 +1,20 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: one + - kind: inline + type: image + isVoid: true + nodes: + - kind: text + text: "" + - kind: text + text: two + - kind: block + type: paragraph + nodes: + - kind: text + text: three diff --git a/test/changes/fixtures/at-current-range/delete/void-inline-as-first-with-non-void-block-next/output.yaml b/test/changes/fixtures/at-current-range/delete/void-inline-as-first-with-non-void-block-next/output.yaml new file mode 100644 index 000000000..ab9dcade9 --- /dev/null +++ b/test/changes/fixtures/at-current-range/delete/void-inline-as-first-with-non-void-block-next/output.yaml @@ -0,0 +1,7 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: onethree diff --git a/test/changes/fixtures/at-current-range/delete/whole-block-with-other-block-type-below/index.js b/test/changes/fixtures/at-current-range/delete/whole-block-with-other-block-type-below/index.js new file mode 100644 index 000000000..24ed7692c --- /dev/null +++ b/test/changes/fixtures/at-current-range/delete/whole-block-with-other-block-type-below/index.js @@ -0,0 +1,38 @@ + +import assert from 'assert' + +export default function (state) { + const { document, selection } = state + const texts = document.getTexts() + const first = texts.first() + const second = texts.get(1) + const range = selection.merge({ + anchorKey: first.key, + anchorOffset: 0, + focusKey: second.key, + focusOffset: 0 + }) + + const next = state + .change() + .select(range) + .delete() + .state + + const newFirst = next.document.getTexts().first() + + assert.deepEqual( + next.selection.toJS(), + { + anchorKey: newFirst.key, + anchorOffset: 0, + focusKey: newFirst.key, + focusOffset: 0, + isBackward: false, + isFocused: false, + marks: null + } + ) + + return next +} diff --git a/test/changes/fixtures/at-current-range/delete/whole-block-with-other-block-type-below/input.yaml b/test/changes/fixtures/at-current-range/delete/whole-block-with-other-block-type-below/input.yaml new file mode 100644 index 000000000..47875159e --- /dev/null +++ b/test/changes/fixtures/at-current-range/delete/whole-block-with-other-block-type-below/input.yaml @@ -0,0 +1,12 @@ + +nodes: + - kind: block + type: h1 + nodes: + - kind: text + text: "A header" + - kind: block + type: paragraph + nodes: + - kind: text + text: "A paragraph" diff --git a/test/changes/fixtures/at-current-range/delete/whole-block-with-other-block-type-below/output.yaml b/test/changes/fixtures/at-current-range/delete/whole-block-with-other-block-type-below/output.yaml new file mode 100644 index 000000000..812327c6a --- /dev/null +++ b/test/changes/fixtures/at-current-range/delete/whole-block-with-other-block-type-below/output.yaml @@ -0,0 +1,7 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: "A paragraph"