Skip to content

Commit

Permalink
fix(editor): fix merge spans normalisation logic
Browse files Browse the repository at this point in the history 
There were a couple of problems with the code before this change:

1. Merge spans normalisation would only be run on spans and as a result of
  specific operations.
2. Merge spans normalisation could run on spans that were already removed.

Now, the normalisation logic is structured as normalisation logic would
normally be structured. It's the parent block that takes care of merging spans
and the normalisation is run whenever the editor deems that it's time to run
normalisation. This makes sure that all spans in the editor at all times are
normalised, not just those that were touched by a specific operation. This
takes care of both problem (1) and (2). We now also don't run
`editor.onChange()` after custom normalisation anymore. Instead, we `return` to
trigger a new normalisation pass.
  • Loading branch information
@christianhg
christianhg committed Aug 19, 2024
1 parent 7797c03 commit 763de2a
Showing 1 changed file with 62 additions and 82 deletions.
144 changes: 62 additions & 82 deletions packages/editor/src/editor/plugins/createWithPortableTextMarkModel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import {isPortableTextBlock, isPortableTextSpan} from '@portabletext/toolkit'
import {isEqual, uniq} from 'lodash'
import {type Subject} from 'rxjs'
import {type Descendant, Editor, Element, Path, Range, Text, Transforms} from 'slate'
import {type Descendant, Editor, Element, Node, Path, Range, Text, Transforms} from 'slate'

import {
type EditorChange,
Expand Down Expand Up @@ -54,29 +54,38 @@ export function createWithPortableTextMarkModel(

// Extend Slate's default normalization. Merge spans with same set of .marks when doing merge_node operations, and clean up markDefs / marks
editor.normalizeNode = (nodeEntry) => {
normalizeNode(nodeEntry)
if (
editor.operations.some((op) =>
[
'insert_node',
'insert_text',
'merge_node',
'remove_node',
'remove_text',
'set_node',
].includes(op.type),
)
) {
mergeSpans(editor)
}
const [node, path] = nodeEntry

const isSpan = Text.isText(node) && node._type === types.span.name
const isTextBlock = editor.isTextBlock(node)

if (editor.isTextBlock(node)) {
const children = Node.children(editor, path)

for (const [child, childPath] of children) {
const nextNode = node.children[childPath[1] + 1]

if (
editor.isTextSpan(child) &&
editor.isTextSpan(nextNode) &&
isEqual(child.marks, nextNode.marks)
) {
debug(
'Merging spans',
JSON.stringify(child, null, 2),
JSON.stringify(nextNode, null, 2),
)
Transforms.mergeNodes(editor, {at: [childPath[0], childPath[1] + 1], voids: true})
return
}
}
}

if (isSpan || isTextBlock) {
if (isSpan && !Array.isArray(node.marks)) {
debug('Adding .marks to span node')
Transforms.setNodes(editor, {marks: []}, {at: path})
editor.onChange()
return
}
const hasSpanMarks = isSpan && (node.marks || []).length > 0
if (hasSpanMarks) {
Expand All @@ -100,7 +109,7 @@ export function createWithPortableTextMarkModel(
{marks: spanMarks.filter((mark) => !orphanedMarks.includes(mark))},
{at: path},
)
editor.onChange()
return
}
}
}
Expand All @@ -124,7 +133,7 @@ export function createWithPortableTextMarkModel(
// eslint-disable-next-line max-depth
if (!isNormalized) {
Transforms.setNodes(editor, {markDefs: newMarkDefs}, {at: targetPath, voids: false})
editor.onChange()
return
}
}
}
Expand All @@ -148,7 +157,7 @@ export function createWithPortableTextMarkModel(
{markDefs: uniq([...oldDefs, ...op.properties.markDefs])},
{at: targetPath, voids: false},
)
editor.onChange()
return
}
}
// Make sure marks are reset, if a block is split at the end.
Expand All @@ -169,7 +178,7 @@ export function createWithPortableTextMarkModel(
child.marks.length > 0
) {
Transforms.setNodes(editor, {marks: []}, {at: childPath, voids: false})
editor.onChange()
return
}
}
// Make sure markDefs are reset, if a block is split at start.
Expand All @@ -192,7 +201,7 @@ export function createWithPortableTextMarkModel(
(!block.children[0].marks || block.children[0].marks.length === 0)
) {
Transforms.setNodes(editor, {markDefs: []}, {at: blockPath})
editor.onChange()
return
}
}
}
Expand All @@ -203,7 +212,7 @@ export function createWithPortableTextMarkModel(
(!node.marks || (node.marks.length > 0 && node.text === ''))
) {
Transforms.setNodes(editor, {marks: []}, {at: path, voids: false})
editor.onChange()
return
}
}
// Check consistency of markDefs (unless we are merging two nodes)
Expand All @@ -229,9 +238,11 @@ export function createWithPortableTextMarkModel(
},
{at: path},
)
editor.onChange()
return
}
}

