From 9c7254887954c77c8010d4896babd57fd9d703d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 22 Jun 2016 10:12:48 -0400 Subject: [PATCH 01/16] make scattermapbox defaults compatible with lineDefaults again --- src/traces/scattermapbox/defaults.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/traces/scattermapbox/defaults.js b/src/traces/scattermapbox/defaults.js index 8368ddabede..aa9dfde2a53 100644 --- a/src/traces/scattermapbox/defaults.js +++ b/src/traces/scattermapbox/defaults.js @@ -45,7 +45,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout coerce('mode'); if(subTypes.hasLines(traceOut)) { - handleLineDefaults(traceIn, traceOut, defaultColor, coerce); + handleLineDefaults(traceIn, traceOut, defaultColor, layout, coerce); coerce('connectgaps'); } From 4a8563b5dcf3045b0407e9f7fe6acb4aaa4054ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 22 Jun 2016 12:46:19 -0400 Subject: [PATCH 02/16] add webgl-support check in mapbox test suites - to skip over test the require a gl context, if gl isn't supported - allows us to run _some_ mapbox tests on CircleCI --- test/jasmine/assets/has_webgl_support.js | 22 ++++++++++++++++++++++ test/jasmine/karma.ciconf.js | 4 +--- test/jasmine/tests/mapbox_test.js | 5 +++++ test/jasmine/tests/scattermapbox_test.js | 9 ++++++--- 4 files changed, 34 insertions(+), 6 deletions(-) create mode 100644 test/jasmine/assets/has_webgl_support.js diff --git a/test/jasmine/assets/has_webgl_support.js b/test/jasmine/assets/has_webgl_support.js new file mode 100644 index 00000000000..dcf9f2ec3d1 --- /dev/null +++ b/test/jasmine/assets/has_webgl_support.js @@ -0,0 +1,22 @@ +'use strict'; + + +module.exports = function hasWebGLSupport(testName) { + var gl, canvas; + + try { + canvas = document.createElement('canvas'); + gl = canvas.getContext('webgl'); + } + catch(err) { + gl = null; + } + + var hasSupport = !!gl; + + if(!hasSupport) { + console.warn('Cannot get WebGL context. Skip test *' + testName + '*'); + } + + return hasSupport; +}; diff --git a/test/jasmine/karma.ciconf.js b/test/jasmine/karma.ciconf.js index 38f63ccac97..e735ef639ad 100644 --- a/test/jasmine/karma.ciconf.js +++ b/test/jasmine/karma.ciconf.js @@ -17,9 +17,7 @@ function func(config) { func.defaultConfig.exclude = [ 'tests/gl_plot_interact_test.js', 'tests/gl_plot_interact_basic_test.js', - 'tests/gl2d_scatterplot_contour_test.js', - 'tests/mapbox_test.js', - 'tests/scattermapbox_test.js' + 'tests/gl2d_scatterplot_contour_test.js' ]; // if true, Karma captures browsers, runs the tests and exits diff --git a/test/jasmine/tests/mapbox_test.js b/test/jasmine/tests/mapbox_test.js index b335b861f95..ab71da4b007 100644 --- a/test/jasmine/tests/mapbox_test.js +++ b/test/jasmine/tests/mapbox_test.js @@ -7,6 +7,7 @@ var supplyLayoutDefaults = require('@src/plots/mapbox/layout_defaults'); var d3 = require('d3'); var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); +var hasWebGLSupport = require('../assets/has_webgl_support'); var mouseEvent = require('../assets/mouse_event'); var customMatchers = require('../assets/custom_matchers'); @@ -121,6 +122,8 @@ describe('mapbox defaults', function() { describe('mapbox credentials', function() { 'use strict'; + if(!hasWebGLSupport('scattermapbox hover')) return; + var dummyToken = 'asfdsa124331wersdsa1321q3'; var gd; @@ -168,6 +171,8 @@ describe('mapbox credentials', function() { describe('mapbox plots', function() { 'use strict'; + if(!hasWebGLSupport('scattermapbox hover')) return; + var mock = require('@mocks/mapbox_0.json'), gd; diff --git a/test/jasmine/tests/scattermapbox_test.js b/test/jasmine/tests/scattermapbox_test.js index 5aa47a1b385..7607d8f1739 100644 --- a/test/jasmine/tests/scattermapbox_test.js +++ b/test/jasmine/tests/scattermapbox_test.js @@ -7,6 +7,7 @@ var convert = require('@src/traces/scattermapbox/convert'); var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); +var hasWebGLSupport = require('../assets/has_webgl_support'); var customMatchers = require('../assets/custom_matchers'); // until it is part of the main plotly.js bundle @@ -410,6 +411,8 @@ describe('scattermapbox convert', function() { describe('scattermapbox hover', function() { 'use strict'; + if(!hasWebGLSupport('scattermapbox hover')) return; + var hoverPoints = ScatterMapbox.hoverPoints; var gd; @@ -456,7 +459,7 @@ describe('scattermapbox hover', function() { expect(out.index).toEqual(0); expect([out.x0, out.x1, out.y0, out.y1]).toBeCloseToArray([ - 444.444, 446.444, 105.410, 107.410 + 297.444, 299.444, 105.410, 107.410 ]); expect(out.extraText).toEqual('(10°, 10°)
A'); expect(out.color).toEqual('#1f77b4'); @@ -470,7 +473,7 @@ describe('scattermapbox hover', function() { expect(out.index).toEqual(0); expect([out.x0, out.x1, out.y0, out.y1]).toBeCloseToArray([ - 2492.444, 2494.444, 105.410, 107.410 + 2345.444, 2347.444, 105.410, 107.410 ]); expect(out.extraText).toEqual('(10°, 10°)
A'); expect(out.color).toEqual('#1f77b4'); @@ -484,7 +487,7 @@ describe('scattermapbox hover', function() { expect(out.index).toEqual(0); expect([out.x0, out.x1, out.y0, out.y1]).toBeCloseToArray([ - -2627.555, -2625.555, 105.410, 107.410 + -2774.555, -2772.555, 105.410, 107.410 ]); expect(out.extraText).toEqual('(10°, 10°)
A'); expect(out.color).toEqual('#1f77b4'); From 32973fc9520ae893c116256d2474d8ec0bbc0209 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 22 Jun 2016 16:01:46 -0400 Subject: [PATCH 03/16] add layer type 'circle' : - so that Point geojson can be displayed as mapbox circle layers - change layer attribute to be more one-to-one with the mapbox api, where styles of different layer types will be set in different container objects --- src/plots/mapbox/layers.js | 21 ++++++-- src/plots/mapbox/layout_attributes.js | 74 +++++++++++++++++++++++---- src/plots/mapbox/layout_defaults.js | 29 ++++++++--- test/image/mocks/mapbox_layers.json | 4 +- test/jasmine/tests/mapbox_test.js | 17 +++--- 5 files changed, 113 insertions(+), 32 deletions(-) diff --git a/src/plots/mapbox/layers.js b/src/plots/mapbox/layers.js index 64b95d6b966..b63b66b3706 100644 --- a/src/plots/mapbox/layers.js +++ b/src/plots/mapbox/layers.js @@ -126,21 +126,32 @@ function convertPaintOpts(opts) { switch(opts.type) { + case 'circle': + var circle = opts.circle; + Lib.extendFlat(paintOpts, { + 'circle-radius': circle.radius, + 'circle-color': circle.color, + 'circle-opacity': opts.opacity + }); + break; + case 'line': + var line = opts.line; Lib.extendFlat(paintOpts, { - 'line-width': opts.line.width, - 'line-color': opts.line.color, + 'line-width': line.width, + 'line-color': line.color, 'line-opacity': opts.opacity }); break; case 'fill': + var fill = opts.fill; Lib.extendFlat(paintOpts, { - 'fill-color': opts.fillcolor, - 'fill-outline-color': opts.line.color, + 'fill-color': fill.color, + 'fill-outline-color': fill.outlinecolor, 'fill-opacity': opts.opacity - // no way to pass line.width at the moment + // no way to pass specify outline width at the moment }); break; } diff --git a/src/plots/mapbox/layout_attributes.js b/src/plots/mapbox/layout_attributes.js index 9386335395b..2f9dd0ac916 100644 --- a/src/plots/mapbox/layout_attributes.js +++ b/src/plots/mapbox/layout_attributes.js @@ -9,11 +9,7 @@ 'use strict'; -var scatterMapboxAttrs = require('../../traces/scattermapbox/attributes'); var defaultLine = require('../../components/color').defaultLine; -var extendFlat = require('../../lib').extendFlat; - -var lineAttrs = scatterMapboxAttrs.line; module.exports = { @@ -129,12 +125,14 @@ module.exports = { type: { valType: 'enumerated', - values: ['line', 'fill'], + values: ['circle', 'line', 'fill'], dflt: 'line', role: 'info', description: [ 'Sets the layer type.', - 'Support for *raster*, *background* types is coming soon.' + 'Support for *raster*, *background* types is coming soon.', + 'Note that *line* and *fill* are not compatible with Point', + 'GeoJSON geometry.' ].join(' ') }, @@ -150,14 +148,68 @@ module.exports = { ].join(' ') }, + circle: { + radius: { + valType: 'number', + dflt: 15, + role: 'style', + description: [ + 'Sets the circle radius.', + 'Has an effect only when `type` is set to *circle*.' + ].join(' ') + }, + color: { + valType: 'color', + dflt: defaultLine, + role: 'style', + description: [ + 'Sets the circle color.', + 'Has an effect only when `type` is set to *circle*.' + ].join(' ') + } + }, + line: { - color: extendFlat({}, lineAttrs.color, { - dflt: defaultLine - }), - width: lineAttrs.width + width: { + valType: 'number', + dflt: 2, + role: 'style', + description: [ + 'Sets the line radius.', + 'Has an effect only when `type` is set to *line*.' + ].join(' ') + }, + color: { + valType: 'color', + dflt: defaultLine, + role: 'style', + description: [ + 'Sets the line color.', + 'Has an effect only when `type` is set to *line*.' + ].join(' ') + } }, - fillcolor: scatterMapboxAttrs.fillcolor, + fill: { + color: { + valType: 'color', + dflt: defaultLine, + role: 'style', + description: [ + 'Sets the fill color.', + 'Has an effect only when `type` is set to *fill*.' + ].join(' ') + }, + outlinecolor: { + valType: 'color', + dflt: defaultLine, + role: 'style', + description: [ + 'Sets the fill outline color.', + 'Has an effect only when `type` is set to *fill*.' + ].join(' ') + } + }, opacity: { valType: 'number', diff --git a/src/plots/mapbox/layout_defaults.js b/src/plots/mapbox/layout_defaults.js index 0a20ce81f45..cbaaeae2517 100644 --- a/src/plots/mapbox/layout_defaults.js +++ b/src/plots/mapbox/layout_defaults.js @@ -49,7 +49,7 @@ function handleLayerDefaults(containerIn, containerOut) { } for(var i = 0; i < layersIn.length; i++) { - layerIn = layersIn[i]; + layerIn = layersIn[i] || {}; layerOut = {}; var sourceType = coerce('sourcetype'); @@ -57,18 +57,31 @@ function handleLayerDefaults(containerIn, containerOut) { if(sourceType === 'vector') coerce('sourcelayer'); - // maybe add smart default based off 'fillcolor' ??? + // maybe add smart default based off GeoJSON geometry var type = coerce('type'); - var lineColor; - if(type === 'line' || type === 'fill') { - lineColor = coerce('line.color'); + var dfltColor; + + if(type === 'circle') { + dfltColor = (layerIn.line || {}).color || (layerIn.fill || {}).color; + + coerce('circle.color', dfltColor); + coerce('circle.radius'); + } + + if(type === 'line') { + dfltColor = (layerIn.circle || {}).color || (layerIn.fill || {}).color; + + coerce('line.color', dfltColor); + coerce('line.width'); } - // no way to pass line.width to fill layers - if(type === 'line') coerce('line.width'); + if(type === 'fill') { + dfltColor = (layerIn.circle || {}).color || (layerIn.line || {}).color; - if(type === 'fill') coerce('fillcolor', lineColor); + coerce('fill.color', dfltColor); + coerce('fill.outlinecolor', dfltColor); + } coerce('below'); coerce('opacity'); diff --git a/test/image/mocks/mapbox_layers.json b/test/image/mocks/mapbox_layers.json index 9d98da7dcc8..89861df8aae 100644 --- a/test/image/mocks/mapbox_layers.json +++ b/test/image/mocks/mapbox_layers.json @@ -532,7 +532,9 @@ }, "type": "fill", "below": "water", - "fillcolor": "#ece2f0", + "fill": { + "color": "#ece2f0" + }, "opacity": 0.8 }, { diff --git a/test/jasmine/tests/mapbox_test.js b/test/jasmine/tests/mapbox_test.js index ab71da4b007..8ceb80bbbed 100644 --- a/test/jasmine/tests/mapbox_test.js +++ b/test/jasmine/tests/mapbox_test.js @@ -102,7 +102,10 @@ describe('mapbox defaults', function() { color: 'red', width: 3 }, - fillcolor: 'blue' + fill: { + color: 'blue', + outlinecolor: 'green' + } }] } }; @@ -111,11 +114,11 @@ describe('mapbox defaults', function() { expect(layoutOut.mapbox.layers[0].line.color).toEqual('red'); expect(layoutOut.mapbox.layers[0].line.width).toEqual(3); - expect(layoutOut.mapbox.layers[0].fillcolor).toBeUndefined(); + expect(layoutOut.mapbox.layers[0].fill).toBeUndefined(); - expect(layoutOut.mapbox.layers[1].line.color).toEqual('red'); - expect(layoutOut.mapbox.layers[1].line.width).toBeUndefined(); - expect(layoutOut.mapbox.layers[1].fillcolor).toEqual('blue'); + expect(layoutOut.mapbox.layers[1].line).toBeUndefined(); + expect(layoutOut.mapbox.layers[1].fill.color).toEqual('blue'); + expect(layoutOut.mapbox.layers[1].fill.outlinecolor).toEqual('green'); }); }); @@ -353,8 +356,8 @@ describe('mapbox plots', function() { }; var styleUpdate0 = { - 'mapbox.layers[0].fillcolor': 'red', - 'mapbox.layers[0].line.color': 'blue', + 'mapbox.layers[0].fill.color': 'red', + 'mapbox.layers[0].fill.outlinecolor': 'blue', 'mapbox.layers[0].opacity': 0.3 }; From 17f3378053022aed5b9aff2bbda9e80b68075f18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 22 Jun 2016 16:02:04 -0400 Subject: [PATCH 04/16] add special case to needsNewSource logic --- src/plots/mapbox/layers.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/plots/mapbox/layers.js b/src/plots/mapbox/layers.js index b63b66b3706..aa7a110070e 100644 --- a/src/plots/mapbox/layers.js +++ b/src/plots/mapbox/layers.js @@ -45,9 +45,15 @@ proto.update = function update(opts) { }; proto.needsNewSource = function(opts) { + + // for some reason changing layer to 'fill' w/o changing the source + // throws an exception in mapbox-gl 0.18 + var changesToFill = (this.layerType !== 'fill' && opts.type === 'fill'); + return ( this.sourceType !== opts.sourcetype || - this.source !== opts.source + this.source !== opts.source || + changesToFill ); }; From 053058ad93ae526f0de93b6a8181ba2f9cca5bd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 22 Jun 2016 16:04:43 -0400 Subject: [PATCH 05/16] lint long line --- src/plots/mapbox/mapbox.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plots/mapbox/mapbox.js b/src/plots/mapbox/mapbox.js index 7bd0150cc32..49c728c75db 100644 --- a/src/plots/mapbox/mapbox.js +++ b/src/plots/mapbox/mapbox.js @@ -96,7 +96,8 @@ proto.createMap = function(calcData, fullLayout, resolve, reject) { }); // clear navigation container - var controlContainer = this.div.getElementsByClassName(constants.controlContainerClassName)[0]; + var className = constants.controlContainerClassName, + controlContainer = this.div.getElementsByClassName(className)[0]; this.div.removeChild(controlContainer); self.rejectOnError(reject); From c1cd50cdb382fe5da15ae61c8a15552bca6010aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 22 Jun 2016 16:05:09 -0400 Subject: [PATCH 06/16] make sure the bearing and pitch are updated on map move --- src/plots/mapbox/mapbox.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/plots/mapbox/mapbox.js b/src/plots/mapbox/mapbox.js index 49c728c75db..f1a71b60374 100644 --- a/src/plots/mapbox/mapbox.js +++ b/src/plots/mapbox/mapbox.js @@ -114,6 +114,8 @@ proto.createMap = function(calcData, fullLayout, resolve, reject) { var center = map.getCenter(); opts._input.center = opts.center = { lon: center.lng, lat: center.lat }; opts._input.zoom = opts.zoom = map.getZoom(); + opts._input.bearing = opts.bearing = map.getBearing(); + opts._input.pitch = opts.pitch = map.getPitch(); }); map.on('mousemove', function(evt) { From a8992834a040fca7723d2bcb63e2ff64eba3de2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 22 Jun 2016 16:05:33 -0400 Subject: [PATCH 07/16] fix path the image diff (fixes npm run start-image_viewer) --- test/image/assets/get_image_paths.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/image/assets/get_image_paths.js b/test/image/assets/get_image_paths.js index 6883d261c83..915bce2c2c0 100644 --- a/test/image/assets/get_image_paths.js +++ b/test/image/assets/get_image_paths.js @@ -20,10 +20,10 @@ module.exports = function getImagePaths(mockName, format) { return { baseline: join(constants.pathToTestImageBaselines, mockName, format), test: join(constants.pathToTestImages, mockName, format), - diff: join(constants.pathToTestImagesDiff, mockName, format) + diff: join(constants.pathToTestImagesDiff, 'diff-' + mockName, format) }; }; -function join(basePath, mockName, format) { - return path.join(basePath, mockName) + '.' + format; +function join(basePath, fileName, format) { + return path.join(basePath, fileName) + '.' + format; } From 34d527568a0f999d685f5706164e8912505fa471 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 22 Jun 2016 19:02:40 -0400 Subject: [PATCH 08/16] rm textfont.family settting (for now), - it can lead to error in the case where mapbox doesn't have to requested font. --- src/traces/scattermapbox/convert.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/traces/scattermapbox/convert.js b/src/traces/scattermapbox/convert.js index b469acc61d9..6ed8c658a97 100644 --- a/src/traces/scattermapbox/convert.js +++ b/src/traces/scattermapbox/convert.js @@ -111,10 +111,12 @@ module.exports = function convert(calcTrace) { var textOpts = calcTextOpts(trace); Lib.extendFlat(symbol.layout, { - 'text-font': trace.textfont.textfont, 'text-size': trace.textfont.size, 'text-anchor': textOpts.anchor, 'text-offset': textOpts.offset + + // TODO font family + //'text-font': symbol.textfont.family.split(', '), }); Lib.extendFlat(symbol.paint, { From cf27e55c643a3cf5db2f14dd541b8b61936dc5bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 22 Jun 2016 19:03:06 -0400 Subject: [PATCH 09/16] factor out convert text opts logic into file of its own --- src/plots/mapbox/convert_text_opts.js | 72 +++++++++++++++++++++++++++ src/traces/scattermapbox/convert.js | 54 ++------------------ 2 files changed, 75 insertions(+), 51 deletions(-) create mode 100644 src/plots/mapbox/convert_text_opts.js diff --git a/src/plots/mapbox/convert_text_opts.js b/src/plots/mapbox/convert_text_opts.js new file mode 100644 index 00000000000..6ea827651ba --- /dev/null +++ b/src/plots/mapbox/convert_text_opts.js @@ -0,0 +1,72 @@ +/** +* Copyright 2012-2016, 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 Lib = require('../../lib'); + + +/** + * Convert plotly.js 'textposition' to mapbox-gl 'anchor' and 'offset' + * (with the help of the icon size). + * + * @param {string} textpostion : plotly.js textposition value + * @param {number} iconSize : plotly.js icon size (e.g. marker.size for traces) + * + * @return {object} + * - anchor + * - offset + */ +module.exports = function convertTextOpts(textposition, iconSize) { + var parts = textposition.split(' '), + vPos = parts[0], + hPos = parts[1]; + + // ballpack values + var factor = Array.isArray(iconSize) ? Lib.mean(iconSize) : iconSize, + xInc = 0.5 + (factor / 100), + yInc = 1.5 + (factor / 100); + + var anchorVals = ['', ''], + offset = [0, 0]; + + switch(vPos) { + case 'top': + anchorVals[0] = 'top'; + offset[1] = -yInc; + break; + case 'bottom': + anchorVals[0] = 'bottom'; + offset[1] = yInc; + break; + } + + switch(hPos) { + case 'left': + anchorVals[1] = 'right'; + offset[0] = -xInc; + break; + case 'right': + anchorVals[1] = 'left'; + offset[0] = xInc; + break; + } + + // Mapbox text-anchor must be one of: + // center, left, right, top, bottom, + // top-left, top-right, bottom-left, bottom-right + + var anchor; + if(anchorVals[0] && anchorVals[1]) anchor = anchorVals.join('-'); + else if(anchorVals[0]) anchor = anchorVals[0]; + else if(anchorVals[1]) anchor = anchorVals[1]; + else anchor = 'center'; + + return { anchor: anchor, offset: offset }; +}; diff --git a/src/traces/scattermapbox/convert.js b/src/traces/scattermapbox/convert.js index 6ed8c658a97..f8d6d96c9e4 100644 --- a/src/traces/scattermapbox/convert.js +++ b/src/traces/scattermapbox/convert.js @@ -11,6 +11,7 @@ var Lib = require('../../lib'); var subTypes = require('../scatter/subtypes'); +var convertTextOpts = require('../../plots/mapbox/convert_text_opts'); var COLOR_PROP = 'circle-color'; var SIZE_PROP = 'circle-radius'; @@ -108,7 +109,8 @@ module.exports = function convert(calcTrace) { } if(hasText) { - var textOpts = calcTextOpts(trace); + var iconSize = (trace.marker || {}).size, + textOpts = convertTextOpts(trace.textposition, iconSize); Lib.extendFlat(symbol.layout, { 'text-size': trace.textfont.size, @@ -309,56 +311,6 @@ function calcCircleRadius(trace, hash) { return out; } -function calcTextOpts(trace) { - var textposition = trace.textposition, - parts = textposition.split(' '), - vPos = parts[0], - hPos = parts[1]; - - // ballpack values - var ms = (trace.marker || {}).size, - factor = Array.isArray(ms) ? Lib.mean(ms) : ms, - xInc = 0.5 + (factor / 100), - yInc = 1.5 + (factor / 100); - - var anchorVals = ['', ''], - offset = [0, 0]; - - switch(vPos) { - case 'top': - anchorVals[0] = 'top'; - offset[1] = -yInc; - break; - case 'bottom': - anchorVals[0] = 'bottom'; - offset[1] = yInc; - break; - } - - switch(hPos) { - case 'left': - anchorVals[1] = 'right'; - offset[0] = -xInc; - break; - case 'right': - anchorVals[1] = 'left'; - offset[0] = xInc; - break; - } - - // Mapbox text-anchor must be one of: - // center, left, right, top, bottom, - // top-left, top-right, bottom-left, bottom-right - - var anchor; - if(anchorVals[0] && anchorVals[1]) anchor = anchorVals.join('-'); - else if(anchorVals[0]) anchor = anchorVals[0]; - else if(anchorVals[1]) anchor = anchorVals[1]; - else anchor = 'center'; - - return { anchor: anchor, offset: offset }; -} - function getCoords(calcTrace) { var trace = calcTrace[0].trace, connectgaps = trace.connectgaps; From 1620e617b1c0e7fd7eb5cc7589bc2ac39a9d1277 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 22 Jun 2016 19:04:52 -0400 Subject: [PATCH 10/16] make 'color' a shared attribute among layers at all types --- src/plots/mapbox/layout_attributes.js | 56 +++++++++++---------------- src/plots/mapbox/layout_defaults.js | 19 +++------ test/image/mocks/mapbox_layers.json | 4 +- test/jasmine/tests/mapbox_test.js | 53 ++++++++++++++----------- 4 files changed, 58 insertions(+), 74 deletions(-) diff --git a/src/plots/mapbox/layout_attributes.js b/src/plots/mapbox/layout_attributes.js index 2f9dd0ac916..4d436683f82 100644 --- a/src/plots/mapbox/layout_attributes.js +++ b/src/plots/mapbox/layout_attributes.js @@ -136,6 +136,7 @@ module.exports = { ].join(' ') }, + // attributes shared between all types below: { valType: 'string', dflt: '', @@ -147,7 +148,28 @@ module.exports = { 'the layer will be inserted above every existing layer.' ].join(' ') }, + color: { + valType: 'color', + dflt: defaultLine, + role: 'style', + description: [ + 'Sets the primary layer color.', + 'If `type` is *circle*, color corresponds to the circle color', + 'If `type` is *line*, color corresponds to the line color', + 'If `type` is *fill*, color corresponds to the fill color', + 'If `type` is *symbol*, color corresponds to the icon color' + ].join(' ') + }, + opacity: { + valType: 'number', + min: 0, + max: 1, + dflt: 1, + role: 'info', + description: 'Sets the opacity of the layer.' + }, + // type-specific style attributes circle: { radius: { valType: 'number', @@ -157,15 +179,6 @@ module.exports = { 'Sets the circle radius.', 'Has an effect only when `type` is set to *circle*.' ].join(' ') - }, - color: { - valType: 'color', - dflt: defaultLine, - role: 'style', - description: [ - 'Sets the circle color.', - 'Has an effect only when `type` is set to *circle*.' - ].join(' ') } }, @@ -178,28 +191,10 @@ module.exports = { 'Sets the line radius.', 'Has an effect only when `type` is set to *line*.' ].join(' ') - }, - color: { - valType: 'color', - dflt: defaultLine, - role: 'style', - description: [ - 'Sets the line color.', - 'Has an effect only when `type` is set to *line*.' - ].join(' ') } }, fill: { - color: { - valType: 'color', - dflt: defaultLine, - role: 'style', - description: [ - 'Sets the fill color.', - 'Has an effect only when `type` is set to *fill*.' - ].join(' ') - }, outlinecolor: { valType: 'color', dflt: defaultLine, @@ -211,13 +206,6 @@ module.exports = { } }, - opacity: { - valType: 'number', - min: 0, - max: 1, - dflt: 1, - role: 'info', - description: 'Sets the opacity of the layer.' } } diff --git a/src/plots/mapbox/layout_defaults.js b/src/plots/mapbox/layout_defaults.js index cbaaeae2517..898fe334ee0 100644 --- a/src/plots/mapbox/layout_defaults.js +++ b/src/plots/mapbox/layout_defaults.js @@ -57,34 +57,25 @@ function handleLayerDefaults(containerIn, containerOut) { if(sourceType === 'vector') coerce('sourcelayer'); - // maybe add smart default based off GeoJSON geometry + // maybe add smart default based off GeoJSON geometry? var type = coerce('type'); - var dfltColor; + coerce('below'); + coerce('color'); + coerce('opacity'); if(type === 'circle') { - dfltColor = (layerIn.line || {}).color || (layerIn.fill || {}).color; - - coerce('circle.color', dfltColor); coerce('circle.radius'); } if(type === 'line') { - dfltColor = (layerIn.circle || {}).color || (layerIn.fill || {}).color; - - coerce('line.color', dfltColor); coerce('line.width'); } if(type === 'fill') { - dfltColor = (layerIn.circle || {}).color || (layerIn.line || {}).color; - - coerce('fill.color', dfltColor); - coerce('fill.outlinecolor', dfltColor); + coerce('fill.outlinecolor'); } - coerce('below'); - coerce('opacity'); layersOut.push(layerOut); } diff --git a/test/image/mocks/mapbox_layers.json b/test/image/mocks/mapbox_layers.json index 89861df8aae..98815c6e232 100644 --- a/test/image/mocks/mapbox_layers.json +++ b/test/image/mocks/mapbox_layers.json @@ -532,9 +532,7 @@ }, "type": "fill", "below": "water", - "fill": { - "color": "#ece2f0" - }, + "color": "#ece2f0", "opacity": 0.8 }, { diff --git a/test/jasmine/tests/mapbox_test.js b/test/jasmine/tests/mapbox_test.js index 8ceb80bbbed..c8901a64d1f 100644 --- a/test/jasmine/tests/mapbox_test.js +++ b/test/jasmine/tests/mapbox_test.js @@ -85,40 +85,47 @@ describe('mapbox defaults', function() { }); it('should only coerce relevant layer style attributes', function() { + var base = { + line: { width: 3 }, + fill: { outlinecolor: '#d3d3d3' }, + circle: { radius: 20 }, + symbol: { icon: 'monument' } + }; + layoutIn = { mapbox: { - layers: [{ - sourcetype: 'vector', - type: 'line', - line: { - color: 'red', - width: 3 - }, - fillcolor: 'blue' - }, { - sourcetype: 'geojson', - type: 'fill', - line: { - color: 'red', - width: 3 - }, - fill: { - color: 'blue', - outlinecolor: 'green' - } - }] + layers: [ + Lib.extendFlat({}, base, { + type: 'line', + color: 'red' + }), + Lib.extendFlat({}, base, { + type: 'fill', + color: 'blue' + }), + Lib.extendFlat({}, base, { + type: 'circle', + color: 'green' + }), } }; supplyLayoutDefaults(layoutIn, layoutOut, fullData); - expect(layoutOut.mapbox.layers[0].line.color).toEqual('red'); + expect(layoutOut.mapbox.layers[0].color).toEqual('red'); expect(layoutOut.mapbox.layers[0].line.width).toEqual(3); expect(layoutOut.mapbox.layers[0].fill).toBeUndefined(); + expect(layoutOut.mapbox.layers[0].circle).toBeUndefined(); + expect(layoutOut.mapbox.layers[1].color).toEqual('blue'); + expect(layoutOut.mapbox.layers[1].fill.outlinecolor).toEqual('#d3d3d3'); expect(layoutOut.mapbox.layers[1].line).toBeUndefined(); - expect(layoutOut.mapbox.layers[1].fill.color).toEqual('blue'); - expect(layoutOut.mapbox.layers[1].fill.outlinecolor).toEqual('green'); + expect(layoutOut.mapbox.layers[1].circle).toBeUndefined(); + + expect(layoutOut.mapbox.layers[2].color).toEqual('green'); + expect(layoutOut.mapbox.layers[2].circle.radius).toEqual(20); + expect(layoutOut.mapbox.layers[2].line).toBeUndefined(); + expect(layoutOut.mapbox.layers[2].fill).toBeUndefined(); }); }); From 51fa1eaff26670489247a7ac1a5c5241a1b45723 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 22 Jun 2016 19:09:20 -0400 Subject: [PATCH 11/16] add 'symbol' layer type --- src/plots/mapbox/layers.js | 68 ++++++++++++++++++-------- src/plots/mapbox/layout_attributes.js | 42 +++++++++++++++- src/plots/mapbox/layout_defaults.js | 8 +++ src/traces/scattermapbox/attributes.js | 5 +- test/jasmine/tests/mapbox_test.js | 18 ++++++- 5 files changed, 115 insertions(+), 26 deletions(-) diff --git a/src/plots/mapbox/layers.js b/src/plots/mapbox/layers.js index aa7a110070e..619e26e499d 100644 --- a/src/plots/mapbox/layers.js +++ b/src/plots/mapbox/layers.js @@ -10,6 +10,7 @@ 'use strict'; var Lib = require('../../lib'); +var convertTextOpts = require('./convert_text_opts'); function MapboxLayer(mapbox, index) { @@ -46,14 +47,17 @@ proto.update = function update(opts) { proto.needsNewSource = function(opts) { - // for some reason changing layer to 'fill' w/o changing the source - // throws an exception in mapbox-gl 0.18 - var changesToFill = (this.layerType !== 'fill' && opts.type === 'fill'); + // for some reason changing layer to 'fill' or 'symbol' + // w/o changing the source throws an exception in mapbox-gl 0.18 + + var changesToFill = (this.layerType !== 'fill' && opts.type === 'fill'), + changesToSymbol = (this.layerType !== 'symbol' && opts.type === 'symbol'); return ( this.sourceType !== opts.sourcetype || this.source !== opts.source || - changesToFill + changesToFill || + changesToSymbol ); }; @@ -101,10 +105,11 @@ proto.updateLayer = function(opts) { }; proto.updateStyle = function(opts) { - var paintOpts = convertPaintOpts(opts); + var convertedOpts = convertOpts(opts); if(isVisible(opts)) { - this.mapbox.setOptions(this.idLayer, 'setPaintProperty', paintOpts); + this.mapbox.setOptions(this.idLayer, 'setLayoutProperty', convertedOpts.layout); + this.mapbox.setOptions(this.idLayer, 'setPaintProperty', convertedOpts.paint); } }; @@ -127,42 +132,63 @@ function isVisible(opts) { ); } -function convertPaintOpts(opts) { - var paintOpts = {}; +function convertOpts(opts) { + var layout = {}, + paint = {}; switch(opts.type) { case 'circle': - var circle = opts.circle; - Lib.extendFlat(paintOpts, { - 'circle-radius': circle.radius, - 'circle-color': circle.color, + Lib.extendFlat(paint, { + 'circle-radius': opts.circle.radius, + 'circle-color': opts.color, 'circle-opacity': opts.opacity }); break; case 'line': - var line = opts.line; - Lib.extendFlat(paintOpts, { - 'line-width': line.width, - 'line-color': line.color, + Lib.extendFlat(paint, { + 'line-width': opts.line.width, + 'line-color': opts.color, 'line-opacity': opts.opacity }); break; case 'fill': - var fill = opts.fill; - Lib.extendFlat(paintOpts, { - 'fill-color': fill.color, - 'fill-outline-color': fill.outlinecolor, + Lib.extendFlat(paint, { + 'fill-color': opts.color, + 'fill-outline-color': opts.fill.outlinecolor, 'fill-opacity': opts.opacity // no way to pass specify outline width at the moment }); break; + + case 'symbol': + var symbol = opts.symbol, + textOpts = convertTextOpts(symbol.textposition, symbol.iconsize); + + Lib.extendFlat(layout, { + 'icon-image': symbol.icon + '-15', + 'icon-size': symbol.iconsize / 10, + + 'text-field': symbol.text, + 'text-size': symbol.textfont.size, + 'text-anchor': textOpts.anchor, + 'text-offset': textOpts.offset + + // TODO font family + //'text-font': symbol.textfont.family.split(', '), + }); + + Lib.extendFlat(paint, { + 'text-color': symbol.textfont.color, + 'text-opacity': opts.opacity + }); + break; } - return paintOpts; + return { layout: layout, paint: paint }; } function convertSourceOpts(opts) { diff --git a/src/plots/mapbox/layout_attributes.js b/src/plots/mapbox/layout_attributes.js index 4d436683f82..0a44b831a65 100644 --- a/src/plots/mapbox/layout_attributes.js +++ b/src/plots/mapbox/layout_attributes.js @@ -9,7 +9,10 @@ 'use strict'; +var Lib = require('../../lib'); var defaultLine = require('../../components/color').defaultLine; +var fontAttrs = require('../font_attributes'); +var textposition = require('../../traces/scatter/attributes').textposition; module.exports = { @@ -125,7 +128,7 @@ module.exports = { type: { valType: 'enumerated', - values: ['circle', 'line', 'fill'], + values: ['circle', 'line', 'fill', 'symbol'], dflt: 'line', role: 'info', description: [ @@ -206,6 +209,43 @@ module.exports = { } }, + symbol: { + icon: { + valType: 'string', + dflt: 'circle', + role: 'style', + description: [ + 'Sets the symbol icon image.', + 'Full list: https://www.mapbox.com/maki-icons/' + ].join(' ') + }, + iconsize: { + valType: 'number', + dflt: 10, + role: 'style', + description: [ + 'Sets the icon size.', + 'Has an effect only when `type` is set to *symbol*.' + ].join(' ') + }, + text: { + valType: 'string', + dflt: '', + role: 'info', + description: [ + 'Sets the symbol text.' + ].join(' ') + }, + textfont: Lib.extendDeep({}, fontAttrs, { + description: [ + 'Sets the icon text font.', + 'Has an effect only when `type` is set to *symbol*.' + ].join(' '), + family: { + dflt: 'Open Sans Regular, Arial Unicode MS Regular' + } + }), + textposition: Lib.extendFlat({}, textposition, { arrayOk: false }) } } diff --git a/src/plots/mapbox/layout_defaults.js b/src/plots/mapbox/layout_defaults.js index 898fe334ee0..92eac67f355 100644 --- a/src/plots/mapbox/layout_defaults.js +++ b/src/plots/mapbox/layout_defaults.js @@ -76,6 +76,14 @@ function handleLayerDefaults(containerIn, containerOut) { coerce('fill.outlinecolor'); } + if(type === 'symbol') { + coerce('symbol.icon'); + coerce('symbol.iconsize'); + + coerce('symbol.text'); + Lib.coerceFont(coerce, 'symbol.textfont'); + coerce('symbol.textposition'); + } layersOut.push(layerOut); } diff --git a/src/traces/scattermapbox/attributes.js b/src/traces/scattermapbox/attributes.js index 38bb7d909e6..c66c259e809 100644 --- a/src/traces/scattermapbox/attributes.js +++ b/src/traces/scattermapbox/attributes.js @@ -10,6 +10,7 @@ var scatterGeoAttrs = require('../scattergeo/attributes'); var scatterAttrs = require('../scatter/attributes'); +var mapboxAttrs = require('../../plots/mapbox/layout_attributes'); var plotAttrs = require('../../plots/attributes'); var extendFlat = require('../../lib/extend').extendFlat; @@ -104,8 +105,8 @@ module.exports = { }, fillcolor: scatterAttrs.fillcolor, - textfont: extendFlat({}, scatterAttrs.textfont, { arrayOk: false }), - textposition: extendFlat({}, scatterAttrs.textposition, { arrayOk: false }), + textfont: mapboxAttrs.layers.symbol.textfont, + textposition: mapboxAttrs.layers.symbol.textposition, hoverinfo: extendFlat({}, plotAttrs.hoverinfo, { flags: ['lon', 'lat', 'text', 'name'] diff --git a/test/jasmine/tests/mapbox_test.js b/test/jasmine/tests/mapbox_test.js index c8901a64d1f..b6184d991cb 100644 --- a/test/jasmine/tests/mapbox_test.js +++ b/test/jasmine/tests/mapbox_test.js @@ -107,6 +107,11 @@ describe('mapbox defaults', function() { type: 'circle', color: 'green' }), + Lib.extendFlat({}, base, { + type: 'symbol', + color: 'yellow' + }) + ] } }; @@ -116,16 +121,25 @@ describe('mapbox defaults', function() { expect(layoutOut.mapbox.layers[0].line.width).toEqual(3); expect(layoutOut.mapbox.layers[0].fill).toBeUndefined(); expect(layoutOut.mapbox.layers[0].circle).toBeUndefined(); + expect(layoutOut.mapbox.layers[0].symbol).toBeUndefined(); expect(layoutOut.mapbox.layers[1].color).toEqual('blue'); expect(layoutOut.mapbox.layers[1].fill.outlinecolor).toEqual('#d3d3d3'); expect(layoutOut.mapbox.layers[1].line).toBeUndefined(); expect(layoutOut.mapbox.layers[1].circle).toBeUndefined(); + expect(layoutOut.mapbox.layers[1].symbol).toBeUndefined(); expect(layoutOut.mapbox.layers[2].color).toEqual('green'); expect(layoutOut.mapbox.layers[2].circle.radius).toEqual(20); expect(layoutOut.mapbox.layers[2].line).toBeUndefined(); expect(layoutOut.mapbox.layers[2].fill).toBeUndefined(); + expect(layoutOut.mapbox.layers[2].symbol).toBeUndefined(); + + expect(layoutOut.mapbox.layers[3].color).toEqual('yellow'); + expect(layoutOut.mapbox.layers[3].symbol.icon).toEqual('monument'); + expect(layoutOut.mapbox.layers[3].line).toBeUndefined(); + expect(layoutOut.mapbox.layers[3].fill).toBeUndefined(); + expect(layoutOut.mapbox.layers[3].circle).toBeUndefined(); }); }); @@ -363,14 +377,14 @@ describe('mapbox plots', function() { }; var styleUpdate0 = { - 'mapbox.layers[0].fill.color': 'red', + 'mapbox.layers[0].color': 'red', 'mapbox.layers[0].fill.outlinecolor': 'blue', 'mapbox.layers[0].opacity': 0.3 }; var styleUpdate1 = { + 'mapbox.layers[1].color': 'blue', 'mapbox.layers[1].line.width': 3, - 'mapbox.layers[1].line.color': 'blue', 'mapbox.layers[1].opacity': 0.6 }; From 60e428556b14d492f9dd76e329b26676a4152dac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 22 Jun 2016 19:22:23 -0400 Subject: [PATCH 12/16] pass layer 'color' to mapbox layer paint --- src/plots/mapbox/layers.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/plots/mapbox/layers.js b/src/plots/mapbox/layers.js index 619e26e499d..c4cba3eea4b 100644 --- a/src/plots/mapbox/layers.js +++ b/src/plots/mapbox/layers.js @@ -182,6 +182,7 @@ function convertOpts(opts) { }); Lib.extendFlat(paint, { + 'icon-color': opts.color, 'text-color': symbol.textfont.color, 'text-opacity': opts.opacity }); From 0d082e4334c2071a76f04b4678bf610dd8c0b1f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 22 Jun 2016 19:22:57 -0400 Subject: [PATCH 13/16] make 'circle' the default layer type --- src/plots/mapbox/layout_attributes.js | 4 ++-- test/image/mocks/mapbox_layers.json | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/plots/mapbox/layout_attributes.js b/src/plots/mapbox/layout_attributes.js index 0a44b831a65..3af998c10ac 100644 --- a/src/plots/mapbox/layout_attributes.js +++ b/src/plots/mapbox/layout_attributes.js @@ -129,13 +129,13 @@ module.exports = { type: { valType: 'enumerated', values: ['circle', 'line', 'fill', 'symbol'], - dflt: 'line', + dflt: 'circle', role: 'info', description: [ 'Sets the layer type.', 'Support for *raster*, *background* types is coming soon.', 'Note that *line* and *fill* are not compatible with Point', - 'GeoJSON geometry.' + 'GeoJSON geometries.' ].join(' ') }, diff --git a/test/image/mocks/mapbox_layers.json b/test/image/mocks/mapbox_layers.json index 98815c6e232..9edb823de66 100644 --- a/test/image/mocks/mapbox_layers.json +++ b/test/image/mocks/mapbox_layers.json @@ -539,9 +539,10 @@ "sourcetype": "vector", "source": "mapbox://mapbox.mapbox-terrain-v2", "sourcelayer": "contour", + "type": "line", + "color": "red", "line": { - "width": 2, - "color": "red" + "width": 2 } } ] From 0fdf70ce6d3f324541414573fe59ba897834a091 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 22 Jun 2016 19:23:14 -0400 Subject: [PATCH 14/16] make 'marker' the default layer symbol icon --- src/plots/mapbox/layout_attributes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plots/mapbox/layout_attributes.js b/src/plots/mapbox/layout_attributes.js index 3af998c10ac..28831be505b 100644 --- a/src/plots/mapbox/layout_attributes.js +++ b/src/plots/mapbox/layout_attributes.js @@ -212,7 +212,7 @@ module.exports = { symbol: { icon: { valType: 'string', - dflt: 'circle', + dflt: 'marker', role: 'style', description: [ 'Sets the symbol icon image.', From cb854fef62d679e60e0fad7c66065c127cd53741 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 22 Jun 2016 22:18:51 -0400 Subject: [PATCH 15/16] fixup symbo.iconsize description --- src/plots/mapbox/layout_attributes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plots/mapbox/layout_attributes.js b/src/plots/mapbox/layout_attributes.js index 28831be505b..f70fbfe027b 100644 --- a/src/plots/mapbox/layout_attributes.js +++ b/src/plots/mapbox/layout_attributes.js @@ -224,7 +224,7 @@ module.exports = { dflt: 10, role: 'style', description: [ - 'Sets the icon size.', + 'Sets the symbol icon size.', 'Has an effect only when `type` is set to *symbol*.' ].join(' ') }, From 752c419b25822e4e7d41e6b19450c072bce3bd8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 22 Jun 2016 22:19:57 -0400 Subject: [PATCH 16/16] make layer type change go through a source refresh, - staying safe as some type change lead to exceptions --- src/plots/mapbox/layers.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/plots/mapbox/layers.js b/src/plots/mapbox/layers.js index c4cba3eea4b..cc42528951a 100644 --- a/src/plots/mapbox/layers.js +++ b/src/plots/mapbox/layers.js @@ -48,16 +48,13 @@ proto.update = function update(opts) { proto.needsNewSource = function(opts) { // for some reason changing layer to 'fill' or 'symbol' - // w/o changing the source throws an exception in mapbox-gl 0.18 - - var changesToFill = (this.layerType !== 'fill' && opts.type === 'fill'), - changesToSymbol = (this.layerType !== 'symbol' && opts.type === 'symbol'); + // w/o changing the source throws an exception in mapbox-gl 0.18 ; + // stay safe and make new source on type changes return ( this.sourceType !== opts.sourcetype || this.source !== opts.source || - changesToFill || - changesToSymbol + this.layerType !== opts.type ); };