[Time Conductor] #933 Clean up time conductor listeners on scope destruction

Author: akhenry

platform/features/conductor-v2/compatibility/src/ConductorRepresenter.js

@@ -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;

platform/features/conductor-v2/conductor/src/ui/MctConductorAxis.js

@@ -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 () {
@@ -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;
@@ -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();

platform/features/conductor-v2/conductor/src/ui/MctConductorAxisSpec.js

@@ -32,7 +32,9 @@ define(['./MctConductorAxis'], function (MctConductorAxis) {
             d3;
 
         beforeEach(function () {
-            mockScope = {};
+            mockScope = jasmine.createSpyObj("scope", [
+                "$on"
+            ]);
 
             //Add some HTML elements
             mockTarget = {
@@ -49,7 +51,8 @@ define(['./MctConductorAxis'], function (MctConductorAxis) {
             mockConductor = jasmine.createSpyObj("conductor", [
                 "timeSystem",
                 "bounds",
-                "on"
+                "on",
+                "off"
             ]);
             mockConductor.bounds.andReturn(mockBounds);
 
@@ -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;

platform/features/conductor-v2/conductor/src/ui/TimeConductorController.js

@@ -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
@@ -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);
         };
 
         /**

platform/features/conductor-v2/conductor/src/ui/TimeConductorControllerSpec.js

@@ -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});
@@ -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 () {
@@ -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 () {