vega icon indicating copy to clipboard operation
vega copied to clipboard

Vega editor / apps re-register hover listeners

Open Etzix opened this issue 7 years ago • 6 comments

Posted this on the react-vega board at first, but noticed the bug is actually in Vega, and can be found in the Vega examples.

My problem is that whenever you navigate your webpage, interact with buttons etc, another event listener for mouseover and mouseout is added. over a short period of time this makes your graph slow down significantly.

The bug can be produced on Vegas own example code in Vegas editor. See: https://vega.github.io/editor/#/examples/vega/county-unemployment Press the "Renderer: Canvas/SVG" button a few times and watch in the dev tools how another listener is added each time. After a while it will start to get really slow.

Is there a solution to this? Are the hover functions implemented the wrong way in the demo?

Etzix avatar Dec 19 '18 12:12 Etzix

Thank you for catching and filing the issue!

Technically this is not a Vega bug; for example, if you run the local web tests in vega-lib, the error does not arise. Rather this is a "misuse" of the Vega API by third party apps. I'm not familiar with react-vega, but I believe I have isolated the bug that is causing this problem in the online Vega editor.

The editor is reusing the same View instance, but re-invoking the hover method on every view update. The result is that hover listeners are being added every time the view is updated via the code path at renderer/renderer.tsx#L105 in the vega/editor repo. (As a minor note, this code could also be made slightly more efficient by calling the renderer function before calling initialize.)

My suggestion is to update the appropriate projects to separate view initialization (e.g., calling initialize and hover methods) so that those methods are only invoked once for a given View object, then use a separate code path for view updates (e.g., changing the renderer type and then invoking view.run; note that updating the renderer will force appropriate reinitialization behind the scenes, so there is no need to call initialize explicitly). Not only will this prevent errors, it should also be more efficient.

That said, I'm open to feature request suggestions that might prevent downstream errors! For example, we could consider updating the hover method to track state and reuse/remove prior listeners as needed.

(Flagging @domoritz so he can see this and oversee fixes to vega/editor.)

jheer avatar Dec 19 '18 17:12 jheer

@jheer Thank you so much for the reply! With the information provided i decided to take some parts of the react-vega project and instead create my own wrapper the way it's supposed to work!

Etzix avatar Jan 10 '19 13:01 Etzix

Flagging @kristw and @kanitw for react-vega.

domoritz avatar Jan 15 '19 12:01 domoritz

I fixed the editor.

domoritz avatar Mar 02 '19 18:03 domoritz

Vega 5 now includes expanded View constructor options, including for enabling hover processing and passing a parent DOM element container. I encourage everyone to use those instead of the previous method-chaining approaches to View setup to reduce confusion.

jheer avatar Mar 05 '19 05:03 jheer

Given https://github.com/vega/editor/issues/199 and the Vega 5 updates, are we set to close this issue?

hydrosquall avatar Jan 19 '25 18:01 hydrosquall