-
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
[lexical-react] menu positioning: Unrevert PR6510 but with gating #6566
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
size-limit report 📦
|
@@ -482,6 +482,7 @@ export function useMenuAnchorRef( | |||
setResolution: (r: MenuResolution | null) => void, | |||
className?: string, | |||
parent: HTMLElement = document.body, | |||
shouldIncludePageYOffset: boolean = true, |
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.
Prob just fine for a temporary use case but otherwise I'd be keen a suffix __EXPERIMENTAL or __DEPRECATED so that it doesn't leak into product code in OSS which is harder to deprecate
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.
suffixed with __EXPERIMENTAL
Description
#6510 is correct but it broke implementations internally
hence it got reverted to mitigate the internal emergency: #6558
here, #6510 is reintroduced but gated behind the shouldIncludePageYOffset option to aid investigations internally and to fix internal to be compatible
Test plan
shouldIncludePageYOffset = true
Screen.Recording.2024-08-28.at.12.56.41.AM.mov
shouldIncludePageYOffset = false
Screen.Recording.2024-08-28.at.12.56.27.AM.mov