From c95dd4b3507f59fa2714f2d76299fb938142cbd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 29 Jun 2018 14:29:31 -0400 Subject: [PATCH 1/3] fix mocked plotly_relayout event data on mapbox interactions --- src/plots/mapbox/mapbox.js | 13 ++++++++++--- test/jasmine/tests/mapbox_test.js | 5 ++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/plots/mapbox/mapbox.js b/src/plots/mapbox/mapbox.js index d69a580a90a..3d0e30ebf4a 100644 --- a/src/plots/mapbox/mapbox.js +++ b/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; }); @@ -212,6 +210,15 @@ proto.createMap = function(calcData, fullLayout, resolve, reject) { gd.emit('plotly_doubleclick', null); }); + 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() { diff --git a/test/jasmine/tests/mapbox_test.js b/test/jasmine/tests/mapbox_test.js index ecfad33802c..ecb118c2172 100644 --- a/test/jasmine/tests/mapbox_test.js +++ b/test/jasmine/tests/mapbox_test.js @@ -896,10 +896,9 @@ describe('@noCI, mapbox plots', function() { expect(layout.zoom).toBeCloseTo(zoom); if(opts && opts.withUpdateData) { - var mapboxUpdate = updateData.mapbox; - 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); } } From 6f1a55be939b528dd29f5e243cff054097f37e5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 29 Jun 2018 14:30:03 -0400 Subject: [PATCH 2/3] emit plotly_relayout (w/ mocked evt data) on mapbox dblclick --- src/plots/mapbox/mapbox.js | 4 +++- test/jasmine/tests/mapbox_test.js | 30 +++++++++++++++++------------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/plots/mapbox/mapbox.js b/src/plots/mapbox/mapbox.js index 3d0e30ebf4a..c5332d05fd8 100644 --- a/src/plots/mapbox/mapbox.js +++ b/src/plots/mapbox/mapbox.js @@ -193,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)); @@ -207,7 +209,7 @@ 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) { diff --git a/test/jasmine/tests/mapbox_test.js b/test/jasmine/tests/mapbox_test.js index ecb118c2172..9eb20f66fe9 100644 --- a/test/jasmine/tests/mapbox_test.js +++ b/test/jasmine/tests/mapbox_test.js @@ -866,11 +866,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() { @@ -885,42 +885,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) { + function _assert(center, zoom) { + _assertLayout(center, 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); From f2fe2b2c609203f8b6f22b8914d23071571b074a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 29 Jun 2018 17:41:10 -0400 Subject: [PATCH 3/3] fix #2727 - do not fill *marker.line* in fullData, ... while it is not available for scattermapbox, add fallbacks downstream instead. --- src/components/drawing/index.js | 30 +++++++++++++++--------- src/traces/scattermapbox/defaults.js | 4 ---- test/jasmine/tests/scattermapbox_test.js | 11 +++++++++ 3 files changed, 30 insertions(+), 15 deletions(-) diff --git a/src/components/drawing/index.js b/src/components/drawing/index.js index ae9c8120e16..5635d2b062d 100644 --- a/src/components/drawing/index.js +++ b/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}; diff --git a/src/traces/scattermapbox/defaults.js b/src/traces/scattermapbox/defaults.js index d64e1c92476..efbe968ae18 100644 --- a/src/traces/scattermapbox/defaults.js +++ b/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]; diff --git a/test/jasmine/tests/scattermapbox_test.js b/test/jasmine/tests/scattermapbox_test.js index dddb8c77a58..70234c89bcf 100644 --- a/test/jasmine/tests/scattermapbox_test.js +++ b/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() {