-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
fix: Single anchor element per LexicalTypeaheadMenuPlugin instance #2768
fix: Single anchor element per LexicalTypeaheadMenuPlugin instance #2768
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Thanks for the contribution @Morphyish! One small fix, but I'll let @tylerjbainbridge review this fully as this was his component.
- Move useEffect hook to position anchorElement to LexicalTypeaheadMenuPlugin - Replace anchorElementRef prop with anchorElement
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 makes sense to me, good idea!
Fixes #2755
Creating the anchorElement in the Plugin itself and passing it down to the
ShortcutTypeahead
avoid creating a lot of unnecessary nodes that are never removed.Every plugin that relies on
LexicalTypeaheadMenuPlugin
will get its anchor element that will then be properly updated with the new position, as it should.Please note that I had to remove the
startTransition
when setting theresolution
tonull
.The transition was causing the
/
menu to flicker, but not the mentions. I'm not sure if it's a specific implementation issue in either of those playground plugins, but as hiding the menu should not cause any unnecessary loading state on the UI, it felt safe to remove.Demo
Lexical.Playground.-.Google.Chrome.2022-08-05.11-31-58_Trim.mp4