normalizeNode(nodeEntry)
}

editor.apply = (op) => {
Expand Down Expand Up @@ -353,34 +364,35 @@ export function createWithPortableTextMarkModel(
editor.addMark = (mark: string) => {
if (editor.selection) {
if (Range.isExpanded(editor.selection)) {
// Split if needed
Transforms.setNodes(editor, {}, {match: Text.isText, split: true})
// Use new selection
const splitTextNodes = [
...Editor.nodes(editor, {at: editor.selection, match: Text.isText}),
]
const shouldRemoveMark = splitTextNodes.every((node) => node[0].marks?.includes(mark))

if (shouldRemoveMark) {
editor.removeMark(mark)
return editor
}
Editor.withoutNormalizing(editor, () => {
splitTextNodes.forEach(([node, path]) => {
const marks = [
...(Array.isArray(node.marks) ? node.marks : []).filter(
(eMark: string) => eMark !== mark,
),
mark,
]
Transforms.setNodes(
editor,
{marks},
{at: path, match: Text.isText, split: true, hanging: true},
)
})
// Split if needed
Transforms.setNodes(editor, {}, {match: Text.isText, split: true})
// Use new selection
const splitTextNodes = Range.isRange(editor.selection)
? [...Editor.nodes(editor, {at: editor.selection, match: Text.isText})]
: []
const shouldRemoveMark =
splitTextNodes.length > 1 &&
splitTextNodes.every((node) => node[0].marks?.includes(mark))

if (shouldRemoveMark) {
editor.removeMark(mark)
} else {
splitTextNodes.forEach(([node, path]) => {
const marks = [
...(Array.isArray(node.marks) ? node.marks : []).filter(
(eMark: string) => eMark !== mark,
),
mark,
]
Transforms.setNodes(
editor,
{marks},
{at: path, match: Text.isText, split: true, hanging: true},
)
})
}
})
Editor.normalize(editor)
} else {
const existingMarks: string[] =
{
Expand Down Expand Up @@ -486,36 +498,4 @@ export function createWithPortableTextMarkModel(
}
return editor
}

/**
* Normalize re-marked spans in selection
*/
function mergeSpans(editor: PortableTextSlateEditor) {
const {selection} = editor

if (selection) {
const textNodesInSelection = Array.from(
Editor.nodes(editor, {
at: Editor.range(editor, [selection.anchor.path[0]], [selection.focus.path[0]]),
match: Text.isText,
reverse: true,
}),
)

for (const [node, path] of textNodesInSelection) {
const [parent] = path.length > 1 ? Editor.node(editor, Path.parent(path)) : [undefined]
const nextPath = [path[0], path[1] + 1]

if (editor.isTextBlock(parent)) {
const nextNode = parent.children[nextPath[1]]

if (Text.isText(nextNode) && isEqual(nextNode.marks, node.marks)) {
debug('Merging spans')
Transforms.mergeNodes(editor, {at: nextPath, voids: true})
editor.onChange()
}
}
}
}
}
}

0 comments on commit 763de2a

Please sign in to comment.