Skip to content

Commit

Permalink
[Time Conductor] #933 Clean up time conductor listeners on scope dest…
Browse files Browse the repository at this point in the history
…ruction
  • Loading branch information
akhenry committed Sep 22, 2016
1 parent 11e0603 commit 904d56a
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ define(
ConductorRepresenter.prototype.destroy = function destroy() {
this.conductor.off("bounds", this.boundsListener);
this.conductor.off("timeSystem", this.timeSystemListener);
this.conductor.off("follow", this.followListener);
};

return ConductorRepresenter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,20 @@ define(
this.xScale = undefined;
this.xAxis = undefined;
this.axisElement = undefined;
this.setScale = this.setScale.bind(this);
this.changeTimeSystem = this.changeTimeSystem.bind(this);

// Angular Directive interface
this.link = this.link.bind(this);
this.restrict = "E";
this.template =
"<div class=\"l-axis-holder\" mct-resize=\"resize()\"></div>";
this.priority = 1000;

//Bind all class functions to 'this'
Object.keys(MCTConductorAxis.prototype).filter(function (key) {
return typeof MCTConductorAxis.prototype[key] === 'function';
}).forEach(function (key) {
this[key] = this[key].bind(this);
}.bind(this));
}

MCTConductorAxis.prototype.setScale = function () {
Expand Down Expand Up @@ -99,6 +104,11 @@ define(
}
};

MCTConductorAxis.prototype.destroy = function () {
this.conductor.off('timeSystem', this.changeTimeSystem);
this.conductor.off('bounds', this.setScale);
};

MCTConductorAxis.prototype.link = function (scope, element) {
var conductor = this.conductor;
this.target = element[0].firstChild;
Expand All @@ -121,6 +131,8 @@ define(
//On conductor bounds changes, redraw ticks
conductor.on('bounds', this.setScale);

scope.$on("$destroy", this.destroy);

if (conductor.timeSystem() !== undefined) {
this.changeTimeSystem(conductor.timeSystem());
this.setScale();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ define(['./MctConductorAxis'], function (MctConductorAxis) {
d3;

beforeEach(function () {
mockScope = {};
mockScope = jasmine.createSpyObj("scope", [
"$on"
]);

//Add some HTML elements
mockTarget = {
Expand All @@ -49,7 +51,8 @@ define(['./MctConductorAxis'], function (MctConductorAxis) {
mockConductor = jasmine.createSpyObj("conductor", [
"timeSystem",
"bounds",
"on"
"on",
"off"
]);
mockConductor.bounds.andReturn(mockBounds);

Expand Down Expand Up @@ -85,6 +88,13 @@ define(['./MctConductorAxis'], function (MctConductorAxis) {
expect(mockConductor.on).toHaveBeenCalledWith("bounds", directive.setScale);
});

it("on scope destruction, deregisters listeners", function () {
expect(mockScope.$on).toHaveBeenCalledWith("$destroy", directive.destroy);
directive.destroy();
expect(mockConductor.off).toHaveBeenCalledWith("timeSystem", directive.changeTimeSystem);
expect(mockConductor.off).toHaveBeenCalledWith("bounds", directive.setScale);
});

describe("when the time system changes", function () {
var mockTimeSystem;
var mockFormat;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@ define(
this.initializeScope();

this.conductor.on('bounds', this.setFormFromBounds);
this.conductor.on('follow', function (follow) {
$scope.followMode = follow;
});
this.conductor.on('timeSystem', this.changeTimeSystem);

// If no mode selected, select fixed as the default
Expand Down Expand Up @@ -100,6 +97,13 @@ define(

// Watch scope for selection of mode or time system by user
this.$scope.$watch('modeModel.selectedKey', this.setMode);

this.$scope.$on('$destroy', this.destroy);
};

TimeConductorController.prototype.destroy = function () {
this.conductor.off('bounds', this.setFormFromBounds);
this.conductor.off('timeSystem', this.changeTimeSystem);
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,18 @@ define(['./TimeConductorController'], function (TimeConductorController) {
var controller;

beforeEach(function () {
mockScope = jasmine.createSpyObj("$scope", ["$watch"]);
mockScope = jasmine.createSpyObj("$scope", [
"$watch",
"$on"
]);
mockWindow = jasmine.createSpyObj("$window", ["requestAnimationFrame"]);
mockTimeConductor = jasmine.createSpyObj(
"TimeConductor",
[
"bounds",
"timeSystem",
"on"
"on",
"off"
]
);
mockTimeConductor.bounds.andReturn({start: undefined, end: undefined});
Expand Down Expand Up @@ -124,9 +128,16 @@ define(['./TimeConductorController'], function (TimeConductorController) {
});

it("listens for changes to conductor state", function () {
expect(mockTimeConductor.on).toHaveBeenCalledWith("timeSystem", jasmine.any(Function));
expect(mockTimeConductor.on).toHaveBeenCalledWith("bounds", jasmine.any(Function));
expect(mockTimeConductor.on).toHaveBeenCalledWith("follow", jasmine.any(Function));
expect(mockTimeConductor.on).toHaveBeenCalledWith("timeSystem", controller.changeTimeSystem);
expect(mockTimeConductor.on).toHaveBeenCalledWith("bounds", controller.setFormFromBounds);
});

it("deregisters conductor listens when scope is destroyed", function () {
expect(mockScope.$on).toHaveBeenCalledWith("$destroy", controller.destroy);

controller.destroy();
expect(mockTimeConductor.off).toHaveBeenCalledWith("timeSystem", controller.changeTimeSystem);
expect(mockTimeConductor.off).toHaveBeenCalledWith("bounds", controller.setFormFromBounds);
});

it("when time system changes, sets time system on scope", function () {
Expand Down Expand Up @@ -164,17 +175,6 @@ define(['./TimeConductorController'], function (TimeConductorController) {
expect(mockScope.boundsModel.start).toEqual(bounds.start);
expect(mockScope.boundsModel.end).toEqual(bounds.end);
});

it("responds to a change in 'follow' state of the time conductor", function () {
var followListener = getListener("follow");
expect(followListener).toBeDefined();

followListener(true);
expect(mockScope.followMode).toEqual(true);

followListener(false);
expect(mockScope.followMode).toEqual(false);
});
});

describe("when user makes changes from UI", function () {
Expand Down

0 comments on commit 904d56a

Please sign in to comment.