Skip to content

Commit

Permalink
fix(AbstractWidget): Ensure we do not re-register callbacks
Browse files Browse the repository at this point in the history
`listenEvents` would usually be called by `setEnable` and `setInteractor`,
which would register callbacks twice. Also, `listenEvents` is part of the
publicAPI, so callbacks could have been registered again by explicitly
calling that function.

We now ensure that those callbacks are unsubscribed from the event before
subscribing again.
  • Loading branch information
Alexis Girault committed Jan 31, 2018
1 parent 73c48a4 commit 93be5c3
Showing 1 changed file with 13 additions and 18 deletions.
31 changes: 13 additions & 18 deletions Sources/Interaction/Widgets/AbstractWidget/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ function vtkAbstractWidget(publicAPI, model) {
vtkErrorMacro('The interactor must be set before listening events');
return;
}

// Remove current events
while (model.unsubscribes.length) {
model.unsubscribes.pop().unsubscribe();
}

// Check what events we can handle and register callbacks
vtkRenderWindowInteractor.handledEvents.forEach((eventName) => {
if (publicAPI[`handle${eventName}`]) {
model.unsubscribes.push(
Expand All @@ -36,17 +43,9 @@ function vtkAbstractWidget(publicAPI, model) {
if (i === model.interactor) {
return;
}

// if we already have an Interactor then stop observing it
if (model.interactor) {
while (model.unsubscribes.length) {
model.unsubscribes.pop().unsubscribe();
}
}

model.interactor = i;

if (i) {
if (i && model.enabled) {
publicAPI.listenEvents();
}

Expand All @@ -60,11 +59,11 @@ function vtkAbstractWidget(publicAPI, model) {
};

publicAPI.setEnable = (enable) => {
if (enable === model.enabled) {
return;
}

if (enable) {
if (model.enabled) {
// widget already enabled
return;
}
if (!model.interactor) {
vtkErrorMacro(
'The interactor must be set prior to enabling the widget'
Expand Down Expand Up @@ -97,13 +96,9 @@ function vtkAbstractWidget(publicAPI, model) {
model.widgetRep.buildRepresentation();
model.currentRenderer.addViewProp(model.widgetRep);
} else {
if (!model.enabled) {
// already
return;
}
model.enabled = 0;

// Don't listen events
// Don't listen to events
while (model.unsubscribes.length) {
model.unsubscribes.pop().unsubscribe();
}
Expand Down

0 comments on commit 93be5c3

Please sign in to comment.