cytoscape / cytoscape.js-navigator

Bird's eye view pan and zoom control for Cytoscape.js.
MIT License
67 stars 42 forks source link

Navigator should not destroy its container when it hasn't create it #11

Closed MissChocoe closed 9 years ago

MissChocoe commented 9 years ago

When calling "destroy" on the navigator, I don't expect the container that I've specified for it to go away. This prevent me from putting a new instance there.

bumbu commented 9 years ago

Hi, can you please try if replacing destroy method with

    destroy: function () {
      // If container is not created by navigator then just empty it
      if (this.options.container) {
        this.$panel.empty()
      } else {
        this.$panel.remove()
      }
      this.$element.removeData('navigator')
    }

will work for you?

I agree that it should be the default behavior but since this plugin is online for quite some time it may be so that someone relies on the old behavior. So I'm not sure if we can just update the behavior or we need a version bump. @maxkfranz what do you think about that?

MissChocoe commented 9 years ago

It definitely works for me.

Not sure if people would rely on the previous behavior when they provide a container... It is not really usual for a plugin to destroy an element that it hasn't created I think.

Is there a way to make the navigator stop listening to graph change for a while? I need to do that when I load a new graph... For now, what I do is destroy it and recreate it when the new graph is ready.

bumbu commented 9 years ago

There is no option to stop listening events from cytoscape instance.

maxkfranz commented 9 years ago

Would it be possible to unlisten via .off() temporarily?

bumbu commented 9 years ago

I don't se a problem to add a pause/unpause method that would call on/off methods.