Skip to content

Commit 5492542

Browse files
authored
fix(richtext-lexical): prevent extra paragraph when inserting blocks or uploadNodes. Add preemptive selection normalization (#12077)
Fixes #11628 PR #6389 caused bug #11628, which is a regression, as it had already been fixed in #4441 It is likely that some things have changed because [Lexical had recently made improvements](facebook/lexical#7046) to address selection normalization. Although it wasn't necessary to resolve the issue, I added a `NormalizeSelectionPlugin` to the editor, which makes selection handling in the editor more robust. I'm also adding a new collection to the Lexical test suite, intending it to be used by default for most tests going forward. I've left an explanatory comment on the dashboard. ___ Looking at #11628's video, it seems users also want to be able to prevent the first paragraph from being empty. This makes sense to me, so I think in another PR we could add a button at the top, just [like we did at the bottom of the editor](#10530).
1 parent 9948040 commit 5492542

File tree

14 files changed

+312
-52
lines changed

14 files changed

+312
-52
lines changed

packages/richtext-lexical/src/features/blocks/client/plugin/index.tsx

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -56,22 +56,15 @@ export const BlocksPlugin: PluginComponent = () => {
5656

5757
if ($isRangeSelection(selection)) {
5858
const blockNode = $createBlockNode(payload)
59-
// Insert blocks node BEFORE potentially removing focusNode, as $insertNodeToNearestRoot errors if the focusNode doesn't exist
60-
$insertNodeToNearestRoot(blockNode)
6159

60+
// we need to get the focus node before inserting the block node, as $insertNodeToNearestRoot can change the focus node
6261
const { focus } = selection
6362
const focusNode = focus.getNode()
63+
// Insert blocks node BEFORE potentially removing focusNode, as $insertNodeToNearestRoot errors if the focusNode doesn't exist
64+
$insertNodeToNearestRoot(blockNode)
6465

65-
// First, delete currently selected node if it's an empty paragraph and if there are sufficient
66-
// paragraph nodes (more than 1) left in the parent node, so that we don't "trap" the user
67-
if (
68-
$isParagraphNode(focusNode) &&
69-
focusNode.getTextContentSize() === 0 &&
70-
focusNode
71-
.getParentOrThrow()
72-
.getChildren()
73-
.filter((node) => $isParagraphNode(node)).length > 1
74-
) {
66+
// Delete the node it it's an empty paragraph
67+
if ($isParagraphNode(focusNode) && !focusNode.__first) {
7568
focusNode.remove()
7669
}
7770
}

packages/richtext-lexical/src/features/relationship/client/plugins/index.tsx

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -53,22 +53,14 @@ export const RelationshipPlugin: PluginComponent<RelationshipFeatureProps> = ({
5353

5454
if ($isRangeSelection(selection)) {
5555
const relationshipNode = $createRelationshipNode(payload)
56-
// Insert relationship node BEFORE potentially removing focusNode, as $insertNodeToNearestRoot errors if the focusNode doesn't exist
57-
$insertNodeToNearestRoot(relationshipNode)
58-
56+
// we need to get the focus node before inserting the block node, as $insertNodeToNearestRoot can change the focus node
5957
const { focus } = selection
6058
const focusNode = focus.getNode()
59+
// Insert relationship node BEFORE potentially removing focusNode, as $insertNodeToNearestRoot errors if the focusNode doesn't exist
60+
$insertNodeToNearestRoot(relationshipNode)
6161

62-
// First, delete currently selected node if it's an empty paragraph and if there are sufficient
63-
// paragraph nodes (more than 1) left in the parent node, so that we don't "trap" the user
64-
if (
65-
$isParagraphNode(focusNode) &&
66-
focusNode.getTextContentSize() === 0 &&
67-
focusNode
68-
.getParentOrThrow()
69-
.getChildren()
70-
.filter((node) => $isParagraphNode(node)).length > 1
71-
) {
62+
// Delete the node it it's an empty paragraph
63+
if ($isParagraphNode(focusNode) && !focusNode.__first) {
7264
focusNode.remove()
7365
}
7466
}

packages/richtext-lexical/src/features/upload/client/plugin/index.tsx

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -53,18 +53,14 @@ export const UploadPlugin: PluginComponent<UploadFeaturePropsClient> = ({ client
5353
value: payload.value,
5454
},
5555
})
56-
// Insert upload node BEFORE potentially removing focusNode, as $insertNodeToNearestRoot errors if the focusNode doesn't exist
57-
$insertNodeToNearestRoot(uploadNode)
58-
56+
// we need to get the focus node before inserting the block node, as $insertNodeToNearestRoot can change the focus node
5957
const { focus } = selection
6058
const focusNode = focus.getNode()
59+
// Insert upload node BEFORE potentially removing focusNode, as $insertNodeToNearestRoot errors if the focusNode doesn't exist
60+
$insertNodeToNearestRoot(uploadNode)
6161

62-
// Delete the node it it's an empty paragraph and it has at least one sibling, so that we don't "trap" the user
63-
if (
64-
$isParagraphNode(focusNode) &&
65-
!focusNode.__first &&
66-
(focusNode.__prev || focusNode.__next)
67-
) {
62+
// Delete the node it it's an empty paragraph
63+
if ($isParagraphNode(focusNode) && !focusNode.__first) {
6864
focusNode.remove()
6965
}
7066
}

packages/richtext-lexical/src/lexical/LexicalEditor.tsx

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,7 @@ import { LexicalErrorBoundary } from '@lexical/react/LexicalErrorBoundary.js'
44
import { HistoryPlugin } from '@lexical/react/LexicalHistoryPlugin.js'
55
import { OnChangePlugin } from '@lexical/react/LexicalOnChangePlugin.js'
66
import { RichTextPlugin } from '@lexical/react/LexicalRichTextPlugin.js'
7-
import {
8-
$createParagraphNode,
9-
$getRoot,
10-
BLUR_COMMAND,
11-
COMMAND_PRIORITY_LOW,
12-
FOCUS_COMMAND,
13-
} from 'lexical'
7+
import { BLUR_COMMAND, COMMAND_PRIORITY_LOW, FOCUS_COMMAND } from 'lexical'
148
import * as React from 'react'
159
import { useEffect, useState } from 'react'
1610

@@ -24,6 +18,7 @@ import { AddBlockHandlePlugin } from './plugins/handles/AddBlockHandlePlugin/ind
2418
import { DraggableBlockPlugin } from './plugins/handles/DraggableBlockPlugin/index.js'
2519
import { InsertParagraphAtEndPlugin } from './plugins/InsertParagraphAtEnd/index.js'
2620
import { MarkdownShortcutPlugin } from './plugins/MarkdownShortcut/index.js'
21+
import { NormalizeSelectionPlugin } from './plugins/NormalizeSelection/index.js'
2722
import { SlashMenuPlugin } from './plugins/SlashMenu/index.js'
2823
import { TextPlugin } from './plugins/TextPlugin/index.js'
2924
import { LexicalContentEditable } from './ui/ContentEditable.js'
@@ -112,6 +107,7 @@ export const LexicalEditor: React.FC<
112107
}
113108
ErrorBoundary={LexicalErrorBoundary}
114109
/>
110+
<NormalizeSelectionPlugin />
115111
<InsertParagraphAtEndPlugin />
116112
<DecoratorPlugin />
117113
<TextPlugin features={editorConfig.features} />
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import { useLexicalComposerContext } from '@lexical/react/LexicalComposerContext'
2+
import { $getSelection, $isRangeSelection, RootNode } from 'lexical'
3+
import { useEffect } from 'react'
4+
5+
/**
6+
* By default, Lexical throws an error if the selection ends in deleted nodes.
7+
* This is very aggressive considering there are reasons why this can happen
8+
* outside of Payload's control (custom features or conflicting features, for example).
9+
* In the case of selections on nonexistent nodes, this plugin moves the selection to
10+
* the end of the editor and displays a warning instead of an error.
11+
*/
12+
export function NormalizeSelectionPlugin() {
13+
const [editor] = useLexicalComposerContext()
14+
15+
useEffect(() => {
16+
return editor.registerNodeTransform(RootNode, (root) => {
17+
const selection = $getSelection()
18+
if ($isRangeSelection(selection)) {
19+
const anchorNode = selection.anchor.getNode()
20+
const focusNode = selection.focus.getNode()
21+
if (!anchorNode.isAttached() || !focusNode.isAttached()) {
22+
root.selectEnd()
23+
// eslint-disable-next-line no-console
24+
console.warn(
25+
'updateEditor: selection has been moved to the end of the editor because the previously selected nodes have been removed and ' +
26+
"selection wasn't moved to another node. Ensure selection changes after removing/replacing a selected node.",
27+
)
28+
}
29+
}
30+
return false
31+
})
32+
}, [editor])
33+
34+
return null
35+
}

test/lexical/baseConfig.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { fileURLToPath } from 'node:url'
22
import path from 'path'
33
import { type Config } from 'payload'
44

5+
import { LexicalFullyFeatured } from './collections/_LexicalFullyFeatured/index.js'
56
import ArrayFields from './collections/Array/index.js'
67
import {
78
getLexicalFieldsCollection,
@@ -26,6 +27,7 @@ const dirname = path.dirname(filename)
2627
export const baseConfig: Partial<Config> = {
2728
// ...extend config here
2829
collections: [
30+
LexicalFullyFeatured,
2931
getLexicalFieldsCollection({
3032
blocks: lexicalBlocks,
3133
inlineBlocks: lexicalInlineBlocks,
@@ -42,10 +44,18 @@ export const baseConfig: Partial<Config> = {
4244
ArrayFields,
4345
],
4446
globals: [TabsWithRichText],
47+
4548
admin: {
4649
importMap: {
4750
baseDir: path.resolve(dirname),
4851
},
52+
components: {
53+
beforeDashboard: [
54+
{
55+
path: './components/CollectionsExplained.tsx#CollectionsExplained',
56+
},
57+
],
58+
},
4959
},
5060
onInit: async (payload) => {
5161
if (process.env.SEED_IN_CONFIG_ONINIT !== 'false') {

test/lexical/collections/Lexical/e2e/blocks/e2e.spec.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ describe('lexicalBlocks', () => {
302302
await assertLexicalDoc({
303303
fn: ({ lexicalWithBlocks }) => {
304304
const rscBlock: SerializedBlockNode = lexicalWithBlocks.root
305-
.children[14] as SerializedBlockNode
305+
.children[13] as SerializedBlockNode
306306
const paragraphNode: SerializedParagraphNode = lexicalWithBlocks.root
307307
.children[12] as SerializedParagraphNode
308308

@@ -1133,9 +1133,9 @@ describe('lexicalBlocks', () => {
11331133
).docs[0] as never
11341134

11351135
const richTextBlock: SerializedBlockNode = lexicalWithBlocks.root
1136-
.children[13] as SerializedBlockNode
1136+
.children[12] as SerializedBlockNode
11371137
const subRichTextBlock: SerializedBlockNode = richTextBlock.fields.richTextField.root
1138-
.children[1] as SerializedBlockNode // index 0 and 2 are paragraphs created by default around the block node when a new block is added via slash command
1138+
.children[0] as SerializedBlockNode // index 0 and 2 are paragraphs created by default around the block node when a new block is added via slash command
11391139

11401140
const subSubRichTextField = subRichTextBlock.fields.subRichTextField
11411141
const subSubUploadField = subRichTextBlock.fields.subUploadField
@@ -1163,9 +1163,9 @@ describe('lexicalBlocks', () => {
11631163
).docs[0] as never
11641164

11651165
const richTextBlock2: SerializedBlockNode = lexicalWithBlocks.root
1166-
.children[13] as SerializedBlockNode
1166+
.children[12] as SerializedBlockNode
11671167
const subRichTextBlock2: SerializedBlockNode = richTextBlock2.fields.richTextField.root
1168-
.children[1] as SerializedBlockNode // index 0 and 2 are paragraphs created by default around the block node when a new block is added via slash command
1168+
.children[0] as SerializedBlockNode // index 0 and 2 are paragraphs created by default around the block node when a new block is added via slash command
11691169

11701170
const subSubRichTextField2 = subRichTextBlock2.fields.subRichTextField
11711171
const subSubUploadField2 = subRichTextBlock2.fields.subUploadField

test/lexical/collections/Lexical/e2e/main/e2e.spec.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -728,7 +728,8 @@ describe('lexicalMain', () => {
728728
await expect(relationshipListDrawer).toBeVisible()
729729
await wait(500)
730730

731-
await expect(relationshipListDrawer.locator('.rs__single-value')).toHaveText('Lexical Field')
731+
await relationshipListDrawer.locator('.rs__input').first().click()
732+
await relationshipListDrawer.locator('.rs__menu').getByText('Lexical Field').click()
732733

733734
await relationshipListDrawer.locator('button').getByText('Rich Text').first().click()
734735
await expect(relationshipListDrawer).toBeHidden()
@@ -1203,10 +1204,11 @@ describe('lexicalMain', () => {
12031204
await expect(newUploadNode.locator('.lexical-upload__bottomRow')).toContainText('payload.png')
12041205

12051206
await page.keyboard.press('Enter') // floating toolbar needs to appear with enough distance to the upload node, otherwise clicking may fail
1207+
await page.keyboard.press('Enter')
12061208
await page.keyboard.press('ArrowLeft')
12071209
await page.keyboard.press('ArrowLeft')
12081210
// Select "there" by pressing shift + arrow left
1209-
for (let i = 0; i < 4; i++) {
1211+
for (let i = 0; i < 5; i++) {
12101212
await page.keyboard.press('Shift+ArrowLeft')
12111213
}
12121214

@@ -1258,10 +1260,10 @@ describe('lexicalMain', () => {
12581260
const firstParagraph: SerializedParagraphNode = lexicalField.root
12591261
.children[0] as SerializedParagraphNode
12601262
const secondParagraph: SerializedParagraphNode = lexicalField.root
1261-
.children[1] as SerializedParagraphNode
1262-
const thirdParagraph: SerializedParagraphNode = lexicalField.root
12631263
.children[2] as SerializedParagraphNode
1264-
const uploadNode: SerializedUploadNode = lexicalField.root.children[3] as SerializedUploadNode
1264+
const thirdParagraph: SerializedParagraphNode = lexicalField.root
1265+
.children[3] as SerializedParagraphNode
1266+
const uploadNode: SerializedUploadNode = lexicalField.root.children[1] as SerializedUploadNode
12651267

12661268
expect(firstParagraph.children).toHaveLength(2)
12671269
expect((firstParagraph.children[0] as SerializedTextNode).text).toBe('Some ')
@@ -1391,7 +1393,7 @@ describe('lexicalMain', () => {
13911393
const lexicalField: SerializedEditorState = lexicalDoc.lexicalRootEditor
13921394

13931395
// @ts-expect-error no need to type this
1394-
expect(lexicalField?.root?.children[1].fields.someTextRequired).toEqual('test')
1396+
expect(lexicalField?.root?.children[0].fields.someTextRequired).toEqual('test')
13951397
}).toPass({
13961398
timeout: POLL_TOPASS_TIMEOUT,
13971399
})
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import { expect, test } from '@playwright/test'
2+
import { AdminUrlUtil } from 'helpers/adminUrlUtil.js'
3+
import { reInitializeDB } from 'helpers/reInitializeDB.js'
4+
import { lexicalFullyFeaturedSlug } from 'lexical/slugs.js'
5+
import path from 'path'
6+
import { fileURLToPath } from 'url'
7+
8+
import { ensureCompilationIsDone } from '../../../helpers.js'
9+
import { initPayloadE2ENoConfig } from '../../../helpers/initPayloadE2ENoConfig.js'
10+
import { TEST_TIMEOUT_LONG } from '../../../playwright.config.js'
11+
import { LexicalHelpers } from './utils.js'
12+
const filename = fileURLToPath(import.meta.url)
13+
const currentFolder = path.dirname(filename)
14+
const dirname = path.resolve(currentFolder, '../../')
15+
16+
const { beforeAll, beforeEach, describe } = test
17+
18+
// Unlike the other suites, this one runs in parallel, as they run on the `lexical-fully-featured/create` URL and are "pure" tests
19+
test.describe.configure({ mode: 'parallel' })
20+
21+
const { serverURL } = await initPayloadE2ENoConfig({
22+
dirname,
23+
})
24+
25+
describe('Lexical Fully Featured', () => {
26+
beforeAll(async ({ browser }, testInfo) => {
27+
testInfo.setTimeout(TEST_TIMEOUT_LONG)
28+
process.env.SEED_IN_CONFIG_ONINIT = 'false' // Makes it so the payload config onInit seed is not run. Otherwise, the seed would be run unnecessarily twice for the initial test run - once for beforeEach and once for onInit
29+
const page = await browser.newPage()
30+
await ensureCompilationIsDone({ page, serverURL })
31+
await page.close()
32+
})
33+
beforeEach(async ({ page }) => {
34+
await reInitializeDB({
35+
serverURL,
36+
snapshotKey: 'fieldsTest',
37+
uploadsDir: [
38+
path.resolve(dirname, './collections/Upload/uploads'),
39+
path.resolve(dirname, './collections/Upload2/uploads2'),
40+
],
41+
})
42+
const url = new AdminUrlUtil(serverURL, lexicalFullyFeaturedSlug)
43+
const lexical = new LexicalHelpers(page)
44+
await page.goto(url.create)
45+
await lexical.editor.first().focus()
46+
})
47+
test('prevent extra paragraph when inserting decorator blocks like blocks or upload node', async ({
48+
page,
49+
}) => {
50+
const lexical = new LexicalHelpers(page)
51+
await lexical.slashCommand('block')
52+
await lexical.slashCommand('relationship')
53+
await lexical.drawer.locator('.list-drawer__header').getByText('Create New').click()
54+
await lexical.save('drawer')
55+
await expect(lexical.decorator).toHaveCount(2)
56+
await lexical.slashCommand('upload')
57+
await lexical.drawer.locator('.list-drawer__header').getByText('Create New').click()
58+
await lexical.drawer.getByText('Paste URL').click()
59+
await lexical.drawer
60+
.locator('.file-field__remote-file')
61+
.fill('https://payloadcms.com/images/universal-truth.jpg')
62+
await lexical.drawer.getByText('Add file').click()
63+
await lexical.save('drawer')
64+
await expect(lexical.decorator).toHaveCount(3)
65+
const paragraph = lexical.editor.locator('> p')
66+
await expect(paragraph).toHaveText('')
67+
})
68+
})
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import type { CollectionConfig } from 'payload'
2+
3+
import {
4+
BlocksFeature,
5+
EXPERIMENTAL_TableFeature,
6+
FixedToolbarFeature,
7+
lexicalEditor,
8+
TreeViewFeature,
9+
} from '@payloadcms/richtext-lexical'
10+
11+
import { lexicalFullyFeaturedSlug } from '../../slugs.js'
12+
13+
export const LexicalFullyFeatured: CollectionConfig = {
14+
slug: lexicalFullyFeaturedSlug,
15+
labels: {
16+
singular: 'Lexical Fully Featured',
17+
plural: 'Lexical Fully Featured',
18+
},
19+
fields: [
20+
{
21+
name: 'richText',
22+
type: 'richText',
23+
editor: lexicalEditor({
24+
features: ({ defaultFeatures }) => [
25+
...defaultFeatures,
26+
TreeViewFeature(),
27+
FixedToolbarFeature(),
28+
EXPERIMENTAL_TableFeature(),
29+
BlocksFeature({
30+
blocks: [
31+
{
32+
slug: 'myBlock',
33+
fields: [
34+
{
35+
name: 'someText',
36+
type: 'text',
37+
},
38+
],
39+
},
40+
],
41+
inlineBlocks: [
42+
{
43+
slug: 'myInlineBlock',
44+
fields: [
45+
{
46+
name: 'someText',
47+
type: 'text',
48+
},
49+
],
50+
},
51+
],
52+
}),
53+
],
54+
}),
55+
},
56+
],
57+
}

0 commit comments

Comments
 (0)