Skip to content

Commit

Permalink
Fix overly-aggressive unhangRange (#5193)
Browse files Browse the repository at this point in the history
`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.
  • Loading branch information
SmilinBrian authored Nov 17, 2022
1 parent c8c75e9 commit 6909a8f
Show file tree
Hide file tree
Showing 13 changed files with 183 additions and 12 deletions.
5 changes: 5 additions & 0 deletions .changeset/purple-planes-study.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'slate': patch
---

Stops Editor.unhangRange() from adjusting the range in some cases when it was not actually hanging
8 changes: 7 additions & 1 deletion packages/slate/src/interfaces/editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions packages/slate/src/transforms/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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 })
Expand Down Expand Up @@ -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)) {
Expand Down
2 changes: 1 addition & 1 deletion packages/slate/src/transforms/text.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/** @jsx jsx */
import { Editor } from 'slate'
import { jsx } from '../../..'

export const input = (
<editor>
<block>
<anchor />
This is a first paragraph
</block>
<block>This is the second paragraph</block>
<block void>
This is the third paragraph
{/* unhang should move focus to here */}
</block>
<block>
<focus />
</block>
</editor>
)

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 },
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@ export const input = (
<anchor />
This is a first paragraph
</block>
<block>This is the second paragraph</block>
<block void />
<block>
This is the second paragraph
{/* unhang should move focus to here because, without `voids` set, it should skip over void block below */}
</block>
<block void>This void paragraph gets skipped over</block>
<block>
<focus />
</block>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/** @jsx jsx */
import { Editor } from 'slate'
import { jsx } from '../../..'

export const input = (
<editor>
<block>
<anchor />
This is a first paragraph
<inline void>
<text />
</inline>
<text />
{/* unhang should move focus to here */}
</block>
<block>
<focus />
This is the second paragraph
</block>
</editor>
)

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 },
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/** @jsx jsx */
import { Editor } from 'slate'
import { jsx } from '../../..'

export const input = (
<editor>
<block>
<anchor />
This is the first paragraph
<inline void>
<text />
</inline>
<text />
</block>
<block>
This is the second paragraph
<inline void>
<text />
</inline>
<text />
{/* unhang should move focus to here */}
</block>
<block>
<focus />
This is the third paragraph
</block>
</editor>
)

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 },
}
Original file line number Diff line number Diff line change
@@ -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 = (
<editor>
<block>
<anchor />
This is the first paragraph
<inline void>
<text />
</inline>
<text>
<focus />
</text>
</block>
<block>This is the second paragraph</block>
</editor>
)

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 },
}
Original file line number Diff line number Diff line change
@@ -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 = (
<editor>
<block>
<anchor />
This is the first paragraph
<inline void>
<text />
</inline>
<text />
</block>
<block>
This is the second paragraph
<inline void>
<text />
</inline>
<text>
<focus />
</text>
</block>
<block>This is the third paragraph</block>
</editor>
)

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 },
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,3 @@ export const output = (
</block>
</editor>
)
export const skip = true
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,3 @@ export const output = (
<block>two</block>
</editor>
)
export const skip = true
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,3 @@ export const output = (
</block>
</editor>
)
// TODO this has to be skipped because the second void and the final empty text fail to be marked bold
export const skip = true

0 comments on commit 6909a8f

Please sign in to comment.