The latest updates on your projects. Learn more about Vercel for Git ↗︎

@etrepum This is ready to skim over. Playground is produced fine. I'll fix the types tomorrow. Most of the code is canonical as per the FloatingUI example for a context menu.

I had to make some creative choices for the conditional rendering and the editor.read for the underlying node, since the examples use pre-defined children to the main component and wanted to keep the main render components exposed to be overridden.

Welcoming interface comments.

The things I'd do differently in the interface:

• Rename showOn and onSelect to $showOn and $onSelect to make it clear that they are called with editor context (I think the onSelect implementation is currently broken in this refactor for this reason, the delete node code depends on editor context)
• Unify options and conditionalOptions, there doesn't really seem to be a reason to have them be separate and by having a single array you can control the exact order

As far as the implementation goes, just some things I'd simplify:

• I don't think it's useful to bind showOn and onSelect to this because the ContextMenuOption class isn't exported and there's not any particularly useful state in there. If they wanted it bound to something they would probably prefer the object that it was defined in (per the constructor) and the caller could do that binding themselves if they really wanted to.
• I generally don't like cloneElement, especially when not used to facilitate an eDSL where the user is specifying the children. This code would be simpler if the items array was props and then you can do the <ContextMenuItem {...getItemProps({ ...props, onClick() { props.onClick(); setIsOpen(false); }, /* … */ } })} /> instead of going through the trouble to create the JSX to turn it back into props to create the JSX again.

The things I'd do differently in the interface:

• Rename showOn and onSelect to $showOn and $onSelect to make it clear that they are called with editor context (I think the onSelect implementation is currently broken in this refactor for this reason, the delete node code depends on editor context)

• Unify options and conditionalOptions, there doesn't really seem to be a reason to have them be separate and by having a single array you can control the exact order

As far as the implementation goes, just some things I'd simplify:

• I don't think it's useful to bind showOn and onSelect to this because the ContextMenuOption class isn't exported and there's not any particularly useful state in there. If they wanted it bound to something they would probably prefer the object that it was defined in (per the constructor) and the caller could do that binding themselves if they really wanted to.

• I generally don't like cloneElement, especially when not used to facilitate an eDSL where the user is specifying the children. This code would be simpler if the items array was props and then you can do the <ContextMenuItem {...getItemProps({ ...props, onClick() { props.onClick(); setIsOpen(false); }, /* … */ } })} /> instead of going through the trouble to create the JSX to turn it back into props to create the JSX again.

Great! Thanks! I'll fix those up. The reason I split the conditional and default options was, that conditional options was intended as on object offering multiple items per 1 rule eval, vs 1 rule for 1 option, but forgot to do it.
I'll also think of moving the Item component to the main menu, will help me when I unify some of the internals with the other menus for later.

One last question I have is, should I add a new plugin in the react package and export ContextMenu2 and make this opt-in and make the breaking change later when we have all menus migrated vs now.

Maybe call it NodeContextMenu and deprecate the other one?

It seems that there's something in here that causes the collab tests to fail a lot more frequently, I restarted the tests and there are still a bunch of failures

It seems that there's something in here that causes the collab tests to fail a lot more frequently, I restarted the tests and there are still a bunch of failures

Yeah, consistently pass locally, but not in CI, will play with a diff playwright syntax, maybe that's the key. Will redo the tests tonight.

I think this looks good but we should add some API docs to LexicalNodeContextMenuPlugin.

Maybe the ContextMenu export should be renamed to NodeContextMenuPlugin? It doesn't really follow the naming convention of the other plugin modules where something with the same name as the module (sans the Lexical prefix) is exported

Happy to rename it, I want to complete the LexicalMenu migration for all menus and then I'll do API docs in a separate PR.
Also, thanks for the tests fix.