store a reference to all added controls, and remove them when the map…

… is removed
mapbox · Jul 28, 2018 · c446bd5 · c446bd5
1 parent 08ef570
commit c446bd5
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 2 deletions.
diff --git a/src/ui/map.js b/src/ui/map.js
@@ -259,6 +259,7 @@ class Map extends Camera {
     _crossFadingFactor: number;
     _collectResourceTiming: boolean;
     _renderTaskQueue: TaskQueue;
+    _controls: Array<IControl>;
 
     /**
      * The map's {@link ScrollZoomHandler}, which implements zooming in and out with a scroll wheel or trackpad.
@@ -319,6 +320,7 @@ class Map extends Camera {
         this._crossFadingFactor = 1;
         this._collectResourceTiming = options.collectResourceTiming;
         this._renderTaskQueue = new TaskQueue();
+        this._controls = [];
 
         const transformRequestFn = options.transformRequest;
         this._transformRequest = transformRequestFn ?  (url, type) => transformRequestFn(url, type) || ({ url }) : (url) => ({ url });
@@ -412,6 +414,8 @@ class Map extends Camera {
             position = 'top-right';
         }
         const controlElement = control.onAdd(this);
+        this._controls.push(control);
+
         const positionContainer = this._controlPositions[position];
         if (position.indexOf('bottom') !== -1) {
             positionContainer.insertBefore(controlElement, positionContainer.firstChild);
@@ -428,6 +432,8 @@ class Map extends Camera {
      * @returns {Map} `this`
      */
     removeControl(control: IControl) {
+        const ci = this._controls.indexOf(control);
+        if (ci > -1) this._controls.splice(ci, 1);
         control.onRemove(this);
         return this;
     }
@@ -1723,6 +1729,16 @@ class Map extends Camera {
             window.removeEventListener('resize', this._onWindowResize, false);
             window.removeEventListener('online', this._onWindowOnline, false);
         }
+
+        // removeControl splices out the removed element from the controls array,
+        // so save the original length, and remove controls in reverse order
+        // to make sure it is a valid index.
+        const numControls = this._controls.length;
+        for (let i = numControls; i > 0; i--) {
+            this.removeControl(this._controls[i - 1]);
+        }
+        this._controls = [];
+
         const extension = this.painter.context.gl.getExtension('WEBGL_lose_context');
         if (extension) extension.loseContext();
         removeNode(this._canvasContainer);

diff --git a/test/unit/ui/map.test.js b/test/unit/ui/map.test.js
@@ -787,6 +787,7 @@ test('Map', (t) => {
         t.equal(map.getContainer().childNodes.length, 3);
         map.remove();
         t.equal(map.getContainer().childNodes.length, 0);
+        t.equal(map._controls.length, 0);
         t.end();
     });
 
@@ -795,11 +796,12 @@ test('Map', (t) => {
         const control = {
             onAdd: function(_) {
                 t.equal(map, _, 'addTo() called with map');
-                t.end();
                 return window.document.createElement('div');
             }
         };
         map.addControl(control);
+        t.equal(map._controls[1], control, "saves reference to added controls");
+        t.end();
     });
 
     t.test('#removeControl', (t) => {
@@ -810,11 +812,13 @@ test('Map', (t) => {
             },
             onRemove: function(_) {
                 t.equal(map, _, 'onRemove() called with map');
-                t.end();
             }
         };
         map.addControl(control);
         map.removeControl(control);
+        t.equal(map._controls.length, 1, "removes removed controls from map's control array");
+        t.end();
+
     });
 
     t.test('#project', (t) => {