Skip to content

Commit f06032f

Browse files
etpinardetpinard
authored
Merge pull request #2766 from plotly/mapbox-relayout-on-dblclick
Fix mapbox interaction inconsistencies
2 parents fc0a1f8 + f2fe2b2 commit f06032f

File tree

5 files changed

+62
-35
lines changed

5 files changed

+62
-35
lines changed

‎src/components/drawing/index.js

+19-11
Original file line numberDiff line numberDiff line change
@@ -373,9 +373,14 @@ drawing.singlePointStyle = function(d, sel, trace, fns, gd) {
373373
lineColor = markerLine.outliercolor;
374374
fillColor = marker.outliercolor;
375375
} else {
376-
lineWidth = (d.mlw + 1 || markerLine.width + 1 ||
376+
var markerLineWidth = (markerLine || {}).width;
377+
378+
lineWidth = (
379+
d.mlw + 1 ||
380+
markerLineWidth + 1 ||
377381
// TODO: we need the latter for legends... can we get rid of it?
378-
(d.trace ? d.trace.marker.line.width : 0) + 1) - 1;
382+
(d.trace ? (d.trace.marker.line || {}).width : 0) + 1
383+
) - 1 || 0;
379384

380385
if('mlc' in d) lineColor = d.mlcc = fns.lineScale(d.mlc);
381386
// weird case: array wasn't long enough to apply to every point
@@ -591,16 +596,19 @@ drawing.selectedPointStyle = function(s, trace) {
591596
};
592597

593598
drawing.tryColorscale = function(marker, prefix) {
594-
var cont = prefix ? Lib.nestedProperty(marker, prefix).get() : marker,
595-
scl = cont.colorscale,
596-
colorArray = cont.color;
597-
598-
if(scl && Lib.isArrayOrTypedArray(colorArray)) {
599-
return Colorscale.makeColorScaleFunc(
600-
Colorscale.extractScale(scl, cont.cmin, cont.cmax)
601-
);
599+
var cont = prefix ? Lib.nestedProperty(marker, prefix).get() : marker;
600+
601+
if(cont) {
602+
var scl = cont.colorscale;
603+
var colorArray = cont.color;
604+
605+
if(scl && Lib.isArrayOrTypedArray(colorArray)) {
606+
return Colorscale.makeColorScaleFunc(
607+
Colorscale.extractScale(scl, cont.cmin, cont.cmax)
608+
);
609+
}
602610
}
603-
else return Lib.identity;
611+
return Lib.identity;
604612
};
605613

606614
var TEXTOFFSETSIGN = {start: 1, end: -1, middle: 0, bottom: 1, top: -1};

‎src/plots/mapbox/mapbox.js

+13-4
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,7 @@ proto.createMap = function(calcData, fullLayout, resolve, reject) {
152152
// duplicate 'plotly_relayout' events.
153153

154154
if(eventData.originalEvent || wheeling) {
155-
var update = {};
156-
update[self.id] = Lib.extendFlat({}, view);
157-
gd.emit('plotly_relayout', update);
155+
emitRelayoutFromView(view);
158156
}
159157
wheeling = false;
160158
});
@@ -195,6 +193,8 @@ proto.createMap = function(calcData, fullLayout, resolve, reject) {
195193
map.on('zoomstart', unhover);
196194

197195
map.on('dblclick', function() {
196+
gd.emit('plotly_doubleclick', null);
197+
198198
var viewInitial = self.viewInitial;
199199

200200
map.setCenter(convertCenter(viewInitial.center));
@@ -209,9 +209,18 @@ proto.createMap = function(calcData, fullLayout, resolve, reject) {
209209
opts._input.bearing = opts.bearing = viewNow.bearing;
210210
opts._input.pitch = opts.pitch = viewNow.pitch;
211211

212-
gd.emit('plotly_doubleclick', null);
212+
emitRelayoutFromView(viewNow);
213213
});
214214

215+
function emitRelayoutFromView(view) {
216+
var id = self.id;
217+
var evtData = {};
218+
for(var k in view) {
219+
evtData[id + '.' + k] = view[k];
220+
}
221+
gd.emit('plotly_relayout', evtData);
222+
}
223+
215224
// define clear select on map creation, to keep one ref per map,
216225
// so that map.on / map.off in updateFx works as expected
217226
self.clearSelect = function() {

‎src/traces/scattermapbox/defaults.js

-4
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
4141
handleMarkerDefaults(traceIn, traceOut, defaultColor, layout, coerce, {noLine: true});
4242

4343
// array marker.size and marker.color are only supported with circles
44-
4544
var marker = traceOut.marker;
46-
// we need mock marker.line object to make legends happy
47-
marker.line = {width: 0};
48-
4945
if(marker.symbol !== 'circle') {
5046
if(Lib.isArrayOrTypedArray(marker.size)) marker.size = marker.size[0];
5147
if(Lib.isArrayOrTypedArray(marker.color)) marker.color = marker.color[0];

‎test/jasmine/tests/mapbox_test.js

+19-16
Original file line numberDiff line numberDiff line change
@@ -887,11 +887,11 @@ describe('@noCI, mapbox plots', function() {
887887
it('should respond drag / scroll / double-click interactions', function(done) {
888888
var relayoutCnt = 0;
889889
var doubleClickCnt = 0;
890-
var updateData;
890+
var evtData;
891891

892-
gd.on('plotly_relayout', function(eventData) {
892+
gd.on('plotly_relayout', function(d) {
893893
relayoutCnt++;
894-
updateData = eventData;
894+
evtData = d;
895895
});
896896

897897
gd.on('plotly_doubleclick', function() {
@@ -906,43 +906,46 @@ describe('@noCI, mapbox plots', function() {
906906
});
907907
}
908908

909-
function assertLayout(center, zoom, opts) {
910-
var mapInfo = getMapInfo(gd),
911-
layout = gd.layout.mapbox;
909+
function _assertLayout(center, zoom) {
910+
var mapInfo = getMapInfo(gd);
911+
var layout = gd.layout.mapbox;
912912

913913
expect([mapInfo.center.lng, mapInfo.center.lat]).toBeCloseToArray(center);
914914
expect(mapInfo.zoom).toBeCloseTo(zoom);
915915

916916
expect([layout.center.lon, layout.center.lat]).toBeCloseToArray(center);
917917
expect(layout.zoom).toBeCloseTo(zoom);
918+
}
918919

919-
if(opts && opts.withUpdateData) {
920-
var mapboxUpdate = updateData.mapbox;
920+
function _assert(center, zoom) {
921+
_assertLayout(center, zoom);
921922

922-
expect([mapboxUpdate.center.lon, mapboxUpdate.center.lat]).toBeCloseToArray(center);
923-
expect(mapboxUpdate.zoom).toBeCloseTo(zoom);
924-
}
923+
expect([evtData['mapbox.center'].lon, evtData['mapbox.center'].lat]).toBeCloseToArray(center);
924+
expect(evtData['mapbox.zoom']).toBeCloseTo(zoom);
925925
}
926926

927-
assertLayout([-4.710, 19.475], 1.234);
927+
_assertLayout([-4.710, 19.475], 1.234);
928928

929929
var p1 = [pointPos[0] + 50, pointPos[1] - 20];
930930

931931
_drag(pointPos, p1, function() {
932-
expect(relayoutCnt).toEqual(1);
933-
assertLayout([-19.651, 13.751], 1.234, {withUpdateData: true});
932+
expect(relayoutCnt).toBe(1, 'relayout cnt');
933+
expect(doubleClickCnt).toBe(0, 'double click cnt');
934+
_assert([-19.651, 13.751], 1.234);
934935

935936
return _doubleClick(p1);
936937
})
937938
.then(function() {
939+
expect(relayoutCnt).toBe(2, 'relayout cnt');
938940
expect(doubleClickCnt).toBe(1, 'double click cnt');
939-
assertLayout([-4.710, 19.475], 1.234);
941+
_assert([-4.710, 19.475], 1.234);
940942

941943
return _scroll(pointPos);
942944
})
943945
.then(function() {
946+
expect(relayoutCnt).toBe(3, 'relayout cnt');
947+
expect(doubleClickCnt).toBe(1, 'double click cnt');
944948
expect(getMapInfo(gd).zoom).toBeGreaterThan(1.234);
945-
expect(relayoutCnt).toBe(2);
946949
})
947950
.catch(failTest)
948951
.then(done);

‎test/jasmine/tests/scattermapbox_test.js

+11
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,17 @@ describe('scattermapbox defaults', function() {
110110
expect(fullTrace.marker.color).toEqual(['red', 'green', 'blue']);
111111
expect(fullTrace.marker.size).toEqual([10, 20, 30]);
112112
});
113+
114+
it('should not fill *marker.line* in fullData while is not available', function() {
115+
var fullTrace = _supply({
116+
mode: 'markers',
117+
lon: [10, 20, 30],
118+
lat: [10, 20, 30]
119+
});
120+
121+
expect(fullTrace.marker).toBeDefined();
122+
expect(fullTrace.marker.line).toBeUndefined();
123+
});
113124
});
114125

115126
describe('scattermapbox convert', function() {

0 commit comments

Comments
 (0)