[lexical-link] Fix bug when can't remove link formatting from autolink #2
Conversation
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
|
Clone of the PR facebook/lexical#6306 |
|
My review is in progress 📖 - I will have feedback for you in a few minutes! |
There was a problem hiding this comment.
I have reviewed your code and found 2 potential issues.
To discuss my individual comments that I have added, tag me in replies using @korbit-ai.
Please react with a 👍 to my comments that you find helpful and a 👎 to those you find unhelpful - this will help me learn and improve as we collaborate.
| updateDOM( | ||
| prevNode: AutoLinkNode, | ||
| anchor: LinkHTMLElementType, | ||
| config: EditorConfig, | ||
| ): boolean { | ||
| return ( | ||
| super.updateDOM(prevNode, anchor, config) || | ||
| prevNode.__isUnlinked !== this.__isUnlinked | ||
| ); | ||
| } |
There was a problem hiding this comment.
The updateDOM method in AutoLinkNode needs to be enhanced to handle the transition between anchor and span elements when the isUnlinked state changes. Currently, it only checks if the isUnlinked state has changed, but doesn't update the DOM structure accordingly. Consider adding logic to replace the existing element with the appropriate new element (anchor or span) when the isUnlinked state changes.
| (autoLinkNode && | ||
| (!autoLinkNode.is(focusAutoLinkNode) || | ||
| autoLinkNode.getIsUnlinked())) |
There was a problem hiding this comment.
The current implementation doesn't fully account for unlinked AutoLinkNodes. When an AutoLinkNode is unlinked, it should be treated as a regular text node for the purposes of the link editor. Consider modifying the condition to treat unlinked AutoLinkNodes as non-links, ensuring that the link editor behaves correctly for all possible link states.
|
@coderabbitai review |
Actions performedReview triggered.
|
|
Caution Review failedThe pull request is closed. WalkthroughThe recent changes enhance the functionality of link management in the Lexical editor, introducing the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Editor
participant AutoLinkNode
User->>Editor: Insert link
Editor->>AutoLinkNode: Create AutoLinkNode
AutoLinkNode-->>Editor: Return link node
User->>Editor: Click link
Editor->>AutoLinkNode: Toggle unlinking
AutoLinkNode-->>Editor: Update state
User->>Editor: Reinsert link
Editor->>AutoLinkNode: Create AutoLinkNode
AutoLinkNode-->>Editor: Return updated link node
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
|
/review |
PR Reviewer Guide 🔍(Review updated until commit b8c6ec3)
|
|
Persistent review updated to latest commit b8c6ec3 |
|
@coderabbitai review |
Actions performedReview triggered.
|
This PR fixes a bug where a user cannot remove link formatting (unlink) from links created by AutoLinkNode. However, it works for links created by LinkNode. The proposal is to add a flag property in AutoLinkNode to indicate if the link was unlinked. If so, render the link as regular text, keeping styles. The hidden auto-link can become a link again via the Toolbar Link button.
Closes #5607
Test plan
Before
Screen.Recording.2024-06-13.at.15.18.13.mov
After
Screen.Recording.2024-06-13.at.15.20.36.mov
Description by Korbit AI
Note
This feature is in early access. You can enable or disable it in the Korbit Console.
[lexical-link] Fix bug when can't remove link formatting from autolink
This PR introduces a fix for the issue where link formatting could not be removed from an autolink, by adding support for an
isUnlinkedattribute in theAutoLinkNodeclass and updating relevant methods and tests.The changes were made to address a bug where users were unable to remove link formatting from autolinks. The
isUnlinkedattribute allows theAutoLinkNodeto be rendered as a<span>instead of an<a>element when unlinked, and theupdateDOMmethod was updated to handle this new attribute. Additionally, new tests were added to ensure the functionality works as expected.Summary by CodeRabbit
New Features
AutoLinkNodeclass for enhanced hyperlink functionality.Bug Fixes
Tests
AutoLinkNodeclass.Documentation