From 171e0d83692505aaa9c3a068e69ecc0d9ee35dcf Mon Sep 17 00:00:00 2001 From: Ty Mick Date: Mon, 9 Sep 2024 13:35:13 -0700 Subject: [PATCH 1/5] Fix selections with non-void non-editable focus "Non-void non-editable" refers to `contentEditable={false}` DOM nodes that are rendered by a Slate element render but which are not void elements. For instance, [the checkboxes in the checklists example][1]. [1]: https://github.com/ianstormtaylor/slate/blob/7e77a932f0489a9fff2d8a1957aa2dd9b324aa78/site/examples/check-lists.tsx#L153-L170 --- .changeset/brown-ears-tap.md | 5 ++ .../slate-react/src/components/editable.tsx | 4 +- .../slate-react/src/plugin/react-editor.ts | 62 +++++++++++++++---- packages/slate-react/src/utils/dom.ts | 18 ++++++ 4 files changed, 73 insertions(+), 16 deletions(-) create mode 100644 .changeset/brown-ears-tap.md diff --git a/.changeset/brown-ears-tap.md b/.changeset/brown-ears-tap.md new file mode 100644 index 0000000000..82c8d0895a --- /dev/null +++ b/.changeset/brown-ears-tap.md @@ -0,0 +1,5 @@ +--- +'slate-react': patch +--- + +Fix selections with non-void non-editable focus diff --git a/packages/slate-react/src/components/editable.tsx b/packages/slate-react/src/components/editable.tsx index 065e14ffef..88e15487fd 100644 --- a/packages/slate-react/src/components/editable.tsx +++ b/packages/slate-react/src/components/editable.tsx @@ -255,9 +255,7 @@ export const Editable = forwardRef( ReactEditor.hasEditableTarget(editor, anchorNode) || ReactEditor.isTargetInsideNonReadonlyVoid(editor, anchorNode) - const focusNodeSelectable = - ReactEditor.hasEditableTarget(editor, focusNode) || - ReactEditor.isTargetInsideNonReadonlyVoid(editor, focusNode) + const focusNodeSelectable = ReactEditor.hasTarget(editor, focusNode) if (anchorNodeSelectable && focusNodeSelectable) { const range = ReactEditor.toSlateRange(editor, domSelection, { diff --git a/packages/slate-react/src/plugin/react-editor.ts b/packages/slate-react/src/plugin/react-editor.ts index d2430211a0..e763456e4c 100644 --- a/packages/slate-react/src/plugin/react-editor.ts +++ b/packages/slate-react/src/plugin/react-editor.ts @@ -20,6 +20,8 @@ import { DOMText, getSelection, hasShadowRoot, + isAfter, + isBefore, isDOMElement, isDOMNode, isDOMSelection, @@ -244,6 +246,11 @@ export interface ReactEditorInterface { options: { exactMatch: boolean suppressThrow: T + /** + * The direction to search for Slate leaf nodes if `domPoint` is + * non-editable and non-void. + */ + searchDirection?: 'forward' | 'backward' } ) => T extends true ? Point | null : Point @@ -681,9 +688,10 @@ export const ReactEditor: ReactEditorInterface = { options: { exactMatch: boolean suppressThrow: T + searchDirection?: 'forward' | 'backward' } ): T extends true ? Point | null : Point => { - const { exactMatch, suppressThrow } = options + const { exactMatch, suppressThrow, searchDirection } = options const [nearestNode, nearestOffset] = exactMatch ? domPoint : normalizeDOMPoint(domPoint) @@ -702,6 +710,13 @@ export const ReactEditor: ReactEditorInterface = { potentialVoidNode && editorEl.contains(potentialVoidNode) ? potentialVoidNode : null + const potentialNonEditableNode = parentNode.closest( + '[contenteditable="false"]' + ) + const nonEditableNode = + potentialNonEditableNode && editorEl.contains(potentialNonEditableNode) + ? potentialNonEditableNode + : null let leafNode = parentNode.closest('[data-slate-leaf]') let domNode: DOMElement | null = null @@ -778,6 +793,35 @@ export const ReactEditor: ReactEditorInterface = { offset -= el.textContent!.length }) } + } else if (nonEditableNode) { + // Find the edge of the nearest leaf in `searchDirection` from the + // non-editable node + const leafNodes = Array.from( + editorEl.querySelectorAll( + // This editor's leaves (not those of a nested editor) + '[data-slate-leaf]:not(:scope [data-slate-editor] [data-slate-leaf])' + ) + ) + leafNode = + (searchDirection === 'forward' + ? leafNodes.find(leaf => isAfter(nonEditableNode, leaf)) + : leafNodes.findLast(leaf => isBefore(nonEditableNode, leaf))) ?? + null + + if (!leafNode) { + offset = 1 + } else { + textNode = leafNode.closest('[data-slate-node="text"]')! + domNode = leafNode + if (searchDirection === 'forward') { + offset = 0 + } else { + offset = domNode.textContent!.length + domNode.querySelectorAll('[data-slate-zero-width]').forEach(el => { + offset -= el.textContent!.length + }) + } + } } if ( @@ -978,18 +1022,6 @@ export const ReactEditor: ReactEditorInterface = { focusOffset-- } - // COMPAT: Triple-clicking a word in chrome will sometimes place the focus - // inside a `contenteditable="false"` DOM node following the word, which - // will cause `toSlatePoint` to throw an error. (2023/03/07) - if ( - 'getAttribute' in focusNode && - (focusNode as HTMLElement).getAttribute('contenteditable') === 'false' && - (focusNode as HTMLElement).getAttribute('data-slate-void') !== 'true' - ) { - focusNode = anchorNode - focusOffset = anchorNode.textContent?.length || 0 - } - const anchor = ReactEditor.toSlatePoint( editor, [anchorNode, anchorOffset], @@ -1002,11 +1034,15 @@ export const ReactEditor: ReactEditorInterface = { return null as T extends true ? Range | null : Range } + const focusBeforeAnchor = + isBefore(anchorNode, focusNode) || + (anchorNode === focusNode && focusOffset < anchorOffset) const focus = isCollapsed ? anchor : ReactEditor.toSlatePoint(editor, [focusNode, focusOffset], { exactMatch, suppressThrow, + searchDirection: focusBeforeAnchor ? 'forward' : 'backward', }) if (!focus) { return null as T extends true ? Range | null : Range diff --git a/packages/slate-react/src/utils/dom.ts b/packages/slate-react/src/utils/dom.ts index 32f53975ee..d600faee9c 100644 --- a/packages/slate-react/src/utils/dom.ts +++ b/packages/slate-react/src/utils/dom.ts @@ -337,3 +337,21 @@ export const getActiveElement = () => { return activeElement } + +/** + * @returns `true` if `otherNode` is before `node` in the document; otherwise, `false`. + */ +export const isBefore = (node: DOMNode, otherNode: DOMNode): boolean => + Boolean( + node.compareDocumentPosition(otherNode) & + DOMNode.DOCUMENT_POSITION_PRECEDING + ) + +/** + * @returns `true` if `otherNode` is after `node` in the document; otherwise, `false`. + */ +export const isAfter = (node: DOMNode, otherNode: DOMNode): boolean => + Boolean( + node.compareDocumentPosition(otherNode) & + DOMNode.DOCUMENT_POSITION_FOLLOWING + ) From 9534c218343c72f535c45af5718037c8c42d9e89 Mon Sep 17 00:00:00 2001 From: Ty Mick Date: Tue, 10 Sep 2024 13:31:29 -0700 Subject: [PATCH 2/5] fixup! Fix selections with non-void non-editable focus Optimize leaf node search --- .../slate-react/src/plugin/react-editor.ts | 37 +++++++++++++------ 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/packages/slate-react/src/plugin/react-editor.ts b/packages/slate-react/src/plugin/react-editor.ts index e763456e4c..362a657e80 100644 --- a/packages/slate-react/src/plugin/react-editor.ts +++ b/packages/slate-react/src/plugin/react-editor.ts @@ -691,7 +691,7 @@ export const ReactEditor: ReactEditorInterface = { searchDirection?: 'forward' | 'backward' } ): T extends true ? Point | null : Point => { - const { exactMatch, suppressThrow, searchDirection } = options + const { exactMatch, suppressThrow, searchDirection = 'backward' } = options const [nearestNode, nearestOffset] = exactMatch ? domPoint : normalizeDOMPoint(domPoint) @@ -796,17 +796,32 @@ export const ReactEditor: ReactEditorInterface = { } else if (nonEditableNode) { // Find the edge of the nearest leaf in `searchDirection` from the // non-editable node - const leafNodes = Array.from( - editorEl.querySelectorAll( - // This editor's leaves (not those of a nested editor) - '[data-slate-leaf]:not(:scope [data-slate-editor] [data-slate-leaf])' - ) + const getLeafNodes = (node: DOMElement | null | undefined) => + node + ? node.querySelectorAll( + // Exclude leaf nodes in nested editors + '[data-slate-leaf]:not(:scope [data-slate-editor] [data-slate-leaf])' + ) + : [] + const elementNode = nonEditableNode.closest( + '[data-slate-node="element"]' ) - leafNode = - (searchDirection === 'forward' - ? leafNodes.find(leaf => isAfter(nonEditableNode, leaf)) - : leafNodes.findLast(leaf => isBefore(nonEditableNode, leaf))) ?? - null + + if (searchDirection === 'forward') { + const leafNodes = [ + ...getLeafNodes(elementNode), + ...getLeafNodes(elementNode?.nextElementSibling), + ] + leafNode = + leafNodes.find(leaf => isAfter(nonEditableNode, leaf)) ?? null + } else { + const leafNodes = [ + ...getLeafNodes(elementNode?.previousElementSibling), + ...getLeafNodes(elementNode), + ] + leafNode = + leafNodes.findLast(leaf => isBefore(nonEditableNode, leaf)) ?? null + } if (!leafNode) { offset = 1 From 98630d11ebf14e40fdcfb59c080530e69c0bfb5e Mon Sep 17 00:00:00 2001 From: Ty Mick Date: Tue, 10 Sep 2024 13:48:36 -0700 Subject: [PATCH 3/5] fixup! Fix selections with non-void non-editable focus Rename `focusNodeSelectable` to `focusNodeIsSelectable` A more accurate name given this PR's changes. --- packages/slate-react/src/components/editable.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/slate-react/src/components/editable.tsx b/packages/slate-react/src/components/editable.tsx index 88e15487fd..f1945d53ae 100644 --- a/packages/slate-react/src/components/editable.tsx +++ b/packages/slate-react/src/components/editable.tsx @@ -255,9 +255,9 @@ export const Editable = forwardRef( ReactEditor.hasEditableTarget(editor, anchorNode) || ReactEditor.isTargetInsideNonReadonlyVoid(editor, anchorNode) - const focusNodeSelectable = ReactEditor.hasTarget(editor, focusNode) + const focusNodeInEditor = ReactEditor.hasTarget(editor, focusNode) - if (anchorNodeSelectable && focusNodeSelectable) { + if (anchorNodeSelectable && focusNodeInEditor) { const range = ReactEditor.toSlateRange(editor, domSelection, { exactMatch: false, suppressThrow: true, @@ -277,7 +277,7 @@ export const Editable = forwardRef( } // Deselect the editor if the dom selection is not selectable in readonly mode - if (readOnly && (!anchorNodeSelectable || !focusNodeSelectable)) { + if (readOnly && (!anchorNodeSelectable || !focusNodeInEditor)) { Transforms.deselect(editor) } } From 1f368a9748298dac1ed8870031d2268ae14011e0 Mon Sep 17 00:00:00 2001 From: Ty Mick Date: Tue, 10 Sep 2024 13:57:37 -0700 Subject: [PATCH 4/5] fixup! Fix selections with non-void non-editable focus Remove inapplicable `if` branch --- packages/slate-react/src/plugin/react-editor.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/slate-react/src/plugin/react-editor.ts b/packages/slate-react/src/plugin/react-editor.ts index 362a657e80..1f4b9bc425 100644 --- a/packages/slate-react/src/plugin/react-editor.ts +++ b/packages/slate-react/src/plugin/react-editor.ts @@ -823,9 +823,7 @@ export const ReactEditor: ReactEditorInterface = { leafNodes.findLast(leaf => isBefore(nonEditableNode, leaf)) ?? null } - if (!leafNode) { - offset = 1 - } else { + if (leafNode) { textNode = leafNode.closest('[data-slate-node="text"]')! domNode = leafNode if (searchDirection === 'forward') { From f4654c3496bb759a5e34e871fb8de602f9386754 Mon Sep 17 00:00:00 2001 From: Ty Mick Date: Tue, 10 Sep 2024 13:58:14 -0700 Subject: [PATCH 5/5] fixup! Fix selections with non-void non-editable focus Improve comment --- packages/slate-react/src/plugin/react-editor.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/slate-react/src/plugin/react-editor.ts b/packages/slate-react/src/plugin/react-editor.ts index 1f4b9bc425..a221f7e6d0 100644 --- a/packages/slate-react/src/plugin/react-editor.ts +++ b/packages/slate-react/src/plugin/react-editor.ts @@ -794,8 +794,7 @@ export const ReactEditor: ReactEditorInterface = { }) } } else if (nonEditableNode) { - // Find the edge of the nearest leaf in `searchDirection` from the - // non-editable node + // Find the edge of the nearest leaf in `searchDirection` const getLeafNodes = (node: DOMElement | null | undefined) => node ? node.querySelectorAll(