From 6909a8f7da0f70b1ef3b5c3a665e8d0d09e6fa99 Mon Sep 17 00:00:00 2001 From: Brian Bucknam Date: Thu, 17 Nov 2022 09:18:11 -0800 Subject: [PATCH] Fix overly-aggressive unhangRange (#5193) `Editor.unhangRange()` could decide to proceed with an adjustment in cases where the range was not hanging. Because the algorithm it uses *always* skips over the first node it encounters, this meant the selection was adjusted in non-hanging cases. This change reduces the chances of an incorrect decision to adjust. Transforms now pass the `voids` flag to `unhangRange()` as it seems logical that the adjusted range should reflect the intention of the operation. This fixes a unit test I added for markable voids that had to be skipped because of the `unhangRange()` error, and fixes a couple other long-skipped tests. --- .changeset/purple-planes-study.md | 5 +++ packages/slate/src/interfaces/editor.ts | 8 +++- packages/slate/src/transforms/node.ts | 8 ++-- packages/slate/src/transforms/text.ts | 2 +- ...-over-non-empty-void-with-voids-option.tsx | 29 +++++++++++++++ .../unhangRange/block-hanging-over-void.tsx | 7 +++- .../Editor/unhangRange/inline-at-end.tsx | 30 +++++++++++++++ .../unhangRange/multi-block-inline-at-end.tsx | 37 +++++++++++++++++++ .../unhangRange/not-hanging-inline-at-end.tsx | 29 +++++++++++++++ .../not-hanging-multi-block-inline-at-end.tsx | 36 ++++++++++++++++++ .../delete/voids-false/inline-over.tsx | 1 - .../insertText/selection/block-hanging.tsx | 1 - .../marks/mark-void-range-hanging.tsx | 2 - 13 files changed, 183 insertions(+), 12 deletions(-) create mode 100644 .changeset/purple-planes-study.md create mode 100644 packages/slate/test/interfaces/Editor/unhangRange/block-hanging-over-non-empty-void-with-voids-option.tsx create mode 100644 packages/slate/test/interfaces/Editor/unhangRange/inline-at-end.tsx create mode 100644 packages/slate/test/interfaces/Editor/unhangRange/multi-block-inline-at-end.tsx create mode 100644 packages/slate/test/interfaces/Editor/unhangRange/not-hanging-inline-at-end.tsx create mode 100644 packages/slate/test/interfaces/Editor/unhangRange/not-hanging-multi-block-inline-at-end.tsx diff --git a/.changeset/purple-planes-study.md b/.changeset/purple-planes-study.md new file mode 100644 index 000000000..e413c1477 --- /dev/null +++ b/.changeset/purple-planes-study.md @@ -0,0 +1,5 @@ +--- +'slate': patch +--- + +Stops Editor.unhangRange() from adjusting the range in some cases when it was not actually hanging diff --git a/packages/slate/src/interfaces/editor.ts b/packages/slate/src/interfaces/editor.ts index 9cfcef841..243eff101 100644 --- a/packages/slate/src/interfaces/editor.ts +++ b/packages/slate/src/interfaces/editor.ts @@ -1632,13 +1632,19 @@ export const Editor: EditorInterface = { let [start, end] = Range.edges(range) // PERF: exit early if we can guarantee that the range isn't hanging. - if (start.offset !== 0 || end.offset !== 0 || Range.isCollapsed(range)) { + if ( + start.offset !== 0 || + end.offset !== 0 || + Range.isCollapsed(range) || + Path.hasPrevious(end.path) + ) { return range } const endBlock = Editor.above(editor, { at: end, match: n => Editor.isBlock(editor, n), + voids, }) const blockPath = endBlock ? endBlock[1] : [] const first = Editor.start(editor, start) diff --git a/packages/slate/src/transforms/node.ts b/packages/slate/src/transforms/node.ts index 97d244472..bf3a07be6 100644 --- a/packages/slate/src/transforms/node.ts +++ b/packages/slate/src/transforms/node.ts @@ -180,7 +180,7 @@ export const NodeTransforms: NodeTransforms = { if (Range.isRange(at)) { if (!hanging) { - at = Editor.unhangRange(editor, at) + at = Editor.unhangRange(editor, at, { voids }) } if (Range.isCollapsed(at)) { @@ -345,7 +345,7 @@ export const NodeTransforms: NodeTransforms = { } if (!hanging && Range.isRange(at)) { - at = Editor.unhangRange(editor, at) + at = Editor.unhangRange(editor, at, { voids }) } if (Range.isRange(at)) { @@ -543,7 +543,7 @@ export const NodeTransforms: NodeTransforms = { } if (!hanging && Range.isRange(at)) { - at = Editor.unhangRange(editor, at) + at = Editor.unhangRange(editor, at, { voids }) } const depths = Editor.nodes(editor, { at, match, mode, voids }) @@ -598,7 +598,7 @@ export const NodeTransforms: NodeTransforms = { } if (!hanging && Range.isRange(at)) { - at = Editor.unhangRange(editor, at) + at = Editor.unhangRange(editor, at, { voids }) } if (split && Range.isRange(at)) { diff --git a/packages/slate/src/transforms/text.ts b/packages/slate/src/transforms/text.ts index 8fcc271a1..177784b9d 100644 --- a/packages/slate/src/transforms/text.ts +++ b/packages/slate/src/transforms/text.ts @@ -265,7 +265,7 @@ export const TextTransforms: TextTransforms = { return } else if (Range.isRange(at)) { if (!hanging) { - at = Editor.unhangRange(editor, at) + at = Editor.unhangRange(editor, at, { voids }) } if (Range.isCollapsed(at)) { diff --git a/packages/slate/test/interfaces/Editor/unhangRange/block-hanging-over-non-empty-void-with-voids-option.tsx b/packages/slate/test/interfaces/Editor/unhangRange/block-hanging-over-non-empty-void-with-voids-option.tsx new file mode 100644 index 000000000..d12f810e2 --- /dev/null +++ b/packages/slate/test/interfaces/Editor/unhangRange/block-hanging-over-non-empty-void-with-voids-option.tsx @@ -0,0 +1,29 @@ +/** @jsx jsx */ +import { Editor } from 'slate' +import { jsx } from '../../..' + +export const input = ( + + + + This is a first paragraph + + This is the second paragraph + + This is the third paragraph + {/* unhang should move focus to here */} + + + + + +) + +export const test = editor => { + return Editor.unhangRange(editor, editor.selection, { voids: true }) +} + +export const output = { + anchor: { path: [0, 0], offset: 0 }, + focus: { path: [2, 0], offset: 27 }, +} diff --git a/packages/slate/test/interfaces/Editor/unhangRange/block-hanging-over-void.tsx b/packages/slate/test/interfaces/Editor/unhangRange/block-hanging-over-void.tsx index c764b120c..63f7e29e0 100644 --- a/packages/slate/test/interfaces/Editor/unhangRange/block-hanging-over-void.tsx +++ b/packages/slate/test/interfaces/Editor/unhangRange/block-hanging-over-void.tsx @@ -8,8 +8,11 @@ export const input = ( This is a first paragraph - This is the second paragraph - + + This is the second paragraph + {/* unhang should move focus to here because, without `voids` set, it should skip over void block below */} + + This void paragraph gets skipped over diff --git a/packages/slate/test/interfaces/Editor/unhangRange/inline-at-end.tsx b/packages/slate/test/interfaces/Editor/unhangRange/inline-at-end.tsx new file mode 100644 index 000000000..d71c9b56e --- /dev/null +++ b/packages/slate/test/interfaces/Editor/unhangRange/inline-at-end.tsx @@ -0,0 +1,30 @@ +/** @jsx jsx */ +import { Editor } from 'slate' +import { jsx } from '../../..' + +export const input = ( + + + + This is a first paragraph + + + + + {/* unhang should move focus to here */} + + + + This is the second paragraph + + +) + +export const test = editor => { + return Editor.unhangRange(editor, editor.selection, { voids: true }) +} + +export const output = { + anchor: { path: [0, 0], offset: 0 }, + focus: { path: [0, 2], offset: 0 }, +} diff --git a/packages/slate/test/interfaces/Editor/unhangRange/multi-block-inline-at-end.tsx b/packages/slate/test/interfaces/Editor/unhangRange/multi-block-inline-at-end.tsx new file mode 100644 index 000000000..af266d1e3 --- /dev/null +++ b/packages/slate/test/interfaces/Editor/unhangRange/multi-block-inline-at-end.tsx @@ -0,0 +1,37 @@ +/** @jsx jsx */ +import { Editor } from 'slate' +import { jsx } from '../../..' + +export const input = ( + + + + This is the first paragraph + + + + + + + This is the second paragraph + + + + + {/* unhang should move focus to here */} + + + + This is the third paragraph + + +) + +export const test = editor => { + return Editor.unhangRange(editor, editor.selection, { voids: true }) +} + +export const output = { + anchor: { path: [0, 0], offset: 0 }, + focus: { path: [1, 2], offset: 0 }, +} diff --git a/packages/slate/test/interfaces/Editor/unhangRange/not-hanging-inline-at-end.tsx b/packages/slate/test/interfaces/Editor/unhangRange/not-hanging-inline-at-end.tsx new file mode 100644 index 000000000..2d259f11f --- /dev/null +++ b/packages/slate/test/interfaces/Editor/unhangRange/not-hanging-inline-at-end.tsx @@ -0,0 +1,29 @@ +/** @jsx jsx */ +/* The starting selection range is not hanging, so should not be adjusted */ +import { Editor } from 'slate' +import { jsx } from '../../..' + +export const input = ( + + + + This is the first paragraph + + + + + + + + This is the second paragraph + +) + +export const test = editor => { + return Editor.unhangRange(editor, editor.selection, { voids: true }) +} + +export const output = { + anchor: { path: [0, 0], offset: 0 }, + focus: { path: [0, 2], offset: 0 }, +} diff --git a/packages/slate/test/interfaces/Editor/unhangRange/not-hanging-multi-block-inline-at-end.tsx b/packages/slate/test/interfaces/Editor/unhangRange/not-hanging-multi-block-inline-at-end.tsx new file mode 100644 index 000000000..c805db18a --- /dev/null +++ b/packages/slate/test/interfaces/Editor/unhangRange/not-hanging-multi-block-inline-at-end.tsx @@ -0,0 +1,36 @@ +/** @jsx jsx */ +/* The starting selection range is not hanging, so should not be adjusted */ +import { Editor } from 'slate' +import { jsx } from '../../..' + +export const input = ( + + + + This is the first paragraph + + + + + + + This is the second paragraph + + + + + + + + This is the third paragraph + +) + +export const test = editor => { + return Editor.unhangRange(editor, editor.selection, { voids: true }) +} + +export const output = { + anchor: { path: [0, 0], offset: 0 }, + focus: { path: [1, 2], offset: 0 }, +} diff --git a/packages/slate/test/transforms/delete/voids-false/inline-over.tsx b/packages/slate/test/transforms/delete/voids-false/inline-over.tsx index 5a6ddb458..e7cecc548 100644 --- a/packages/slate/test/transforms/delete/voids-false/inline-over.tsx +++ b/packages/slate/test/transforms/delete/voids-false/inline-over.tsx @@ -28,4 +28,3 @@ export const output = ( ) -export const skip = true diff --git a/packages/slate/test/transforms/insertText/selection/block-hanging.tsx b/packages/slate/test/transforms/insertText/selection/block-hanging.tsx index c8468289c..3fa7c3435 100644 --- a/packages/slate/test/transforms/insertText/selection/block-hanging.tsx +++ b/packages/slate/test/transforms/insertText/selection/block-hanging.tsx @@ -25,4 +25,3 @@ export const output = ( two ) -export const skip = true diff --git a/packages/slate/test/transforms/setNodes/marks/mark-void-range-hanging.tsx b/packages/slate/test/transforms/setNodes/marks/mark-void-range-hanging.tsx index b67971c1f..50f738442 100644 --- a/packages/slate/test/transforms/setNodes/marks/mark-void-range-hanging.tsx +++ b/packages/slate/test/transforms/setNodes/marks/mark-void-range-hanging.tsx @@ -47,5 +47,3 @@ export const output = ( ) -// TODO this has to be skipped because the second void and the final empty text fail to be marked bold -export const skip = true