Skip to content

Commit

Permalink
Improve insertText handling of inline elements (facebook#2042)
Browse files Browse the repository at this point in the history 
* Improve insertText handling of inline elements

* Prettier

* Address feedback
  • Loading branch information
@trueadm
trueadm authored Apr 29, 2022
1 parent 4b8b301 commit 8b0e0bb
Show file tree
Hide file tree
Showing 2 changed files with 151 additions and 41 deletions.
86 changes: 86 additions & 0 deletions packages/lexical-playground/__tests__/e2e/Links.spec.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,92 @@ test.describe('Links', () => {
);
});

test(`Can create a link with some text after, insert paragraph, then backspace, it should merge correctly`, async ({
page,
}) => {
await focusEditor(page);
await page.keyboard.type(' abc def ');
await moveLeft(page, 5);
await selectCharacters(page, 'left', 3);

// link
await click(page, '.link');

await assertHTML(
page,
html`
<p
class="PlaygroundEditorTheme__paragraph PlaygroundEditorTheme__ltr"
dir="ltr">
<span data-lexical-text="true"></span>
<a
class="PlaygroundEditorTheme__link PlaygroundEditorTheme__ltr"
dir="ltr"
href="https://">
<span data-lexical-text="true">abc</span>
</a>
<span data-lexical-text="true">def</span>
</p>
`,
);

await moveLeft(page, 1);
await moveRight(page, 2);
await page.keyboard.press('Enter');

await assertHTML(
page,
html`
<p class="PlaygroundEditorTheme__paragraph">
<span data-lexical-text="true"></span>
<a
class="PlaygroundEditorTheme__link PlaygroundEditorTheme__ltr"
dir="ltr"
href="https://">
<span data-lexical-text="true">ab</span>
</a>
</p>
<p
class="PlaygroundEditorTheme__paragraph PlaygroundEditorTheme__ltr"
dir="ltr">
<a
class="PlaygroundEditorTheme__link PlaygroundEditorTheme__ltr"
dir="ltr"
href="https://">
<span data-lexical-text="true">c</span>
</a>
<span data-lexical-text="true">def</span>
</p>
`,
);

await page.keyboard.press('Backspace');

await assertHTML(
page,
html`
<p
class="PlaygroundEditorTheme__paragraph PlaygroundEditorTheme__ltr"
dir="ltr">
<span data-lexical-text="true"></span>
<a
class="PlaygroundEditorTheme__link PlaygroundEditorTheme__ltr"
dir="ltr"
href="https://">
<span data-lexical-text="true">ab</span>
</a>
<a
class="PlaygroundEditorTheme__link PlaygroundEditorTheme__ltr"
dir="ltr"
href="https://">
<span data-lexical-text="true">c</span>
</a>
<span data-lexical-text="true">def</span>
</p>
`,
);
});

test(`Can create a link then replace it with plain text`, async ({page}) => {
await focusEditor(page);
await page.keyboard.type(' abc ');
Expand Down
106 changes: 65 additions & 41 deletions packages/lexical/src/LexicalSelection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -800,12 +800,25 @@ export class RangeSelection implements BaseSelection {
...firstNode.getParentKeys(),
...lastNode.getParentKeys(),
]);
// We have to get the parent elements before the next section,
// as in that section we might mutate the lastNode.
const firstElement = $isElementNode(firstNode)
? firstNode
: firstNode.getParentOrThrow();
const lastElement = $isElementNode(lastNode)
let lastElement = $isElementNode(lastNode)
? lastNode
: lastNode.getParentOrThrow();
let lastElementWasInline = false;

// If the last element is inline, we should instead look at getting
// the nodes of its parent, rather than itself. This behavior will
// then better match how text node insertions work.
// TODO: should we keep on traversing parents if we're inside another
// nested inline element?
if (!firstElement.is(lastElement) && lastElement.isInline()) {
lastElementWasInline = true;
lastElement = lastElement.getParentOrThrow();
}

// Handle mutations to the last node.
if (
Expand Down Expand Up @@ -848,55 +861,66 @@ export class RangeSelection implements BaseSelection {
const selectedNodesSet = new Set(selectedNodes);
const firstAndLastElementsAreEqual = firstElement.is(lastElement);

// If the last element is an "inline" element, don't move it's text nodes to the first node.
// Instead, preserve the "inline" element's children and append to the first element.
if (!lastElement.canBeEmpty() && firstElement !== lastElement) {
firstElement.append(lastElement);
} else {
for (let i = lastNodeChildren.length - 1; i >= 0; i--) {
const lastNodeChild = lastNodeChildren[i];
// We choose a target to insert all nodes after. In the case of having
// and inline starting parent element with a starting node that has no
// siblings, we should insert after the starting parent element, otherwise
// we will incorrectly merge into the starting parent element.
// TODO: should we keep on traversing parents if we're inside another
// nested inline element?
const insertionTarget =
firstElement.isInline() && firstNode.getNextSibling() === null
? firstElement
: firstNode;

for (let i = lastNodeChildren.length - 1; i >= 0; i--) {
const lastNodeChild = lastNodeChildren[i];

if (
lastNodeChild.is(firstNode) ||
($isElementNode(lastNodeChild) && lastNodeChild.isParentOf(firstNode))
) {
break;
}

if (lastNodeChild.isAttached()) {
if (
lastNodeChild.is(firstNode) ||
($isElementNode(lastNodeChild) &&
lastNodeChild.isParentOf(firstNode))
!selectedNodesSet.has(lastNodeChild) ||
lastNodeChild.is(lastNode) ||
// If the last node parent element was an inline element, then we're
// using the last node's grand parent. This means that the above
// heuristics for checking if the lastNodeChild.is(lastNode) can never
// happen. Instead, we should check the lastNode's parent, which will
// correctly correlate to the right node.
(lastElementWasInline &&
lastNodeChild.is(lastNode.getParentOrThrow()))
) {
break;
}

if (lastNodeChild.isAttached()) {
if (
!selectedNodesSet.has(lastNodeChild) ||
lastNodeChild.is(lastNode)
) {
if (!firstAndLastElementsAreEqual) {
firstNode.insertAfter(lastNodeChild);
}
} else {
lastNodeChild.remove();
if (!firstAndLastElementsAreEqual) {
insertionTarget.insertAfter(lastNodeChild);
}
} else {
lastNodeChild.remove();
}
}
}

if (!firstAndLastElementsAreEqual) {
// Check if we have already moved out all the nodes of the
// last parent, and if so, traverse the parent tree and mark
// them all as being able to deleted too.
let parent = lastElement;
let lastRemovedParent = null;
if (!firstAndLastElementsAreEqual) {
// Check if we have already moved out all the nodes of the
// last parent, and if so, traverse the parent tree and mark
// them all as being able to deleted too.
let parent = lastElement;
let lastRemovedParent = null;

while (parent !== null) {
const children = parent.getChildren();
const childrenLength = children.length;
if (
childrenLength === 0 ||
children[childrenLength - 1].is(lastRemovedParent)
) {
markedNodeKeysForKeep.delete(parent.__key);
lastRemovedParent = parent;
}
parent = parent.getParent();
while (parent !== null) {
const children = parent.getChildren();
const childrenLength = children.length;
if (
childrenLength === 0 ||
children[childrenLength - 1].is(lastRemovedParent)
) {
markedNodeKeysForKeep.delete(parent.__key);
lastRemovedParent = parent;
}
parent = parent.getParent();
}
}

Expand Down

0 comments on commit 8b0e0bb

Please sign in to comment.