Don't fire sourcedata event with sourceDataType: 'metadata' when a so…

…urce is removed This event doesn't really make sense for this scenario; the metadata isn't changing. Instead, have the LogoControl listen for the styledata event that's triggered when a source is removed.
mapbox · Nov 29, 2017 · 8543a5e · 8543a5e
1 parent be03543
commit 8543a5e
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 67 deletions.
diff --git a/src/style/style.js b/src/style/style.js
@@ -511,7 +511,6 @@ class Style extends Evented {
         const sourceCache = this.sourceCaches[id];
         delete this.sourceCaches[id];
         delete this._updatedSources[id];
-        sourceCache.fire('data', {sourceDataType: 'metadata', dataType:'source', sourceId: id});
         sourceCache.setEventedParent(null);
         sourceCache.clearTiles();
 

diff --git a/src/ui/control/logo_control.js b/src/ui/control/logo_control.js
@@ -32,6 +32,7 @@ class LogoControl {
         this._container.appendChild(anchor);
         this._container.style.display = 'none';
 
+        this._map.on('styledata', this._updateLogo);
         this._map.on('sourcedata', this._updateLogo);
         this._updateLogo();
         return this._container;
@@ -46,10 +47,8 @@ class LogoControl {
         return 'bottom-left';
     }
 
-    _updateLogo(e: any) {
-        if (!e || e.sourceDataType === 'metadata') {
-            this._container.style.display = this._logoRequired() ? 'block' : 'none';
-        }
+    _updateLogo() {
+        this._container.style.display = this._logoRequired() ? 'block' : 'none';
     }
 
     _logoRequired() {
@@ -65,8 +64,6 @@ class LogoControl {
 
         return false;
     }
-
 }
 
-
 module.exports = LogoControl;
diff --git a/test/unit/ui/control/logo.test.js b/test/unit/ui/control/logo.test.js
@@ -1,88 +1,109 @@
 'use strict';
+
 const test = require('mapbox-gl-js-test').test;
-const VectorTileSource = require('../../../../src/source/vector_tile_source');
 const window = require('../../../../src/util/window');
 const Map = require('../../../../src/ui/map');
 
-function createMap(logoPosition, logoRequired) {
+test('LogoControl appears in bottom-left by default', (t) => {
+    const container = window.document.createElement('div');
+    const map = new Map({container});
+    t.equal(map.getContainer().querySelectorAll(
+        '.mapboxgl-ctrl-bottom-left .mapboxgl-ctrl-logo'
+    ).length, 1);
+    t.end();
+});
+
+test('LogoControl appears in the position specified by the position option', (t) => {
     const container = window.document.createElement('div');
-    return new Map({
-        container: container,
+    const map = new Map({container, logoPosition: 'top-left'});
+    t.equal(map.getContainer().querySelectorAll(
+        '.mapboxgl-ctrl-top-left .mapboxgl-ctrl-logo'
+    ).length, 1);
+    t.end();
+});
+
+test('LogoControl is hidden when no sources with the mapbox_logo property exist', (t) => {
+    const container = window.document.createElement('div');
+    const map = new Map({
+        container,
         style: {
             version: 8,
-            sources: {
-                'composite': createSource({
-                    minzoom: 1,
-                    maxzoom: 10,
-                    attribution: "Mapbox",
-                    tiles: [
-                        "http://example.com/{z}/{x}/{y}.png"
-                    ]
-                }, logoRequired)
-            },
+            sources: {},
             layers: []
         },
-        logoPosition: logoPosition || undefined
     });
-}
-
-function createSource(options, logoRequired) {
-    const source = new VectorTileSource('id', options, { send: function () {} });
-    source.onAdd({
-        transform: { angle: 0, pitch: 0, showCollisionBoxes: false }
-    });
-    source.on('error', (e) => {
-        throw e.error;
-    });
-    const logoFlag = "mapbox_logo";
-    source[logoFlag] = logoRequired === undefined ? true : logoRequired;
-    return source;
-}
-test('LogoControl appears in bottom-left by default', (t) => {
-    const map = createMap();
     map.on('load', () => {
-        t.equal(map.getContainer().querySelectorAll(
-            '.mapboxgl-ctrl-bottom-left .mapboxgl-ctrl-logo'
-        ).length, 1);
+        t.equal(map.getContainer().querySelector('.mapboxgl-ctrl-logo').parentNode.style.display, 'none');
         t.end();
     });
 });
 
-test('LogoControl appears in the position specified by the position option', (t) => {
-    const map = createMap('top-left');
+test('LogoControl is shown when a source with the mapbox_logo property is added', (t) => {
+    const container = window.document.createElement('div');
+    const map = new Map({
+        container,
+        style: {
+            version: 8,
+            sources: {},
+            layers: []
+        },
+    });
     map.on('load', () => {
-        t.equal(map.getContainer().querySelectorAll(
-            '.mapboxgl-ctrl-top-left .mapboxgl-ctrl-logo'
-        ).length, 1);
-        t.end();
+        map.addSource('source', {
+            type: 'raster',
+            tiles: ["http://example.com/{z}/{x}/{y}.png"],
+            mapbox_logo: true // eslint-disable-line
+        });
+        map.once('sourcedata', () => {
+            t.equal(map.getContainer().querySelector('.mapboxgl-ctrl-logo').parentNode.style.display, 'block');
+            t.end();
+        });
     });
 });
 
-test('LogoControl is not displayed when the mapbox_logo property is false', (t) => {
-    const map = createMap('top-left', false);
+test('LogoControl is visible when a source with the mapbox_logo property exists', (t) => {
+    const container = window.document.createElement('div');
+    const map = new Map({
+        container,
+        style: {
+            version: 8,
+            sources: {
+                source: {
+                    type: 'raster',
+                    tiles: ["http://example.com/{z}/{x}/{y}.png"],
+                    mapbox_logo: true // eslint-disable-line
+                }
+            },
+            layers: []
+        },
+    });
     map.on('load', () => {
-        t.equal(map.getContainer().querySelectorAll('.mapboxgl-ctrl-top-left > .mapboxgl-ctrl')[0].style.display, 'none');
+        t.equal(map.getContainer().querySelector('.mapboxgl-ctrl-logo').parentNode.style.display, 'block');
         t.end();
     });
 });
-test('LogoControl is not added more than once', (t)=>{
-    const map = createMap();
-    const source = createSource({
-        minzoom: 1,
-        maxzoom: 10,
-        attribution: "Mapbox",
-        tiles: [
-            "http://example.com/{z}/{x}/{y}.png"
-        ]
+
+test('LogoControl is hidden when the last source with the mapbox_logo property is removed', (t) => {
+    const container = window.document.createElement('div');
+    const map = new Map({
+        container,
+        style: {
+            version: 8,
+            sources: {
+                source: {
+                    type: 'raster',
+                    tiles: ["http://example.com/{z}/{x}/{y}.png"],
+                    mapbox_logo: true // eslint-disable-line
+                }
+            },
+            layers: []
+        },
     });
-    map.on('load', ()=>{
-        t.equal(map.getContainer().querySelectorAll('.mapboxgl-ctrl-logo').length, 1, 'first LogoControl');
-        map.addSource('source2', source);
-        map.on('sourcedata', (e)=>{
-            if (e.isSourceLoaded && e.sourceId === 'source2' && e.sourceDataType === 'metadata') {
-                t.equal(map.getContainer().querySelectorAll('.mapboxgl-ctrl-logo').length, 1, 'only one LogoControl is added with multiple sources');
-                t.end();
-            }
+    map.on('load', () => {
+        map.removeSource('source');
+        map.once('styledata', () => {
+            t.equal(map.getContainer().querySelector('.mapboxgl-ctrl-logo').parentNode.style.display, 'none');
+            t.end();
         });
     });
 });