[lexical-react] Chore: Remove confusing return value#7607
[lexical-react] Chore: Remove confusing return value#7607etrepum merged 2 commits intofacebook:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
etrepum
left a comment
There was a problem hiding this comment.
I think the correct fix may be to remove the return true; - what is the actual behavior that you are trying to fix with this PR?
Hey @etrepum , we are inside a command handler, so the return value ( |
|
I understand how command handlers work, but I think that the most appropriate fix for this listener is probably to remove the return statement altogether. It looks like what you're trying to do is fix the code to make sense (you are correct, there is something about this code that doesn't make sense), but not considering what the effect of this listener is supposed to be. What this command is doing is toggling all link nodes in a selection, there is no clear reason why you would only want it to toggle the first AutoLinkNode and then stop. Returning true would prevent further AutoLinkNodes from being toggled, and also avoid calling any other TOGGLE_LINK_COMMAND listener that was registered at a lower priority or registered after at the same priority. |
|
Either way, if this does fix a behavior issue, then we should describe what issue is being fixed and add an e2e test to show that it continues to work as expected. |
|
You're right, I totally missed the fact that this command could be called on a selection with more than one matching node. Thanks for your feedback! |
|
Can you fix up the description of this PR to match the current implementation? Looks good to merge otherwise! |
|
Done! |
Description
In the AutoLinkPlugin, specifically in the
TOGGLE_LINK_COMMANDhandler, a value is returned from aforEachloop, which has no effect, is confusing and doesn't make much sense.The return value was removed.