-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Revert "[lexical-playground] Fix: in playground show component-menu w… #6558
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
size-limit report 📦
|
Can you put together a repro that shows what goes wrong with this approach? It fixed all of the positioning issues that we could find, this revert is an OSS regression |
Maybe a complete rewrite of the floating elements - context menu and typeahead/mentions/hashtags suggestions is worth it, maybe using something like https://floating-ui.com/ that would handle all of the render details correctly. All of those popovers are playground-only, so it should be OK relying on a 3rd party package for the rendering of these two. Just food for thought. |
Yeah, Floating UI is what I used in the past for doing this sort of thing and it worked well. I don't have a strong opinion on whether we should have a dependency handle floating UI or have an in-tree implementation without known issues but we should certainly not have an in-tree implementation that we know is incorrect for the cases that we demonstrate. |
reintroducing the changes back in #6566 but with gating so we can investigate internally and fix intern |
Breaks internally, might need to revise the approach or our internal code...
…hen scroll (#6510)"
This reverts commit fd7a93d.
Description
Describe the changes in this pull request
Closes #
Test plan
Before
Insert relevant screenshots/recordings/automated-tests
After
Insert relevant screenshots/recordings/automated-tests