diff --git a/src/plots/mapbox/mapbox.js b/src/plots/mapbox/mapbox.js index 3c3185b1c44..6062376404b 100644 --- a/src/plots/mapbox/mapbox.js +++ b/src/plots/mapbox/mapbox.js @@ -111,10 +111,14 @@ proto.createMap = function(calcData, fullLayout, resolve, reject) { }); // clear navigation container - var className = constants.controlContainerClassName, - controlContainer = self.div.getElementsByClassName(className)[0]; + var className = constants.controlContainerClassName; + var controlContainer = self.div.getElementsByClassName(className)[0]; self.div.removeChild(controlContainer); + // make sure canvas does not inherit left and top css + map._canvas.canvas.style.left = '0px'; + map._canvas.canvas.style.top = '0px'; + self.rejectOnError(reject); map.once('load', function() { @@ -176,7 +180,6 @@ proto.createMap = function(calcData, fullLayout, resolve, reject) { map.on('dragstart', unhover); map.on('zoomstart', unhover); - }; proto.updateMap = function(calcData, fullLayout, resolve, reject) { diff --git a/src/plots/plots.js b/src/plots/plots.js index 6f4dfcf913e..51757322917 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -16,6 +16,7 @@ var Plotly = require('../plotly'); var Registry = require('../registry'); var Lib = require('../lib'); var Color = require('../components/color'); +var BADNUM = require('../constants/numerical').BADNUM; var plots = module.exports = {}; @@ -2012,11 +2013,8 @@ plots.doCalcdata = function(gd, traces) { // // This ensures there is a calcdata item for every trace, // even if cartesian logic doesn't handle it (for things like legends). - // - // Tag this artificial calc point with 'placeholder: true', - // to make it easier to skip over them in during the plot and hover step. if(!Array.isArray(cd) || !cd[0]) { - cd = [{x: false, y: false, placeholder: true}]; + cd = [{x: BADNUM, y: BADNUM}]; } // add the trace-wide properties to the first point, diff --git a/src/traces/scattermapbox/calc.js b/src/traces/scattermapbox/calc.js deleted file mode 100644 index 6f6230851e8..00000000000 --- a/src/traces/scattermapbox/calc.js +++ /dev/null @@ -1,101 +0,0 @@ -/** -* Copyright 2012-2017, Plotly, Inc. -* All rights reserved. -* -* This source code is licensed under the MIT license found in the -* LICENSE file in the root directory of this source tree. -*/ - - -'use strict'; - -var isNumeric = require('fast-isnumeric'); - -var Lib = require('../../lib'); -var Colorscale = require('../../components/colorscale'); - -var subtypes = require('../scatter/subtypes'); -var calcMarkerColorscale = require('../scatter/colorscale_calc'); -var makeBubbleSizeFn = require('../scatter/make_bubble_size_func'); - - -module.exports = function calc(gd, trace) { - var len = trace.lon.length, - marker = trace.marker; - - var hasMarkers = subtypes.hasMarkers(trace), - hasColorArray = (hasMarkers && Array.isArray(marker.color)), - hasSizeArray = (hasMarkers && Array.isArray(marker.size)), - hasSymbolArray = (hasMarkers && Array.isArray(marker.symbol)), - hasTextArray = Array.isArray(trace.text); - - calcMarkerColorscale(trace); - - var colorFn = Colorscale.hasColorscale(trace, 'marker') ? - Colorscale.makeColorScaleFunc( - Colorscale.extractScale( - marker.colorscale, - marker.cmin, - marker.cmax - ) - ) : - Lib.identity; - - var sizeFn = subtypes.isBubble(trace) ? - makeBubbleSizeFn(trace) : - Lib.identity; - - var calcTrace = [], - cnt = 0; - - // Different than cartesian calc step - // as skip over non-numeric lon, lat pairs. - // This makes the hover and convert calculations simpler. - - for(var i = 0; i < len; i++) { - var lon = trace.lon[i], - lat = trace.lat[i]; - - if(!isNumeric(lon) || !isNumeric(lat)) { - if(cnt > 0) calcTrace[cnt - 1].gapAfter = true; - continue; - } - - var calcPt = {}; - cnt++; - - // coerce numeric strings into numbers - calcPt.lonlat = [+lon, +lat]; - - if(hasMarkers) { - - if(hasColorArray) { - var mc = marker.color[i]; - - calcPt.mc = mc; - calcPt.mcc = colorFn(mc); - } - - if(hasSizeArray) { - var ms = marker.size[i]; - - calcPt.ms = ms; - calcPt.mrc = sizeFn(ms); - } - - if(hasSymbolArray) { - var mx = marker.symbol[i]; - calcPt.mx = (typeof mx === 'string') ? mx : 'circle'; - } - } - - if(hasTextArray) { - var tx = trace.text[i]; - calcPt.tx = (typeof tx === 'string') ? tx : ''; - } - - calcTrace.push(calcPt); - } - - return calcTrace; -}; diff --git a/src/traces/scattermapbox/convert.js b/src/traces/scattermapbox/convert.js index 3e46c8d8950..e4c17f91973 100644 --- a/src/traces/scattermapbox/convert.js +++ b/src/traces/scattermapbox/convert.js @@ -10,8 +10,11 @@ 'use strict'; var Lib = require('../../lib'); +var BADNUM = require('../../constants/numerical').BADNUM; var geoJsonUtils = require('../../lib/geojson_utils'); +var Colorscale = require('../../components/colorscale'); +var makeBubbleSizeFn = require('../scatter/make_bubble_size_func'); var subTypes = require('../scatter/subtypes'); var convertTextOpts = require('../../plots/mapbox/convert_text_opts'); @@ -43,16 +46,16 @@ module.exports = function convert(calcTrace) { }; // early return if not visible or placeholder - if(!isVisible || calcTrace[0].placeholder) return opts; + if(!isVisible) return opts; // fill layer and line layer use the same coords - var coords; + var lineCoords; if(hasFill || hasLines) { - coords = geoJsonUtils.calcTraceToLineCoords(calcTrace); + lineCoords = geoJsonUtils.calcTraceToLineCoords(calcTrace); } if(hasFill) { - fill.geojson = geoJsonUtils.makePolygon(coords); + fill.geojson = geoJsonUtils.makePolygon(lineCoords); fill.layout.visibility = 'visible'; Lib.extendFlat(fill.paint, { @@ -61,7 +64,7 @@ module.exports = function convert(calcTrace) { } if(hasLines) { - line.geojson = geoJsonUtils.makeLine(coords); + line.geojson = geoJsonUtils.makeLine(lineCoords); line.layout.visibility = 'visible'; Lib.extendFlat(line.paint, { @@ -155,12 +158,30 @@ function initContainer() { // // The solution prove to be more robust than trying to generate // `stops` arrays from scale functions. +// +// TODO axe this when we bump mapbox-gl and rewrite this using +// "identity" property functions. +// See https://github.com/plotly/plotly.js/pull/1543 +// function makeCircleGeoJSON(calcTrace, hash) { var trace = calcTrace[0].trace; + var marker = trace.marker; + + var colorFn; + if(Colorscale.hasColorscale(trace, 'marker')) { + colorFn = Colorscale.makeColorScaleFunc( + Colorscale.extractScale(marker.colorscale, marker.cmin, marker.cmax) + ); + } else if(Array.isArray(marker.color)) { + colorFn = Lib.identity; + } - var marker = trace.marker, - hasColorArray = Array.isArray(marker.color), - hasSizeArray = Array.isArray(marker.size); + var sizeFn; + if(subTypes.isBubble(trace)) { + sizeFn = makeBubbleSizeFn(trace); + } else if(Array.isArray(marker.size)) { + sizeFn = Lib.identity; + } // Translate vals in trace arrayOk containers // into a val-to-index hash object @@ -174,16 +195,19 @@ function makeCircleGeoJSON(calcTrace, hash) { for(var i = 0; i < calcTrace.length; i++) { var calcPt = calcTrace[i]; + var lonlat = calcPt.lonlat; + + if(isBADNUM(lonlat)) continue; var props = {}; - if(hasColorArray) translate(props, COLOR_PROP, calcPt.mcc, i); - if(hasSizeArray) translate(props, SIZE_PROP, calcPt.mrc, i); + if(colorFn) translate(props, COLOR_PROP, colorFn(calcPt.mc), i); + if(sizeFn) translate(props, SIZE_PROP, sizeFn(calcPt.ms), i); features.push({ type: 'Feature', geometry: { type: 'Point', - coordinates: calcPt.lonlat + coordinates: lonlat }, properties: props }); @@ -215,6 +239,8 @@ function makeSymbolGeoJSON(calcTrace) { for(var i = 0; i < calcTrace.length; i++) { var calcPt = calcTrace[i]; + if(isBADNUM(calcPt.lonlat)) continue; + features.push({ type: 'Feature', geometry: { @@ -305,3 +331,8 @@ function getFillFunc(attr) { } function blankFillFunc() { return ''; } + +// only need to check lon (OR lat) +function isBADNUM(lonlat) { + return lonlat[0] === BADNUM; +} diff --git a/src/traces/scattermapbox/hover.js b/src/traces/scattermapbox/hover.js index 088d35f6dc5..2bbc77e3ec1 100644 --- a/src/traces/scattermapbox/hover.js +++ b/src/traces/scattermapbox/hover.js @@ -11,7 +11,7 @@ var Fx = require('../../plots/cartesian/graph_interact'); var getTraceColor = require('../scatter/get_trace_color'); - +var BADNUM = require('../../constants/numerical').BADNUM; module.exports = function hoverPoints(pointData, xval, yval) { var cd = pointData.cd, @@ -19,8 +19,6 @@ module.exports = function hoverPoints(pointData, xval, yval) { xa = pointData.xa, ya = pointData.ya; - if(cd[0].placeholder) return; - // compute winding number about [-180, 180] globe var winding = (xval >= 0) ? Math.floor((xval + 180) / 360) : @@ -31,10 +29,13 @@ module.exports = function hoverPoints(pointData, xval, yval) { var xval2 = xval - lonShift; function distFn(d) { - var lonlat = d.lonlat, - dx = Math.abs(xa.c2p(lonlat) - xa.c2p([xval2, lonlat[1]])), - dy = Math.abs(ya.c2p(lonlat) - ya.c2p([lonlat[0], yval])), - rad = Math.max(3, d.mrc || 0); + var lonlat = d.lonlat; + + if(lonlat[0] === BADNUM) return Infinity; + + var dx = Math.abs(xa.c2p(lonlat) - xa.c2p([xval2, lonlat[1]])); + var dy = Math.abs(ya.c2p(lonlat) - ya.c2p([lonlat[0], yval])); + var rad = Math.max(3, d.mrc || 0); return Math.max(Math.sqrt(dx * dx + dy * dy) - rad, 1 - 3 / rad); } diff --git a/src/traces/scattermapbox/index.js b/src/traces/scattermapbox/index.js index fa32f9847a0..6de4241ed82 100644 --- a/src/traces/scattermapbox/index.js +++ b/src/traces/scattermapbox/index.js @@ -14,7 +14,7 @@ var ScatterMapbox = {}; ScatterMapbox.attributes = require('./attributes'); ScatterMapbox.supplyDefaults = require('./defaults'); ScatterMapbox.colorbar = require('../scatter/colorbar'); -ScatterMapbox.calc = require('./calc'); +ScatterMapbox.calc = require('../scattergeo/calc'); ScatterMapbox.hoverPoints = require('./hover'); ScatterMapbox.eventData = require('./event_data'); ScatterMapbox.plot = require('./plot'); diff --git a/test/image/baselines/mapbox_0.png b/test/image/baselines/mapbox_0.png index 80a431490f0..c5378b6cf85 100644 Binary files a/test/image/baselines/mapbox_0.png and b/test/image/baselines/mapbox_0.png differ diff --git a/test/image/baselines/mapbox_angles.png b/test/image/baselines/mapbox_angles.png index 611a6fc132f..54eae19c19f 100644 Binary files a/test/image/baselines/mapbox_angles.png and b/test/image/baselines/mapbox_angles.png differ diff --git a/test/image/baselines/mapbox_bubbles-text.png b/test/image/baselines/mapbox_bubbles-text.png index a0613245a06..01a663867f0 100644 Binary files a/test/image/baselines/mapbox_bubbles-text.png and b/test/image/baselines/mapbox_bubbles-text.png differ diff --git a/test/image/baselines/mapbox_custom-style.png b/test/image/baselines/mapbox_custom-style.png index 7213f229c04..23d2ed05b92 100644 Binary files a/test/image/baselines/mapbox_custom-style.png and b/test/image/baselines/mapbox_custom-style.png differ diff --git a/test/image/baselines/mapbox_fill.png b/test/image/baselines/mapbox_fill.png index ea1a2370a7d..98fc78f86bd 100644 Binary files a/test/image/baselines/mapbox_fill.png and b/test/image/baselines/mapbox_fill.png differ diff --git a/test/image/baselines/mapbox_layers.png b/test/image/baselines/mapbox_layers.png index 4d39e4de195..95da9fd0db4 100644 Binary files a/test/image/baselines/mapbox_layers.png and b/test/image/baselines/mapbox_layers.png differ diff --git a/test/image/baselines/mapbox_symbol-text.png b/test/image/baselines/mapbox_symbol-text.png index 703b49681f2..1ec7cd40459 100644 Binary files a/test/image/baselines/mapbox_symbol-text.png and b/test/image/baselines/mapbox_symbol-text.png differ diff --git a/test/jasmine/tests/scattermapbox_test.js b/test/jasmine/tests/scattermapbox_test.js index 88aa8adfdb0..ac2c8503648 100644 --- a/test/jasmine/tests/scattermapbox_test.js +++ b/test/jasmine/tests/scattermapbox_test.js @@ -112,143 +112,6 @@ describe('scattermapbox defaults', function() { }); }); -describe('scattermapbox calc', function() { - 'use strict'; - - function _calc(trace) { - var gd = { data: [trace] }; - - Plots.supplyDefaults(gd); - - var fullTrace = gd._fullData[0]; - return ScatterMapbox.calc(gd, fullTrace); - } - - var base = { type: 'scattermapbox' }; - - it('should place lon/lat data in lonlat pairs', function() { - var calcTrace = _calc(Lib.extendFlat({}, base, { - lon: [10, 20, 30], - lat: [20, 30, 10] - })); - - expect(calcTrace).toEqual([ - { lonlat: [10, 20] }, - { lonlat: [20, 30] }, - { lonlat: [30, 10] } - ]); - }); - - it('should coerce numeric strings lon/lat data into numbers', function() { - var calcTrace = _calc(Lib.extendFlat({}, base, { - lon: [10, 20, '30', '40'], - lat: [20, '30', 10, '50'] - })); - - expect(calcTrace).toEqual([ - { lonlat: [10, 20] }, - { lonlat: [20, 30] }, - { lonlat: [30, 10] }, - { lonlat: [40, 50] } - ]); - }); - - it('should keep track of gaps in data', function() { - var calcTrace = _calc(Lib.extendFlat({}, base, { - lon: [null, 10, null, null, 20, '30', null, '40', null, 10], - lat: [10, 20, '30', null, 10, '50', null, 60, null, null] - })); - - expect(calcTrace).toEqual([ - { lonlat: [10, 20], gapAfter: true }, - { lonlat: [20, 10] }, - { lonlat: [30, 50], gapAfter: true }, - { lonlat: [40, 60], gapAfter: true } - ]); - }); - - it('should fill array text (base case)', function() { - var calcTrace = _calc(Lib.extendFlat({}, base, { - lon: [10, 20, 30], - lat: [20, 30, 10], - text: ['A', 'B', 'C'] - })); - - expect(calcTrace).toEqual([ - { lonlat: [10, 20], tx: 'A' }, - { lonlat: [20, 30], tx: 'B' }, - { lonlat: [30, 10], tx: 'C' } - ]); - }); - - it('should fill array text (invalid entry case)', function() { - var calcTrace = _calc(Lib.extendFlat({}, base, { - lon: [10, 20, 30], - lat: [20, 30, 10], - text: ['A', 'B', null] - })); - - expect(calcTrace).toEqual([ - { lonlat: [10, 20], tx: 'A' }, - { lonlat: [20, 30], tx: 'B' }, - { lonlat: [30, 10], tx: '' } - ]); - }); - - it('should fill array marker attributes (base case)', function() { - var calcTrace = _calc(Lib.extendFlat({}, base, { - lon: [10, 20, null, 30], - lat: [20, 30, null, 10], - marker: { - color: ['red', 'blue', 'green', 'yellow'], - size: [10, 20, 8, 10] - } - })); - - expect(calcTrace).toEqual([ - { lonlat: [10, 20], mc: 'red', ms: 10, mcc: 'red', mrc: 5 }, - { lonlat: [20, 30], mc: 'blue', ms: 20, mcc: 'blue', mrc: 10, gapAfter: true }, - { lonlat: [30, 10], mc: 'yellow', ms: 10, mcc: 'yellow', mrc: 5 } - ]); - }); - - it('should fill array marker attributes (invalid scale case)', function() { - var calcTrace = _calc(Lib.extendFlat({}, base, { - lon: [10, 20, null, 30], - lat: [20, 30, null, 10], - marker: { - color: [0, null, 5, 10], - size: [10, NaN, 8, 10], - colorscale: [ - [0, 'blue'], [0.5, 'red'], [1, 'green'] - ] - } - })); - - expect(calcTrace).toEqual([ - { lonlat: [10, 20], mc: 0, ms: 10, mcc: 'rgb(0, 0, 255)', mrc: 5 }, - { lonlat: [20, 30], mc: null, ms: NaN, mcc: '#444', mrc: 0, gapAfter: true }, - { lonlat: [30, 10], mc: 10, ms: 10, mcc: 'rgb(0, 128, 0)', mrc: 5 } - ]); - }); - - it('should fill marker attributes (symbol case)', function() { - var calcTrace = _calc(Lib.extendFlat({}, base, { - lon: [10, 20, null, 30], - lat: [20, 30, null, 10], - marker: { - symbol: ['monument', 'music', 'harbor', null] - } - })); - - expect(calcTrace).toEqual([ - { lonlat: [10, 20], mx: 'monument' }, - { lonlat: [20, 30], mx: 'music', gapAfter: true }, - { lonlat: [30, 10], mx: 'circle' } - ]); - }); -}); - describe('scattermapbox convert', function() { 'use strict'; @@ -376,7 +239,7 @@ describe('scattermapbox convert', function() { return f.properties.text; }); - expect(actualText).toEqual(['A', 'B', 'C', 'F', '']); + expect(actualText).toEqual(['A', 'B', 'C', 'F', undefined]); }); it('should generate correct output for lines traces with trailing gaps', function() { @@ -451,7 +314,9 @@ describe('scattermapbox convert', function() { fill: 'toself' })); - assertVisibility(opts, ['none', 'none', 'none', 'none']); + // not optimal, but doesn't break anything as mapbox-gl accepts empty + // coordinate arrays + assertVisibility(opts, ['visible', 'visible', 'none', 'none']); expect(opts.line.geojson.coordinates).toEqual([], 'line coords'); expect(opts.fill.geojson.coordinates).toEqual([], 'fill coords');