From 8543a5ecda98095bc010b3280339aaf6f0b3a597 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 29 Nov 2017 11:33:24 -0800 Subject: [PATCH] Don't fire sourcedata event with sourceDataType: 'metadata' when a source 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. --- src/style/style.js | 1 - src/ui/control/logo_control.js | 9 +- test/unit/ui/control/logo.test.js | 141 +++++++++++++++++------------- 3 files changed, 84 insertions(+), 67 deletions(-) diff --git a/src/style/style.js b/src/style/style.js index 439e0021acd..c7096a81ec9 100644 --- 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 index 07b39ea6494..1531ab5a212 100644 --- 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 index d70a79017af..25f882f331d 100644 --- 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(); }); }); });