From 31812dae7059dbcbd66aa8ee95529c911e5ad178 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 29 Nov 2017 11:33:24 -0800 Subject: [PATCH 1/5] 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(); }); }); }); From 36fdac9e24fd65e051ccac73e944e9f1459c6bd8 Mon Sep 17 00:00:00 2001 From: Molly Lloyd Date: Thu, 11 Jan 2018 11:27:02 -0800 Subject: [PATCH 2/5] make relevant changes to attribution control, remove sourcedata update from logocontrol --- src/ui/control/attribution_control.js | 12 +++---- src/ui/control/logo_control.js | 1 - test/unit/ui/control/attribution.test.js | 41 ++++++++++++++++-------- 3 files changed, 33 insertions(+), 21 deletions(-) diff --git a/src/ui/control/attribution_control.js b/src/ui/control/attribution_control.js index 821aac95b77..466c76605d2 100644 --- a/src/ui/control/attribution_control.js +++ b/src/ui/control/attribution_control.js @@ -53,7 +53,7 @@ class AttributionControl { this._updateAttributions(); this._updateEditLink(); - this._map.on('sourcedata', this._updateData); + this._map.on('styledata', this._updateData); this._map.on('moveend', this._updateEditLink); if (compact === undefined) { @@ -97,15 +97,13 @@ class AttributionControl { } } - _updateData(e: any) { - if (e && e.sourceDataType === 'metadata') { - this._updateAttributions(); - this._updateEditLink(); - } + _updateData() { + this._updateAttributions(); + this._updateEditLink(); } _updateAttributions() { - if (!this._map.style) return; + if (!this._map || !this._map.style) return; let attributions: Array = []; if (this._map.style.stylesheet) { diff --git a/src/ui/control/logo_control.js b/src/ui/control/logo_control.js index 1531ab5a212..13f187f598d 100644 --- a/src/ui/control/logo_control.js +++ b/src/ui/control/logo_control.js @@ -33,7 +33,6 @@ class LogoControl { this._container.style.display = 'none'; this._map.on('styledata', this._updateLogo); - this._map.on('sourcedata', this._updateLogo); this._updateLogo(); return this._container; } diff --git a/test/unit/ui/control/attribution.test.js b/test/unit/ui/control/attribution.test.js index 4dcb8940339..df6fd01dad1 100644 --- a/test/unit/ui/control/attribution.test.js +++ b/test/unit/ui/control/attribution.test.js @@ -91,16 +91,33 @@ test('AttributionControl dedupes attributions that are substrings of others', (t map.addSource('3', { type: 'vector', attribution: 'Another Source' }); map.addSource('4', { type: 'vector', attribution: 'Hello' }); map.addSource('5', { type: 'vector', attribution: 'Hello World' }); + }); + + let times = 0; + map.on('styledata', () => { + if (++times === 2) { + t.equal(attribution._container.innerHTML, 'Hello World | Another Source'); + t.end(); + } + }); +}); +test('AttributionControl is removed when source is removed', (t) => { + const map = createMap(); + const attribution = new AttributionControl(); + map.addControl(attribution); + + map.on('load', () => { + map.addSource('1', { type: 'vector', attribution: 'Hello World' }); + map.addSource('2', { type: 'vector', attribution: 'Another Source' }); + map.removeSource('2'); }); let times = 0; - map.on('data', (e) => { - if (e.dataType === 'source' && e.sourceDataType === 'metadata') { - if (++times === 5) { - t.equal(attribution._container.innerHTML, 'Hello World | Another Source'); - t.end(); - } + map.on('styledata', () => { + if (++times === 2) { + t.equal(attribution._container.innerHTML, 'Hello World'); + t.end(); } }); }); @@ -111,13 +128,11 @@ test('AttributionControl has the correct edit map link', (t) => { map.addControl(attribution); map.on('load', () => { map.addSource('1', {type: 'vector', attribution: 'Improve this map'}); - map.on('data', (e) => { - if (e.dataType === 'source' && e.sourceDataType === 'metadata') { - t.equal(attribution._editLink.href, 'https://www.mapbox.com/feedback/?owner=mapbox&id=streets-v10&access_token=pk.123#/0/0/0', 'edit link contains map location data'); - map.setZoom(2); - t.equal(attribution._editLink.href, 'https://www.mapbox.com/feedback/?owner=mapbox&id=streets-v10&access_token=pk.123#/0/0/2', 'edit link updates on mapmove'); - t.end(); - } + map.on('styledata', () => { + t.equal(attribution._editLink.href, 'https://www.mapbox.com/feedback/?owner=mapbox&id=streets-v10&access_token=pk.123#/0/0/0', 'edit link contains map location data'); + map.setZoom(2); + t.equal(attribution._editLink.href, 'https://www.mapbox.com/feedback/?owner=mapbox&id=streets-v10&access_token=pk.123#/0/0/2', 'edit link updates on mapmove'); + t.end(); }); }); }); From a3a47b14dbab450cf8d781baaf804fb6a8bd7903 Mon Sep 17 00:00:00 2001 From: Molly Lloyd Date: Thu, 11 Jan 2018 11:47:49 -0800 Subject: [PATCH 3/5] fix logo test --- test/unit/ui/control/logo.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/ui/control/logo.test.js b/test/unit/ui/control/logo.test.js index 25f882f331d..f0c4d182b55 100644 --- a/test/unit/ui/control/logo.test.js +++ b/test/unit/ui/control/logo.test.js @@ -54,7 +54,7 @@ test('LogoControl is shown when a source with the mapbox_logo property is added' tiles: ["http://example.com/{z}/{x}/{y}.png"], mapbox_logo: true // eslint-disable-line }); - map.once('sourcedata', () => { + map.once('styledata', () => { t.equal(map.getContainer().querySelector('.mapboxgl-ctrl-logo').parentNode.style.display, 'block'); t.end(); }); From d8a166f64938818b71391f8e1690d9951538f429 Mon Sep 17 00:00:00 2001 From: Molly Lloyd Date: Thu, 11 Jan 2018 13:30:43 -0800 Subject: [PATCH 4/5] put sourcedata listener back in --- src/ui/control/attribution_control.js | 10 +++++++--- src/ui/control/logo_control.js | 6 ++++-- test/unit/ui/control/attribution.test.js | 24 +++++++++--------------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/ui/control/attribution_control.js b/src/ui/control/attribution_control.js index 466c76605d2..e4b91a22ec0 100644 --- a/src/ui/control/attribution_control.js +++ b/src/ui/control/attribution_control.js @@ -54,6 +54,7 @@ class AttributionControl { this._updateEditLink(); this._map.on('styledata', this._updateData); + this._map.on('sourcedata', this._updateData); this._map.on('moveend', this._updateEditLink); if (compact === undefined) { @@ -67,6 +68,7 @@ class AttributionControl { onRemove() { DOM.remove(this._container); + this._map.off('styledata', this._updateData); this._map.off('sourcedata', this._updateData); this._map.off('moveend', this._updateEditLink); this._map.off('resize', this._updateCompact); @@ -97,9 +99,11 @@ class AttributionControl { } } - _updateData() { - this._updateAttributions(); - this._updateEditLink(); + _updateData(e) { + if (e && (e.type === "styledata" || e.sourceDataType === "metadata")) { + this._updateAttributions(); + this._updateEditLink(); + } } _updateAttributions() { diff --git a/src/ui/control/logo_control.js b/src/ui/control/logo_control.js index 13f187f598d..e63cc20327e 100644 --- a/src/ui/control/logo_control.js +++ b/src/ui/control/logo_control.js @@ -33,12 +33,14 @@ class LogoControl { this._container.style.display = 'none'; this._map.on('styledata', this._updateLogo); + this._map.on('sourcedata', this._updateLogo); this._updateLogo(); return this._container; } onRemove() { DOM.remove(this._container); + this._map.off('styledata', this._updateLogo); this._map.off('sourcedata', this._updateLogo); } @@ -46,8 +48,8 @@ class LogoControl { return 'bottom-left'; } - _updateLogo() { - this._container.style.display = this._logoRequired() ? 'block' : 'none'; + _updateLogo(e) { + if (e && (e.type === 'styledata' || e.sourceDataType === "metadata")) this._container.style.display = this._logoRequired() ? 'block' : 'none'; } _logoRequired() { diff --git a/test/unit/ui/control/attribution.test.js b/test/unit/ui/control/attribution.test.js index df6fd01dad1..cd5c21b1bfa 100644 --- a/test/unit/ui/control/attribution.test.js +++ b/test/unit/ui/control/attribution.test.js @@ -86,20 +86,16 @@ test('AttributionControl dedupes attributions that are substrings of others', (t map.addControl(attribution); map.on('load', () => { + map.on('styledata', () => { + t.equal(attribution._container.innerHTML, 'Hello World | Another Source'); + t.end(); + }); map.addSource('1', { type: 'vector', attribution: 'World' }); map.addSource('2', { type: 'vector', attribution: 'Hello World' }); map.addSource('3', { type: 'vector', attribution: 'Another Source' }); map.addSource('4', { type: 'vector', attribution: 'Hello' }); map.addSource('5', { type: 'vector', attribution: 'Hello World' }); }); - - let times = 0; - map.on('styledata', () => { - if (++times === 2) { - t.equal(attribution._container.innerHTML, 'Hello World | Another Source'); - t.end(); - } - }); }); test('AttributionControl is removed when source is removed', (t) => { @@ -108,18 +104,16 @@ test('AttributionControl is removed when source is removed', (t) => { map.addControl(attribution); map.on('load', () => { + map.on('styledata', () => { + t.equal(attribution._container.innerHTML, 'Hello World'); + t.end(); + }); map.addSource('1', { type: 'vector', attribution: 'Hello World' }); map.addSource('2', { type: 'vector', attribution: 'Another Source' }); map.removeSource('2'); - }); - let times = 0; - map.on('styledata', () => { - if (++times === 2) { - t.equal(attribution._container.innerHTML, 'Hello World'); - t.end(); - } }); + }); test('AttributionControl has the correct edit map link', (t) => { From e22c807f8f619f0aee2eccb5da00290a74c9d7c0 Mon Sep 17 00:00:00 2001 From: Molly Lloyd Date: Thu, 11 Jan 2018 13:50:49 -0800 Subject: [PATCH 5/5] fix flow --- src/ui/control/attribution_control.js | 2 +- src/ui/control/logo_control.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ui/control/attribution_control.js b/src/ui/control/attribution_control.js index e4b91a22ec0..7038a4a5c1f 100644 --- a/src/ui/control/attribution_control.js +++ b/src/ui/control/attribution_control.js @@ -99,7 +99,7 @@ class AttributionControl { } } - _updateData(e) { + _updateData(e: ?Object) { if (e && (e.type === "styledata" || e.sourceDataType === "metadata")) { this._updateAttributions(); this._updateEditLink(); diff --git a/src/ui/control/logo_control.js b/src/ui/control/logo_control.js index e63cc20327e..9f6eef1b160 100644 --- a/src/ui/control/logo_control.js +++ b/src/ui/control/logo_control.js @@ -48,7 +48,7 @@ class LogoControl { return 'bottom-left'; } - _updateLogo(e) { + _updateLogo(e: ?Object) { if (e && (e.type === 'styledata' || e.sourceDataType === "metadata")) this._container.style.display = this._logoRequired() ? 'block' : 'none'; }