-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix(richtext-lexical): prevent extra paragraph when inserting blocks or uploadNodes. Add preemptive selection normalization #12077
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d8f8d9d
cea7ad8
fbb8491
209d370
05538d0
293cd46
fcfbace
9d8cb58
ec9856c
f000130
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,22 +56,15 @@ export const BlocksPlugin: PluginComponent = () => { | |
|
||
if ($isRangeSelection(selection)) { | ||
const blockNode = $createBlockNode(payload) | ||
// Insert blocks node BEFORE potentially removing focusNode, as $insertNodeToNearestRoot errors if the focusNode doesn't exist | ||
$insertNodeToNearestRoot(blockNode) | ||
|
||
// we need to get the focus node before inserting the block node, as $insertNodeToNearestRoot can change the focus node | ||
const { focus } = selection | ||
const focusNode = focus.getNode() | ||
// Insert blocks node BEFORE potentially removing focusNode, as $insertNodeToNearestRoot errors if the focusNode doesn't exist | ||
$insertNodeToNearestRoot(blockNode) | ||
|
||
// First, delete currently selected node if it's an empty paragraph and if there are sufficient | ||
// paragraph nodes (more than 1) left in the parent node, so that we don't "trap" the user | ||
if ( | ||
$isParagraphNode(focusNode) && | ||
focusNode.getTextContentSize() === 0 && | ||
focusNode | ||
.getParentOrThrow() | ||
.getChildren() | ||
.filter((node) => $isParagraphNode(node)).length > 1 | ||
) { | ||
// Delete the node it it's an empty paragraph | ||
if ($isParagraphNode(focusNode) && !focusNode.__first) { | ||
focusNode.remove() | ||
Comment on lines
-65
to
68
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since PR #10530 this is no longer necessary |
||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,13 +4,7 @@ import { LexicalErrorBoundary } from '@lexical/react/LexicalErrorBoundary.js' | |
import { HistoryPlugin } from '@lexical/react/LexicalHistoryPlugin.js' | ||
import { OnChangePlugin } from '@lexical/react/LexicalOnChangePlugin.js' | ||
import { RichTextPlugin } from '@lexical/react/LexicalRichTextPlugin.js' | ||
import { | ||
$createParagraphNode, | ||
$getRoot, | ||
BLUR_COMMAND, | ||
COMMAND_PRIORITY_LOW, | ||
FOCUS_COMMAND, | ||
} from 'lexical' | ||
import { BLUR_COMMAND, COMMAND_PRIORITY_LOW, FOCUS_COMMAND } from 'lexical' | ||
import * as React from 'react' | ||
import { useEffect, useState } from 'react' | ||
|
||
|
@@ -24,6 +18,7 @@ import { AddBlockHandlePlugin } from './plugins/handles/AddBlockHandlePlugin/ind | |
import { DraggableBlockPlugin } from './plugins/handles/DraggableBlockPlugin/index.js' | ||
import { InsertParagraphAtEndPlugin } from './plugins/InsertParagraphAtEnd/index.js' | ||
import { MarkdownShortcutPlugin } from './plugins/MarkdownShortcut/index.js' | ||
import { NormalizeSelectionPlugin } from './plugins/NormalizeSelection/index.js' | ||
import { SlashMenuPlugin } from './plugins/SlashMenu/index.js' | ||
import { TextPlugin } from './plugins/TextPlugin/index.js' | ||
import { LexicalContentEditable } from './ui/ContentEditable.js' | ||
|
@@ -112,6 +107,7 @@ export const LexicalEditor: React.FC< | |
} | ||
ErrorBoundary={LexicalErrorBoundary} | ||
/> | ||
<NormalizeSelectionPlugin /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The issue was resolved with the changes above, so this isn't strictly necessary, but I decided to add it to make the editor more robust and prevent errors in the future. Additionally, I believe this will fix a bug in the email builder plugin. |
||
<InsertParagraphAtEndPlugin /> | ||
<DecoratorPlugin /> | ||
<TextPlugin features={editorConfig.features} /> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
import { useLexicalComposerContext } from '@lexical/react/LexicalComposerContext' | ||
import { $getSelection, $isRangeSelection, RootNode } from 'lexical' | ||
import { useEffect } from 'react' | ||
|
||
/** | ||
* By default, Lexical throws an error if the selection ends in deleted nodes. | ||
* This is very aggressive considering there are reasons why this can happen | ||
* outside of Payload's control (custom features or conflicting features, for example). | ||
* In the case of selections on nonexistent nodes, this plugin moves the selection to | ||
* the end of the editor and displays a warning instead of an error. | ||
*/ | ||
export function NormalizeSelectionPlugin() { | ||
const [editor] = useLexicalComposerContext() | ||
|
||
useEffect(() => { | ||
return editor.registerNodeTransform(RootNode, (root) => { | ||
const selection = $getSelection() | ||
if ($isRangeSelection(selection)) { | ||
const anchorNode = selection.anchor.getNode() | ||
const focusNode = selection.focus.getNode() | ||
if (!anchorNode.isAttached() || !focusNode.isAttached()) { | ||
root.selectEnd() | ||
// eslint-disable-next-line no-console | ||
console.warn( | ||
'updateEditor: selection has been moved to the end of the editor because the previously selected nodes have been removed and ' + | ||
"selection wasn't moved to another node. Ensure selection changes after removing/replacing a selected node.", | ||
) | ||
} | ||
} | ||
return false | ||
}) | ||
}, [editor]) | ||
|
||
return null | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
import { expect, test } from '@playwright/test' | ||
import { AdminUrlUtil } from 'helpers/adminUrlUtil.js' | ||
import { reInitializeDB } from 'helpers/reInitializeDB.js' | ||
import { lexicalFullyFeaturedSlug } from 'lexical/slugs.js' | ||
import path from 'path' | ||
import { fileURLToPath } from 'url' | ||
|
||
import { ensureCompilationIsDone } from '../../../helpers.js' | ||
import { initPayloadE2ENoConfig } from '../../../helpers/initPayloadE2ENoConfig.js' | ||
import { TEST_TIMEOUT_LONG } from '../../../playwright.config.js' | ||
import { LexicalHelpers } from './utils.js' | ||
const filename = fileURLToPath(import.meta.url) | ||
const currentFolder = path.dirname(filename) | ||
const dirname = path.resolve(currentFolder, '../../') | ||
|
||
const { beforeAll, beforeEach, describe } = test | ||
|
||
// Unlike the other suites, this one runs in parallel, as they run on the `lexical-fully-featured/create` URL and are "pure" tests | ||
test.describe.configure({ mode: 'parallel' }) | ||
|
||
const { serverURL } = await initPayloadE2ENoConfig({ | ||
dirname, | ||
}) | ||
|
||
describe('Lexical Fully Featured', () => { | ||
beforeAll(async ({ browser }, testInfo) => { | ||
testInfo.setTimeout(TEST_TIMEOUT_LONG) | ||
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 | ||
const page = await browser.newPage() | ||
await ensureCompilationIsDone({ page, serverURL }) | ||
await page.close() | ||
}) | ||
beforeEach(async ({ page }) => { | ||
await reInitializeDB({ | ||
serverURL, | ||
snapshotKey: 'fieldsTest', | ||
uploadsDir: [ | ||
path.resolve(dirname, './collections/Upload/uploads'), | ||
path.resolve(dirname, './collections/Upload2/uploads2'), | ||
], | ||
}) | ||
const url = new AdminUrlUtil(serverURL, lexicalFullyFeaturedSlug) | ||
const lexical = new LexicalHelpers(page) | ||
await page.goto(url.create) | ||
await lexical.editor.first().focus() | ||
}) | ||
test('prevent extra paragraph when inserting decorator blocks like blocks or upload node', async ({ | ||
page, | ||
}) => { | ||
const lexical = new LexicalHelpers(page) | ||
await lexical.slashCommand('block') | ||
await lexical.slashCommand('relationship') | ||
await lexical.drawer.locator('.list-drawer__header').getByText('Create New').click() | ||
await lexical.save('drawer') | ||
await expect(lexical.decorator).toHaveCount(2) | ||
await lexical.slashCommand('upload') | ||
await lexical.drawer.locator('.list-drawer__header').getByText('Create New').click() | ||
await lexical.drawer.getByText('Paste URL').click() | ||
await lexical.drawer | ||
.locator('.file-field__remote-file') | ||
.fill('https://payloadcms.com/images/universal-truth.jpg') | ||
await lexical.drawer.getByText('Add file').click() | ||
await lexical.save('drawer') | ||
await expect(lexical.decorator).toHaveCount(3) | ||
const paragraph = lexical.editor.locator('> p') | ||
await expect(paragraph).toHaveText('') | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
import type { CollectionConfig } from 'payload' | ||
|
||
import { | ||
BlocksFeature, | ||
EXPERIMENTAL_TableFeature, | ||
FixedToolbarFeature, | ||
lexicalEditor, | ||
TreeViewFeature, | ||
} from '@payloadcms/richtext-lexical' | ||
|
||
import { lexicalFullyFeaturedSlug } from '../../slugs.js' | ||
|
||
export const LexicalFullyFeatured: CollectionConfig = { | ||
slug: lexicalFullyFeaturedSlug, | ||
labels: { | ||
singular: 'Lexical Fully Featured', | ||
plural: 'Lexical Fully Featured', | ||
}, | ||
fields: [ | ||
{ | ||
name: 'richText', | ||
type: 'richText', | ||
editor: lexicalEditor({ | ||
features: ({ defaultFeatures }) => [ | ||
...defaultFeatures, | ||
TreeViewFeature(), | ||
FixedToolbarFeature(), | ||
EXPERIMENTAL_TableFeature(), | ||
BlocksFeature({ | ||
blocks: [ | ||
{ | ||
slug: 'myBlock', | ||
fields: [ | ||
{ | ||
name: 'someText', | ||
type: 'text', | ||
}, | ||
], | ||
}, | ||
], | ||
inlineBlocks: [ | ||
{ | ||
slug: 'myInlineBlock', | ||
fields: [ | ||
{ | ||
name: 'someText', | ||
type: 'text', | ||
}, | ||
], | ||
}, | ||
], | ||
}), | ||
], | ||
}), | ||
}, | ||
], | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the key point of the fix. We still insert before removing focusNode, but after getting focusNode, since
$insertNodeToNearestRoot
can change the selection.