🐛 Fixes list item link pasting behavior #1506
Conversation
Original Issue: Editor Bullet into Link #22822 (TryGhost/Ghost#22822?) Avoid creating an embed/bookmark when pasting a link inside a list item, creating a simple link inside the list item.
WalkthroughThe changes update the paste handling logic for URLs in the Koenig Lexical editor. The logic now includes explicit checks to determine if the current node is a card shortcut (text starting with a slash) or a list item. If a card shortcut is detected, the conversion of pasted URLs into embeds, bookmarks, or links is prevented. If the node is a list item, pasting a URL results in the URL being wrapped as a hyperlink within the list item, rather than being converted into an embed or bookmark. Only if neither condition applies does the logic proceed to convert the URL into an embed, bookmark, or link. Corresponding end-to-end tests were added to verify that pasting a URL inside a list item results in a hyperlink and not an embed or bookmark. The test suite also imports a new utility function, Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/koenig-lexical/src/plugins/KoenigBehaviourPlugin.jsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "react-app" to extend from. Please check that the name of the config is correct. The config "react-app" was referenced from the config file in "/packages/koenig-lexical/.eslintrc.cjs". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. packages/koenig-lexical/test/e2e/paste-behaviour.test.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "react-app" to extend from. Please check that the name of the config is correct. The config "react-app" was referenced from the config file in "/packages/koenig-lexical/.eslintrc.cjs". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/koenig-lexical/src/plugins/KoenigBehaviourPlugin.jsx (1)
1302-1302: Consider using optional chaining for safer node accessThe code could use optional chaining to prevent potential null reference errors when accessing the node's text content.
- const isNodeCardShortcut = node && node.getTextContent().startsWith('/'); + const isNodeCardShortcut = node && node?.getTextContent().startsWith('/');🧰 Tools
🪛 Biome (1.9.4)
[error] 1302-1302: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/koenig-lexical/src/plugins/KoenigBehaviourPlugin.jsx(1 hunks)packages/koenig-lexical/test/e2e/paste-behaviour.test.js(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/koenig-lexical/test/e2e/paste-behaviour.test.js (1)
packages/koenig-lexical/test/utils/e2e.js (5)
focusEditor(58-61)pasteLexical(315-317)paste(283-305)assertHTML(63-106)html(188-197)
🪛 Biome (1.9.4)
packages/koenig-lexical/src/plugins/KoenigBehaviourPlugin.jsx
[error] 1302-1302: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (2)
packages/koenig-lexical/test/e2e/paste-behaviour.test.js (1)
627-671: Well-structured test for the list item URL pasting behavior!This test case properly verifies the fixed behavior from the PR objectives: that pasting a URL inside a list item results in a simple hyperlink rather than an embed/bookmark card. The test is well-structured with clear setup, execution, and verification steps.
packages/koenig-lexical/src/plugins/KoenigBehaviourPlugin.jsx (1)
1300-1319: Good implementation of the list item link pasting fix!The changes correctly implement the fix described in the PR by:
- Clearly identifying when a node is a list item
- Creating a standard link node instead of an embed/bookmark when pasting URLs in list items
- Properly appending the link to the list item
This fix ensures that pasted URLs remain as simple hyperlinks within list items rather than being converted to embed cards outside the list.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1302-1302: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
There was a problem hiding this comment.
@kaueburiti thanks for taking a look at this! 🙌
Looking at the changes, I think the handling is done in the wrong place here. I'd prefer to see the behaviour inside the PASTE_LINK_COMMAND handler.
With the current approach we lose the ability to paste a URL onto highlighted text inside a list and have that text converted into a link (adding a test to confirm this would be helpful 🙏). If the handling is instead inside the PASTE_LINK_COMMAND handler then we can be more selective over the behaviour, pasting a link directly when no text is selected when inside a list and keeping the convert-text-to-link behaviour when there is a selection.
2 participants
Context
This PR responds to the issue Editor Bullet into Link #22822 posted on the Ghost repository.
The Problem
When pasting a link inside a list item, we get an unexpected behavior:
The Solution