[lexical-react][lexical-playground] Feature: Makes an inline node more easily linkable #7359
Conversation
sescandell
requested review from
zurfyx,
fantactuka,
acywatson,
Fetz,
ivailop7,
Sahejkm and
potatowagon
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
the
CLA Signed
There was a problem hiding this comment.
Seems like a great start, it would be nice to clean up the functionality so that it's more correct in the general case. I would recommend adding some function(s) to @lexical/selection to facilitate the shared logic between these different modules so that this code is simpler, and that it's easier to do these things elsewhere too
packages/lexical-playground/src/plugins/FloatingLinkEditorPlugin/index.tsx
Outdated
Show resolved
Hide resolved
packages/lexical-playground/src/plugins/FloatingLinkEditorPlugin/index.tsx
Show resolved
Hide resolved
packages/lexical-playground/src/plugins/FloatingLinkEditorPlugin/index.tsx
Outdated
Show resolved
Hide resolved
| .filter( | ||
| (n) => $isTextNode(n) || $isElementNode(n) || $isDecoratorNode<T>(n), | ||
| ); |
There was a problem hiding this comment.
What's the purpose of this? To avoid LineBreakNode? This filter is why the code fails typecheck because the guard isn't implied with this version of TypeScript.
There was a problem hiding this comment.
Actually, this update wasn’t intended to be reviewed yet — I’m still working on it. Sorry, I shouldn’t have pushed it.
To answer your question: no, I can’t limit it to $isTextNode and $isElementNode because, for the primary objective of this PR, this wouldn’t work for ImageComponent (which is a DecoratorNode).
The goal is not to restrict it to specific blocks. In the initial implementation, getSelectedNode was designed to return either a TextNode or an ElementNode, and I wanted to stay as close as possible to that. However, in a NodeSelection, it can actually return more. For formatting purposes (e.g., links, formatting, toolbar), DecoratorNode should be allowed to support components like ImageComponent.
That said, after looking at your comment (and the code), I realize that, aside from LineBreakNode, there aren’t any other relevant types of elements.
I’m still working on this. I’ll mention you once I think it’s ready for review.
Thanks!
There was a problem hiding this comment.
Hi @etrepum,
I’ve removed the filter and updated the return type of getSelectedNode.
In previous usages, all cases either already checked the type of the node before using it or didn’t require the check at all.
Let me know your thoughts.
Description
Currently if you want to put a link around an image, you need to be clever and "select" around the image. It works in a way, but is not easy to use neither well presented.
Thanks to these changes, we globally allow to put a link on an inline node such as the ImageNode from the playground
Before
Nothing to show
After