Skip to content

Commit 0e50599

Browse files
committed
address minor PR comments
- guard against empty subplot list in modebar button handlers - fix 'when' -> 'went' typo in error msg - guard against (potential) infinite fitbounds scales - improve `geojson` attr description - fixup `projection.rotation` fallback in an assertion wrapper
1 parent ef28e78 commit 0e50599

File tree

8 files changed

+24
-18
lines changed

8 files changed

+24
-18
lines changed

src/components/modebar/buttons.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ function handleDrag3d(gd, ev) {
303303
var button = ev.currentTarget;
304304
var attr = button.getAttribute('data-attr');
305305
var val = button.getAttribute('data-val') || true;
306-
var sceneIds = gd._fullLayout._subplots.gl3d;
306+
var sceneIds = gd._fullLayout._subplots.gl3d || [];
307307
var layoutUpdate = {};
308308

309309
var parts = attr.split('.');
@@ -468,7 +468,7 @@ function handleGeo(gd, ev) {
468468
var attr = button.getAttribute('data-attr');
469469
var val = button.getAttribute('data-val') || true;
470470
var fullLayout = gd._fullLayout;
471-
var geoIds = fullLayout._subplots.geo;
471+
var geoIds = fullLayout._subplots.geo || [];
472472

473473
for(var i = 0; i < geoIds.length; i++) {
474474
var id = geoIds[i];

src/lib/geo_location_utils.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -187,14 +187,14 @@ function feature2polygons(feature) {
187187
}
188188

189189
function getTraceGeojson(trace) {
190-
var geojsonIn = typeof trace.geojson === 'string' ?
191-
(window.PlotlyGeoAssets || {})[trace.geojson] :
192-
trace.geojson;
190+
var g = trace.geojson;
191+
var PlotlyGeoAssets = window.PlotlyGeoAssets || {};
192+
var geojsonIn = typeof g === 'string' ? PlotlyGeoAssets[g] : g;
193193

194194
// This should not happen, but just in case something goes
195195
// really wrong when fetching the GeoJSON
196196
if(!isPlainObject(geojsonIn)) {
197-
loggers.error('Oops ... something when wrong when fetching ' + trace.geojson);
197+
loggers.error('Oops ... something went wrong when fetching ' + g);
198198
return false;
199199
}
200200

src/plots/geo/geo.js

+6-1
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,12 @@ proto.updateProjection = function(geoCalcData, fullLayout) {
282282
(b[1][0] - b[0][0]) / (b2[1][0] - b2[0][0]),
283283
(b[1][1] - b[0][1]) / (b2[1][1] - b2[0][1])
284284
);
285-
projection.scale(k2 * s);
285+
286+
if(isFinite(k2)) {
287+
projection.scale(k2 * s);
288+
} else {
289+
Lib.warn('Something went wrong during' + this.id + 'fitbounds computations.');
290+
}
286291
} else {
287292
// adjust projection to user setting
288293
projection.scale(projLayout.scale * s);

src/traces/choropleth/attributes.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@ module.exports = extendFlat({
3838
'Sets optional GeoJSON data associated with this trace.',
3939
'If not given, the features on the base map are used.',
4040

41-
'Can be set as a valid GeoJSON object or as URL string',
42-
'Note that we only accept GeoJSON of type *FeatureCollection* and *Feature*',
43-
'with geometries of type *Polygon* and *MultiPolygon*.'
41+
'It can be set as a valid GeoJSON object or as a URL string.',
42+
'Note that we only accept GeoJSONs of type *FeatureCollection* or *Feature*',
43+
'with geometries of type *Polygon* or *MultiPolygon*.'
4444

4545
// TODO add topojson support with additional 'topojsonobject' attr?
4646
// https://github.com/topojson/topojson-specification/blob/master/README.md

src/traces/choroplethmapbox/attributes.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,10 @@ module.exports = extendFlat({
4545
editType: 'calc',
4646
description: [
4747
'Sets the GeoJSON data associated with this trace.',
48-
'Can be set as a valid GeoJSON object or as URL string',
49-
'Note that we only accept GeoJSON of type *FeatureCollection* and *Feature*',
50-
'with geometries of type *Polygon* and *MultiPolygon*.'
48+
49+
'It can be set as a valid GeoJSON object or as a URL string.',
50+
'Note that we only accept GeoJSONs of type *FeatureCollection* or *Feature*',
51+
'with geometries of type *Polygon* or *MultiPolygon*.'
5152
].join(' ')
5253
},
5354
featureidkey: extendFlat({}, choroplethAttrs.featureidkey, {

src/traces/scattergeo/attributes.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,9 @@ module.exports = overrideAll({
6262
'Sets optional GeoJSON data associated with this trace.',
6363
'If not given, the features on the base map are used when `locations` is set.',
6464

65-
'Can be set as a valid GeoJSON object or as URL string',
66-
'Note that we only accept GeoJSON of type *FeatureCollection* and *Feature*',
67-
'with geometries of type *Polygon* and *MultiPolygon*.'
65+
'It can be set as a valid GeoJSON object or as a URL string.',
66+
'Note that we only accept GeoJSONs of type *FeatureCollection* or *Feature*',
67+
'with geometries of type *Polygon* or *MultiPolygon*.'
6868

6969
// TODO add topojson support with additional 'topojsonobject' attr?
7070
// https://github.com/topojson/topojson-specification/blob/master/README.md

test/jasmine/tests/choroplethmapbox_test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ describe('Test choroplethmapbox convert:', function() {
158158
geojson: 'url'
159159
});
160160

161-
expect(loggers.error).toHaveBeenCalledWith('Oops ... something when wrong when fetching url');
161+
expect(loggers.error).toHaveBeenCalledWith('Oops ... something went wrong when fetching url');
162162
expectBlank(opts);
163163
});
164164

test/jasmine/tests/geo_test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -2114,7 +2114,7 @@ describe('Test geo zoom/pan/drag interactions:', function() {
21142114
var msg = '[' + step + '] ';
21152115

21162116
var geoLayout = gd._fullLayout.geo;
2117-
var rotation = geoLayout.projection.rotation || [];
2117+
var rotation = geoLayout.projection.rotation || {};
21182118
var scale = geoLayout.projection.scale;
21192119

21202120
expect([rotation.lon, rotation.lat]).toBeCloseToArray(attr[0], -0.5, msg + 'rotation.(lon|lat)');

0 commit comments

Comments
 (0)