Merge pull request #2766 from plotly/mapbox-relayout-on-dblclick

Fix mapbox interaction inconsistencies

Author: etpinard

src/components/drawing/index.js

@@ -373,9 +373,14 @@ drawing.singlePointStyle = function(d, sel, trace, fns, gd) {
         lineColor = markerLine.outliercolor;
         fillColor = marker.outliercolor;
     } else {
-        lineWidth = (d.mlw + 1 || markerLine.width + 1 ||
+        var markerLineWidth = (markerLine || {}).width;
+
+        lineWidth = (
+            d.mlw + 1 ||
+            markerLineWidth + 1 ||
             // TODO: we need the latter for legends... can we get rid of it?
-            (d.trace ? d.trace.marker.line.width : 0) + 1) - 1;
+            (d.trace ? (d.trace.marker.line || {}).width : 0) + 1
+        ) - 1 || 0;
 
         if('mlc' in d) lineColor = d.mlcc = fns.lineScale(d.mlc);
         // weird case: array wasn't long enough to apply to every point
@@ -591,16 +596,19 @@ drawing.selectedPointStyle = function(s, trace) {
 };
 
 drawing.tryColorscale = function(marker, prefix) {
-    var cont = prefix ? Lib.nestedProperty(marker, prefix).get() : marker,
-        scl = cont.colorscale,
-        colorArray = cont.color;
-
-    if(scl && Lib.isArrayOrTypedArray(colorArray)) {
-        return Colorscale.makeColorScaleFunc(
-            Colorscale.extractScale(scl, cont.cmin, cont.cmax)
-        );
+    var cont = prefix ? Lib.nestedProperty(marker, prefix).get() : marker;
+
+    if(cont) {
+        var scl = cont.colorscale;
+        var colorArray = cont.color;
+
+        if(scl && Lib.isArrayOrTypedArray(colorArray)) {
+            return Colorscale.makeColorScaleFunc(
+                Colorscale.extractScale(scl, cont.cmin, cont.cmax)
+            );
+        }
     }
-    else return Lib.identity;
+    return Lib.identity;
 };
 
 var TEXTOFFSETSIGN = {start: 1, end: -1, middle: 0, bottom: 1, top: -1};

src/plots/mapbox/mapbox.js

@@ -152,9 +152,7 @@ proto.createMap = function(calcData, fullLayout, resolve, reject) {
         // duplicate 'plotly_relayout' events.
 
         if(eventData.originalEvent || wheeling) {
-            var update = {};
-            update[self.id] = Lib.extendFlat({}, view);
-            gd.emit('plotly_relayout', update);
+            emitRelayoutFromView(view);
         }
         wheeling = false;
     });
@@ -195,6 +193,8 @@ proto.createMap = function(calcData, fullLayout, resolve, reject) {
     map.on('zoomstart', unhover);
 
     map.on('dblclick', function() {
+        gd.emit('plotly_doubleclick', null);
+
         var viewInitial = self.viewInitial;
 
         map.setCenter(convertCenter(viewInitial.center));
@@ -209,9 +209,18 @@ proto.createMap = function(calcData, fullLayout, resolve, reject) {
         opts._input.bearing = opts.bearing = viewNow.bearing;
         opts._input.pitch = opts.pitch = viewNow.pitch;
 
-        gd.emit('plotly_doubleclick', null);
+        emitRelayoutFromView(viewNow);
     });
 
+    function emitRelayoutFromView(view) {
+        var id = self.id;
+        var evtData = {};
+        for(var k in view) {
+            evtData[id + '.' + k] = view[k];
+        }
+        gd.emit('plotly_relayout', evtData);
+    }
+
     // define clear select on map creation, to keep one ref per map,
     // so that map.on / map.off in updateFx works as expected
     self.clearSelect = function() {

src/traces/scattermapbox/defaults.js

@@ -41,11 +41,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
         handleMarkerDefaults(traceIn, traceOut, defaultColor, layout, coerce, {noLine: true});
 
         // array marker.size and marker.color are only supported with circles
-
         var marker = traceOut.marker;
-        // we need  mock marker.line object to make legends happy
-        marker.line = {width: 0};
-
         if(marker.symbol !== 'circle') {
             if(Lib.isArrayOrTypedArray(marker.size)) marker.size = marker.size[0];
             if(Lib.isArrayOrTypedArray(marker.color)) marker.color = marker.color[0];

test/jasmine/tests/mapbox_test.js

@@ -887,11 +887,11 @@ describe('@noCI, mapbox plots', function() {
     it('should respond drag / scroll / double-click interactions', function(done) {
         var relayoutCnt = 0;
         var doubleClickCnt = 0;
-        var updateData;
+        var evtData;
 
-        gd.on('plotly_relayout', function(eventData) {
+        gd.on('plotly_relayout', function(d) {
             relayoutCnt++;
-            updateData = eventData;
+            evtData = d;
         });
 
         gd.on('plotly_doubleclick', function() {
@@ -906,43 +906,46 @@ describe('@noCI, mapbox plots', function() {
             });
         }
 
-        function assertLayout(center, zoom, opts) {
-            var mapInfo = getMapInfo(gd),
-                layout = gd.layout.mapbox;
+        function _assertLayout(center, zoom) {
+            var mapInfo = getMapInfo(gd);
+            var layout = gd.layout.mapbox;
 
             expect([mapInfo.center.lng, mapInfo.center.lat]).toBeCloseToArray(center);
             expect(mapInfo.zoom).toBeCloseTo(zoom);
 
             expect([layout.center.lon, layout.center.lat]).toBeCloseToArray(center);
             expect(layout.zoom).toBeCloseTo(zoom);
+        }
 
-            if(opts && opts.withUpdateData) {
-                var mapboxUpdate = updateData.mapbox;
+        function _assert(center, zoom) {
+            _assertLayout(center, zoom);
 
-                expect([mapboxUpdate.center.lon, mapboxUpdate.center.lat]).toBeCloseToArray(center);
-                expect(mapboxUpdate.zoom).toBeCloseTo(zoom);
-            }
+            expect([evtData['mapbox.center'].lon, evtData['mapbox.center'].lat]).toBeCloseToArray(center);
+            expect(evtData['mapbox.zoom']).toBeCloseTo(zoom);
         }
 
-        assertLayout([-4.710, 19.475], 1.234);
+        _assertLayout([-4.710, 19.475], 1.234);
 
         var p1 = [pointPos[0] + 50, pointPos[1] - 20];
 
         _drag(pointPos, p1, function() {
-            expect(relayoutCnt).toEqual(1);
-            assertLayout([-19.651, 13.751], 1.234, {withUpdateData: true});
+            expect(relayoutCnt).toBe(1, 'relayout cnt');
+            expect(doubleClickCnt).toBe(0, 'double click cnt');
+            _assert([-19.651, 13.751], 1.234);
 
             return _doubleClick(p1);
         })
         .then(function() {
+            expect(relayoutCnt).toBe(2, 'relayout cnt');
             expect(doubleClickCnt).toBe(1, 'double click cnt');
-            assertLayout([-4.710, 19.475], 1.234);
+            _assert([-4.710, 19.475], 1.234);
 
             return _scroll(pointPos);
         })
         .then(function() {
+            expect(relayoutCnt).toBe(3, 'relayout cnt');
+            expect(doubleClickCnt).toBe(1, 'double click cnt');
             expect(getMapInfo(gd).zoom).toBeGreaterThan(1.234);
-            expect(relayoutCnt).toBe(2);
         })
         .catch(failTest)
         .then(done);

test/jasmine/tests/scattermapbox_test.js

@@ -110,6 +110,17 @@ describe('scattermapbox defaults', function() {
         expect(fullTrace.marker.color).toEqual(['red', 'green', 'blue']);
         expect(fullTrace.marker.size).toEqual([10, 20, 30]);
     });
+
+    it('should not fill *marker.line* in fullData while is not available', function() {
+        var fullTrace = _supply({
+            mode: 'markers',
+            lon: [10, 20, 30],
+            lat: [10, 20, 30]
+        });
+
+        expect(fullTrace.marker).toBeDefined();
+        expect(fullTrace.marker.line).toBeUndefined();
+    });
 });
 
 describe('scattermapbox convert', function() {