From 52ce74a4c3da30f2666018a0bff76bd328ebd609 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= <etienne@plot.ly>
Date: Wed, 13 Jun 2018 12:40:05 -0400
Subject: [PATCH] fix mapbox layout layer updates

- when a new source is given, remove layer, update source and
  add new layer in that order to avoid mapbox-gl errors.
---
 src/plots/mapbox/layers.js        | 13 ++++++--
 test/jasmine/tests/mapbox_test.js | 50 +++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/src/plots/mapbox/layers.js b/src/plots/mapbox/layers.js
index cd535b75141..3262acccade 100644
--- a/src/plots/mapbox/layers.js
+++ b/src/plots/mapbox/layers.js
@@ -39,8 +39,9 @@ proto.update = function update(opts) {
         this.updateLayer(opts);
     } else if(this.needsNewSource(opts)) {
         // IMPORTANT: must delete layer before source to not cause errors
-        this.updateLayer(opts);
+        this.removeLayer();
         this.updateSource(opts);
+        this.updateLayer(opts);
     } else if(this.needsNewLayer(opts)) {
         this.updateLayer(opts);
     } else {
@@ -87,8 +88,7 @@ proto.updateLayer = function(opts) {
     var map = this.map;
     var convertedOpts = convertOpts(opts);
 
-    if(map.getLayer(this.idLayer)) map.removeLayer(this.idLayer);
-
+    this.removeLayer();
     this.layerType = opts.type;
 
     if(isVisible(opts)) {
@@ -111,6 +111,13 @@ proto.updateStyle = function(opts) {
     }
 };
 
+proto.removeLayer = function() {
+    var map = this.map;
+    if(map.getLayer(this.idLayer)) {
+        map.removeLayer(this.idLayer);
+    }
+};
+
 proto.dispose = function dispose() {
     var map = this.map;
     map.removeLayer(this.idLayer);
diff --git a/test/jasmine/tests/mapbox_test.js b/test/jasmine/tests/mapbox_test.js
index 6f68ed85bf6..ecfad33802c 100644
--- a/test/jasmine/tests/mapbox_test.js
+++ b/test/jasmine/tests/mapbox_test.js
@@ -674,6 +674,56 @@ describe('@noCI, mapbox plots', function() {
         .then(done);
     }, LONG_TIMEOUT_INTERVAL);
 
+    it('should be able to react to layer changes', function(done) {
+        function makeFigure(color) {
+            return {
+                data: [{type: 'scattermapbox'}],
+                layout: {
+                    mapbox: {
+                        layers: [{
+                            color: color,
+                            sourcetype: 'geojson',
+                            type: 'fill',
+                            source: {
+                                type: 'Feature',
+                                properties: {},
+                                geometry: {
+                                    type: 'Polygon',
+                                    coordinates: [[
+                                        [174.74475860595703, -36.86533886128865],
+                                        [174.77737426757812, -36.86533886128865],
+                                        [174.77737426757812, -36.84913134182603],
+                                        [174.74475860595703, -36.84913134182603],
+                                        [174.74475860595703, -36.86533886128865]
+                                    ]]
+                                }
+                            }
+                        }]
+                    }
+                }
+            };
+        }
+
+        function _assert(color) {
+            var mapInfo = getMapInfo(gd);
+            var layer = mapInfo.layers[mapInfo.layoutLayers[0]];
+
+            expect(mapInfo.layoutLayers.length).toBe(1, 'one layer');
+            expect(mapInfo.layoutSources.length).toBe(1, 'one layer source');
+            expect(String(layer.paint._values['fill-color'].value.value)).toBe(color, 'layer color');
+        }
+
+        Plotly.react(gd, makeFigure('blue')).then(function() {
+            _assert('rgba(0,0,255,1)');
+            return Plotly.react(gd, makeFigure('red'));
+        })
+        .then(function() {
+            _assert('rgba(255,0,0,1)');
+        })
+        .catch(failTest)
+        .then(done);
+    }, LONG_TIMEOUT_INTERVAL);
+
     it('should be able to update the access token', function(done) {
         Plotly.relayout(gd, 'mapbox.accesstoken', 'wont-work').catch(function(err) {
             expect(gd._fullLayout.mapbox.accesstoken).toEqual('wont-work');