iVis-at-Bilkent / cytoscape.js-context-menus

A Cytoscape.js extension to provide context menu around elements and core instance
MIT License
86 stars 41 forks source link

Add an option to append context menu to cytoscape container #50

Closed rparent closed 4 years ago

rparent commented 4 years ago

Hello,

This PR adds a inCyContainer option for context menu, to append context menu to cytoscape container instead of body. This can be useful in some cases, e.g. when cytoscape container is in a modal or in Full-screen mode.

I found out that an attempt was made some time ago (https://github.com/iVis-at-Bilkent/cytoscape.js-context-menus/pull/13) but it didn't complete. Maybe we can give it another try!

hasanbalci commented 4 years ago

Hi @rparent! Thanks for your contribution. We make our development in unstable branch and then merge it to master branch and publish in npm periodically. Could you please make your changes in unstable branch? Unstable branch currently doesn't use jQuery and has submenu support and we will make a new release soon.

Moreover, as discussed in #13, what do you think about appending context menu only to cy container?

rparent commented 4 years ago

Hi @hasanbalci and thanks for the review!

No problem I will open a new PR on unstable branch. About appending context menu only to cy container without adding an extra-option, I think it is a good idea indeed. I can't think of a case where it would be better to append it to the body.