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
Basic Context Menu (behind Switch) #4285
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
size-limit report 📦
|
Hey, @tylerjbainbridge, any plans for updating this one, since the generic part is now merged? |
Thanks for the reminder. I'll try to get this merged today. |
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.
Wasn't sure, if I should review, since it's still a draft, but this looks pretty complete to me to what what's in the PR description
selectedIndex: number | null; | ||
selectOptionAndCleanUp: (option: TOption) => void; | ||
setHighlightedIndex: (index: number) => void; | ||
options: Array<TOption>; |
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.
Are we planning to add the ability to have nested options?
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.
Not yet, but good call-out.
All LexicalMenu's are flat right now but I can see this being helpful in the future.
(event: MouseEvent) => { | ||
event.preventDefault(); | ||
openNodeMenu({ | ||
getRect: () => new DOMRect(event.clientX, event.clientY, 1, 1), |
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.
1, 1?
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.
Kind of a hack, but the menus are positioned using a div (in this case 1px by 1px) that is replaced with React portal's content. I can add enums though.
onOptionClick={(option: ContextMenuOption, index: number) => { | ||
setHighlightedIndex(index); | ||
selectOptionAndCleanUp(option); | ||
}} | ||
onOptionMouseEnter={(index: number) => { | ||
setHighlightedIndex(index); | ||
}} |
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.
I think this is fine, alternatively we can tell the user which one is highlighted and request what action to take beforehand, more like a framework-like approach instead of a library, but what you have is good imo
Basic working context menu built with LexicalMenus so there's built in support for keyboard interaction, positioning, and an identical API to typeaheads, autoembeds, etc.
WIP demo
Screen.Recording.2023-04-07.at.3.09.20.PM.mov