From 134b54e11bdbe8ff2f9852b35d5e7c4b1e4f2e59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 30 May 2017 17:30:24 -0400 Subject: [PATCH 1/8] add test case for annotation/shape/image zoom/pan on category axes --- test/jasmine/tests/plot_api_test.js | 78 +++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/test/jasmine/tests/plot_api_test.js b/test/jasmine/tests/plot_api_test.js index 24337538273..dee0dc7a315 100644 --- a/test/jasmine/tests/plot_api_test.js +++ b/test/jasmine/tests/plot_api_test.js @@ -10,6 +10,7 @@ var subroutines = require('@src/plot_api/subroutines'); var helpers = require('@src/plot_api/helpers'); var d3 = require('d3'); +var customMatchers = require('../assets/custom_matchers'); var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); var fail = require('../assets/fail_test'); @@ -105,6 +106,10 @@ describe('Test plot api', function() { describe('Plotly.relayout', function() { var gd; + beforeAll(function() { + jasmine.addMatchers(customMatchers); + }); + beforeEach(function() { gd = createGraphDiv(); }); @@ -252,6 +257,79 @@ describe('Test plot api', function() { .catch(fail) .then(done); }); + + it('annotations, shapes and images linked to category axes should update properly on zoom/pan', function(done) { + var jsLogo = 'https://images.plot.ly/language-icons/api-home/js-logo.png'; + + function getPos(sel) { + var rect = sel.node().getBoundingClientRect(); + return [rect.left, rect.bottom]; + } + + function getAnnotationPos() { + return getPos(d3.select('.annotation')); + } + + function getShapePos() { + return getPos(d3.select('.layer-above').select('.shapelayer').select('path')); + } + + function getImagePos() { + return getPos(d3.select('.layer-above').select('.imagelayer').select('image')); + } + + Plotly.plot(gd, [{ + x: ['a', 'b', 'c'], + y: [1, 2, 1] + }], { + xaxis: {range: [-1, 5]}, + annotations: [{ + xref: 'x', + yref: 'y', + x: 'b', + y: 2 + }], + shapes: [{ + xref: 'x', + yref: 'y', + type: 'line', + x0: 'c', + x1: 'c', + y0: -1, + y1: 4 + }], + images: [{ + xref: 'x', + yref: 'y', + source: jsLogo, + x: 'a', + y: 1, + sizex: 0.2, + sizey: 0.2 + }] + }) + .then(function() { + expect(getAnnotationPos()).toBeCloseToArray([247.5, 210.1]); + expect(getShapePos()).toBeCloseToArray([350, 369]); + expect(getImagePos()).toBeCloseToArray([170, 272.52]); + + return Plotly.relayout(gd, 'xaxis.range', [0, 2]); + }) + .then(function() { + expect(getAnnotationPos()).toBeCloseToArray([337.5, 210.1]); + expect(getShapePos()).toBeCloseToArray([620, 369]); + expect(getImagePos()).toBeCloseToArray([80, 272.52]); + + return Plotly.relayout(gd, 'xaxis.range', [-1, 5]); + }) + .then(function() { + expect(getAnnotationPos()).toBeCloseToArray([247.5, 210.1]); + expect(getShapePos()).toBeCloseToArray([350, 369]); + expect(getImagePos()).toBeCloseToArray([170, 272.52]); + }) + .catch(fail) + .then(done); + }); }); describe('Plotly.restyle subroutines switchboard', function() { From 48b81313405abd607db46761d34c8802f8e57a9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 30 May 2017 17:33:02 -0400 Subject: [PATCH 2/8] :hocho: annotation/shape/image extra defaults step for category axes ... in Plots.doCalcdata - this approach was flawed: led to position value not updating properly on calc-less update (e.g. zoom/pan) --- src/plots/plots.js | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index f853c2c003e..c4f18cec787 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -2032,7 +2032,7 @@ plots.doCalcdata = function(gd, traces) { } } - var hasCategoryAxis = initCategories(axList); + initCategories(axList); var hasCalcTransform = false; @@ -2101,25 +2101,11 @@ plots.doCalcdata = function(gd, traces) { } Registry.getComponentMethod('fx', 'calc')(gd); - - // To handle the case of components using category names as coordinates, we - // need to re-supply defaults for these objects now, after calc has - // finished populating the category mappings - // Any component that uses `Axes.coercePosition` falls into this category - if(hasCategoryAxis) { - var dataReferencedComponents = ['annotations', 'shapes', 'images']; - for(i = 0; i < dataReferencedComponents.length; i++) { - Registry.getComponentMethod(dataReferencedComponents[i], 'supplyLayoutDefaults')( - gd.layout, fullLayout, fullData); - } - } }; +// initialize the category list, if there is one, so we start over +// to be filled in later by ax.d2c function initCategories(axList) { - var hasCategoryAxis = false; - - // initialize the category list, if there is one, so we start over - // to be filled in later by ax.d2c for(var i = 0; i < axList.length; i++) { axList[i]._categories = axList[i]._initialCategories.slice(); @@ -2128,11 +2114,7 @@ function initCategories(axList) { for(var j = 0; j < axList[i]._categories.length; j++) { axList[i]._categoriesMap[axList[i]._categories[j]] = j; } - - if(axList[i].type === 'category') hasCategoryAxis = true; } - - return hasCategoryAxis; } plots.rehover = function(gd) { From b67657fdf3e6381deecdd5a8e52d880aec86fba9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 30 May 2017 17:44:38 -0400 Subject: [PATCH 3/8] redefine what 'r' means for category axes & add cleanPos axis method - DO NOT convert category position value to indices (the old 'r') in Axes.coercePosition - as this relied on ax._categories which isn't defined during Plots.supplyDefaults. - modify `type: 'category'` conversions functions accordingly (a bit hacky) - update annotation/click.js to reflect change in 'r' definition - use ax.cleanPos to clean position value per axis type in coercePosition --- src/components/annotations/click.js | 19 +++++++++---- src/lib/index.js | 12 +++++++++ src/plots/cartesian/axes.js | 30 ++++++--------------- src/plots/cartesian/set_convert.js | 37 +++++++++++++++----------- src/traces/heatmap/calc.js | 11 +++++--- test/jasmine/tests/annotations_test.js | 23 ---------------- 6 files changed, 63 insertions(+), 69 deletions(-) diff --git a/src/components/annotations/click.js b/src/components/annotations/click.js index eb6c26f12c5..8bdc8688a21 100644 --- a/src/components/annotations/click.js +++ b/src/components/annotations/click.js @@ -83,18 +83,22 @@ function getToggleSets(gd, hoverData) { explicitOffSet = [], hoverLen = (hoverData || []).length; - var i, j, anni, showMode, pointj, toggleType; + var i, j, anni, showMode, pointj, xa, ya, toggleType; for(i = 0; i < annotations.length; i++) { anni = annotations[i]; showMode = anni.clicktoshow; + if(showMode) { for(j = 0; j < hoverLen; j++) { pointj = hoverData[j]; - if(pointj.xaxis._id === anni.xref && - pointj.yaxis._id === anni.yref && - pointj.xaxis.d2r(pointj.x) === anni._xclick && - pointj.yaxis.d2r(pointj.y) === anni._yclick + xa = pointj.xaxis; + ya = pointj.yaxis; + + if(xa._id === anni.xref && + ya._id === anni.yref && + xa.d2r(pointj.x) === clickData2r(anni._xclick, xa) && + ya.d2r(pointj.y) === clickData2r(anni._yclick, ya) ) { // match! toggle this annotation // regardless of its clicktoshow mode @@ -121,3 +125,8 @@ function getToggleSets(gd, hoverData) { return {on: onSet, off: offSet, explicitOff: explicitOffSet}; } + +// to handle log axes until v2 +function clickData2r(d, ax) { + return ax.type === 'log' ? ax.l2r(d) : ax.d2r(d); +} diff --git a/src/lib/index.js b/src/lib/index.js index 10b3226ac77..961fbd18d31 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -10,6 +10,11 @@ 'use strict'; var d3 = require('d3'); +var isNumeric = require('fast-isnumeric'); + +var numConstants = require('../constants/numerical'); +var FP_SAFE = numConstants.FP_SAFE; +var BADNUM = numConstants.BADNUM; var lib = module.exports = {}; @@ -87,6 +92,13 @@ lib.pushUnique = require('./push_unique'); lib.cleanNumber = require('./clean_number'); +lib.num = function num(v) { + if(!isNumeric(v)) return BADNUM; + v = Number(v); + if(v < -FP_SAFE || v > FP_SAFE) return BADNUM; + return isNumeric(v) ? Number(v) : BADNUM; +}; + lib.noop = require('./noop'); lib.identity = require('./identity'); diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 2317a4f9ebf..577e558f388 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -27,7 +27,6 @@ var ONEDAY = constants.ONEDAY; var ONEHOUR = constants.ONEHOUR; var ONEMIN = constants.ONEMIN; var ONESEC = constants.ONESEC; -var BADNUM = constants.BADNUM; var axes = module.exports = {}; @@ -100,33 +99,20 @@ axes.coerceRef = function(containerIn, containerOut, gd, attr, dflt, extraOption * - for other types: coerce them to numbers */ axes.coercePosition = function(containerOut, gd, coerce, axRef, attr, dflt) { - var pos, - newPos; + var ax, pos; if(axRef === 'paper' || axRef === 'pixel') { + ax = { cleanPos: Lib.num }; pos = coerce(attr, dflt); - } - else { - var ax = axes.getFromId(gd, axRef); - + } else { + ax = axes.getFromId(gd, axRef); dflt = ax.fraction2r(dflt); pos = coerce(attr, dflt); - - if(ax.type === 'category') { - // if position is given as a category name, convert it to a number - if(typeof pos === 'string' && (ax._categories || []).length) { - newPos = ax._categories.indexOf(pos); - containerOut[attr] = (newPos === -1) ? dflt : newPos; - return; - } - } - else if(ax.type === 'date') { - containerOut[attr] = Lib.cleanDate(pos, BADNUM, ax.calendar); - return; - } } - // finally make sure we have a number (unless date type already returned a string) - containerOut[attr] = isNumeric(pos) ? Number(pos) : dflt; + + containerOut[attr] = ax.cleanPos(pos); +}; + }; axes.getDataToCoordFunc = function(gd, trace, target, targetArray) { diff --git a/src/plots/cartesian/set_convert.js b/src/plots/cartesian/set_convert.js index faeea322e46..b5f76eec5e1 100644 --- a/src/plots/cartesian/set_convert.js +++ b/src/plots/cartesian/set_convert.js @@ -16,6 +16,7 @@ var Lib = require('../../lib'); var cleanNumber = Lib.cleanNumber; var ms2DateTime = Lib.ms2DateTime; var dateTime2ms = Lib.dateTime2ms; +var num = Lib.num; var numConstants = require('../../constants/numerical'); var FP_SAFE = numConstants.FP_SAFE; @@ -28,13 +29,6 @@ function fromLog(v) { return Math.pow(10, v); } -function num(v) { - if(!isNumeric(v)) return BADNUM; - v = Number(v); - if(v < -FP_SAFE || v > FP_SAFE) return BADNUM; - return isNumeric(v) ? Number(v) : BADNUM; -} - /** * Define the conversion functions for an axis data is used in 5 ways: * @@ -152,7 +146,7 @@ module.exports = function setConvert(ax, fullLayout) { if(index !== undefined) return index; } - if(typeof v === 'number') { return v; } + if(isNumeric(v)) return +v; } function l2p(v) { @@ -186,6 +180,8 @@ module.exports = function setConvert(ax, fullLayout) { ax.d2p = ax.r2p = function(v) { return ax.l2p(cleanNumber(v)); }; ax.p2d = ax.p2r = p2l; + + ax.cleanPos = num; } else if(ax.type === 'log') { // d and c are data vals, r and l are logged (but d and r need cleaning) @@ -203,6 +199,8 @@ module.exports = function setConvert(ax, fullLayout) { ax.r2p = function(v) { return ax.l2p(cleanNumber(v)); }; ax.p2r = p2l; + + ax.cleanPos = num; } else if(ax.type === 'date') { // r and d are date strings, l and c are ms @@ -222,24 +220,31 @@ module.exports = function setConvert(ax, fullLayout) { ax.d2p = ax.r2p = function(v, _, calendar) { return ax.l2p(dt2ms(v, 0, calendar)); }; ax.p2d = ax.p2r = function(px, r, calendar) { return ms2dt(p2l(px), r, calendar); }; + + ax.cleanPos = function(v) { return Lib.cleanDate(v, BADNUM, ax.calendar); }; } else if(ax.type === 'category') { - // d is categories; r, c, and l are indices - // TODO: should r accept category names too? - // ie r2c and r2l would be getCategoryIndex (and r2p would change) + // d is categories (string) + // c and l are indices (numbers) + // r is categories or numbers - ax.d2r = ax.d2c = ax.d2l = setCategoryIndex; + ax.d2c = ax.d2l = setCategoryIndex; ax.r2d = ax.c2d = ax.l2d = getCategoryName; - // special d2l variant that won't add categories - ax.d2l_noadd = getCategoryIndex; + ax.d2r = ax.d2l_noadd = getCategoryIndex; - ax.r2l = ax.l2r = ax.r2c = ax.c2r = num; + ax.l2r = ax.r2c = ax.c2r = num; + ax.r2l = getCategoryIndex; ax.d2p = function(v) { return ax.l2p(getCategoryIndex(v)); }; ax.p2d = function(px) { return getCategoryName(p2l(px)); }; - ax.r2p = ax.l2p; + ax.r2p = ax.d2p; ax.p2r = p2l; + + ax.cleanPos = function(v) { + if(typeof v === 'string' && v !== '') return v; + return num(v); + }; } // find the range value at the specified (linear) fraction of the axis diff --git a/src/traces/heatmap/calc.js b/src/traces/heatmap/calc.js index e77ab3530db..cad3f3cf478 100644 --- a/src/traces/heatmap/calc.js +++ b/src/traces/heatmap/calc.js @@ -57,10 +57,15 @@ module.exports = function calc(gd, trace) { z = binned.z; } else { - if(hasColumns(trace)) convertColumnData(trace, xa, ya, 'x', 'y', ['z']); + if(hasColumns(trace)) { + convertColumnData(trace, xa, ya, 'x', 'y', ['z']); + x = trace.x; + y = trace.y; + } else { + x = trace.x ? xa.makeCalcdata(trace, 'x') : []; + y = trace.y ? ya.makeCalcdata(trace, 'y') : []; + } - x = trace.x ? xa.makeCalcdata(trace, 'x') : []; - y = trace.y ? ya.makeCalcdata(trace, 'y') : []; x0 = trace.x0 || 0; dx = trace.dx || 1; y0 = trace.y0 || 0; diff --git a/test/jasmine/tests/annotations_test.js b/test/jasmine/tests/annotations_test.js index 6a9d1c846c2..5429c0ed0a1 100644 --- a/test/jasmine/tests/annotations_test.js +++ b/test/jasmine/tests/annotations_test.js @@ -98,39 +98,16 @@ describe('Test annotations', function() { expect(layoutOut.annotations[0].ax).toEqual('2004-07-01'); }); - it('should convert ax/ay category coordinates to linear coords', function() { var layoutIn = { annotations: [{ - showarrow: true, - axref: 'x', - ayref: 'y', - x: 'c', - ax: 1, - y: 'A', - ay: 3 }] }; var layoutOut = { - xaxis: { - type: 'category', - _categories: ['a', 'b', 'c'], - range: [-0.5, 2.5] }, - yaxis: { - type: 'category', - _categories: ['A', 'B', 'C'], - range: [-0.5, 3] - } }; - Axes.setConvert(layoutOut.xaxis); - Axes.setConvert(layoutOut.yaxis); _supply(layoutIn, layoutOut); - expect(layoutOut.annotations[0].x).toEqual(2); - expect(layoutOut.annotations[0].ax).toEqual(1); - expect(layoutOut.annotations[0].y).toEqual(0); - expect(layoutOut.annotations[0].ay).toEqual(3); }); }); }); From d8462ebc6b45e0b8a73cf28ef9fd51ebecb1f9f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 30 May 2017 17:50:59 -0400 Subject: [PATCH 4/8] move 3d annotation coercePosition step from convert to defaults - as now Axes.coercePosition does not rely on ax._categories for `type: 'category' axes. --- src/components/annotations3d/convert.js | 22 ---------------------- src/components/annotations3d/defaults.js | 20 +++++++++++++++----- 2 files changed, 15 insertions(+), 27 deletions(-) diff --git a/src/components/annotations3d/convert.js b/src/components/annotations3d/convert.js index a2cf39345ed..6e8b85ffd1e 100644 --- a/src/components/annotations3d/convert.js +++ b/src/components/annotations3d/convert.js @@ -10,7 +10,6 @@ var Lib = require('../../lib'); var Axes = require('../../plots/cartesian/axes'); -var attributes = require('./attributes'); module.exports = function convert(scene) { var fullSceneLayout = scene.fullSceneLayout; @@ -61,25 +60,4 @@ function mockAnnAxes(ann, scene) { ann._ya.l2p = function() { return 0.5 * (1 - ann.pdata[1] / ann.pdata[3]) * size.h * (domain.y[1] - domain.y[0]); }; - - // or do something more similar to 2d - // where Annotations.supplyLayoutDefaults is called after in Plots.doCalcdata - // if category axes are found. - function coerce(attr, dflt) { - return Lib.coerce(ann, ann, attributes, attr, dflt); - } - - function coercePosition(axLetter) { - var axName = axLetter + 'axis'; - - // mock in such way that getFromId grabs correct 3D axis - var gdMock = { _fullLayout: {} }; - gdMock._fullLayout[axName] = fullSceneLayout[axName]; - - return Axes.coercePosition(ann, gdMock, coerce, axLetter, axLetter, 0.5); - } - - coercePosition('x'); - coercePosition('y'); - coercePosition('z'); } diff --git a/src/components/annotations3d/defaults.js b/src/components/annotations3d/defaults.js index a0353d24dc6..603c0d7cf7b 100644 --- a/src/components/annotations3d/defaults.js +++ b/src/components/annotations3d/defaults.js @@ -9,6 +9,7 @@ 'use strict'; var Lib = require('../../lib'); +var Axes = require('../../plots/cartesian/axes'); var handleArrayContainerDefaults = require('../../plots/array_container_defaults'); var handleAnnotationCommonDefaults = require('../annotations/common_defaults'); var attributes = require('./attributes'); @@ -26,16 +27,25 @@ function handleAnnotationDefaults(annIn, annOut, sceneLayout, opts, itemOpts) { return Lib.coerce(annIn, annOut, attributes, attr, dflt); } + function coercePosition(axLetter) { + var axName = axLetter + 'axis'; + + // mock in such way that getFromId grabs correct 3D axis + var gdMock = { _fullLayout: {} }; + gdMock._fullLayout[axName] = sceneLayout[axName]; + + return Axes.coercePosition(annOut, gdMock, coerce, axLetter, axLetter, 0.5); + } + + var visible = coerce('visible', !itemOpts.itemIsNotPlainObject); if(!visible) return annOut; handleAnnotationCommonDefaults(annIn, annOut, opts.fullLayout, coerce); - // do not use Axes.coercePosition here - // as ax._categories aren't filled in at this stage - coerce('x'); - coerce('y'); - coerce('z'); + coercePosition('x'); + coercePosition('y'); + coercePosition('z'); // if you have one coordinate you should all three Lib.noneOrAll(annIn, annOut, ['x', 'y', 'z']); From ee3b72ce87e07fde9ac283485a1cea6718209f0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 30 May 2017 17:52:02 -0400 Subject: [PATCH 5/8] add Axes.cleanPosition method - a little wrapper around ax.cleanPos - use it for annotations *xclick* and *yclick* --- .../annotations/annotation_defaults.js | 8 +++-- src/plots/cartesian/axes.js | 6 ++++ test/jasmine/tests/annotations_test.js | 32 +++++++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/src/components/annotations/annotation_defaults.js b/src/components/annotations/annotation_defaults.js index 0b659244ade..68178e6f004 100644 --- a/src/components/annotations/annotation_defaults.js +++ b/src/components/annotations/annotation_defaults.js @@ -83,8 +83,12 @@ module.exports = function handleAnnotationDefaults(annIn, annOut, fullLayout, op // put the actual click data to bind to into private attributes // so we don't have to do this little bit of logic on every hover event - annOut._xclick = (xClick === undefined) ? annOut.x : xClick; - annOut._yclick = (yClick === undefined) ? annOut.y : yClick; + annOut._xclick = (xClick === undefined) ? + annOut.x : + Axes.cleanPosition(xClick, gdMock, annOut.xref); + annOut._yclick = (yClick === undefined) ? + annOut.y : + Axes.cleanPosition(yClick, gdMock, annOut.yref); } return annOut; diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 577e558f388..c6b5a3b1713 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -113,6 +113,12 @@ axes.coercePosition = function(containerOut, gd, coerce, axRef, attr, dflt) { containerOut[attr] = ax.cleanPos(pos); }; +axes.cleanPosition = function(pos, gd, axRef) { + var ax = (axRef === 'paper' || axRef === 'pixel') ? + { cleanPos: Lib.num } : + axes.getFromId(gd, axRef); + + return ax.cleanPos(pos); }; axes.getDataToCoordFunc = function(gd, trace, target, targetArray) { diff --git a/test/jasmine/tests/annotations_test.js b/test/jasmine/tests/annotations_test.js index 5429c0ed0a1..5a0e4fbd2f0 100644 --- a/test/jasmine/tests/annotations_test.js +++ b/test/jasmine/tests/annotations_test.js @@ -98,16 +98,48 @@ describe('Test annotations', function() { expect(layoutOut.annotations[0].ax).toEqual('2004-07-01'); }); + it('should clean *xclick* and *yclick* values', function() { var layoutIn = { annotations: [{ + clicktoshow: 'onoff', + xref: 'paper', + yref: 'paper', + xclick: '10', + yclick: '30' + }, { + clicktoshow: 'onoff', + xref: 'x', + yref: 'y', + xclick: '1', + yclick: '2017-13-50' + }, { + clicktoshow: 'onoff', + xref: 'x2', + yref: 'y2', + xclick: '2', + yclick: 'A' }] }; var layoutOut = { + xaxis: {type: 'linear', range: [0, 1]}, + yaxis: {type: 'date', range: ['2000-01-01', '2018-01-01']}, + xaxis2: {type: 'log', range: [1, 2]}, + yaxis2: {type: 'category', range: [0, 1]} }; + ['xaxis', 'xaxis2', 'yaxis', 'yaxis2'].forEach(function(k) { + Axes.setConvert(layoutOut[k]); + }); + _supply(layoutIn, layoutOut); + expect(layoutOut.annotations[0]._xclick).toBe(10, 'paper x'); + expect(layoutOut.annotations[0]._yclick).toBe(30, 'paper y'); + expect(layoutOut.annotations[1]._xclick).toBe(1, 'linear'); + expect(layoutOut.annotations[1]._yclick).toBe(undefined, 'invalid date'); + expect(layoutOut.annotations[2]._xclick).toBe(2, 'log'); + expect(layoutOut.annotations[2]._yclick).toBe('A', 'category'); }); }); }); From 071249f917ca4bb5995d973bf362fd656a1e663c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 31 May 2017 11:16:36 -0400 Subject: [PATCH 6/8] rename Lib.num -> Lib.ensureNumber --- src/lib/index.js | 2 +- src/plots/cartesian/axes.js | 17 +++++++++-------- src/plots/cartesian/set_convert.js | 18 +++++++++--------- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/lib/index.js b/src/lib/index.js index 961fbd18d31..815b627cc83 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -92,7 +92,7 @@ lib.pushUnique = require('./push_unique'); lib.cleanNumber = require('./clean_number'); -lib.num = function num(v) { +lib.ensureNumber = function num(v) { if(!isNumeric(v)) return BADNUM; v = Number(v); if(v < -FP_SAFE || v > FP_SAFE) return BADNUM; diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index c6b5a3b1713..f2560c08f12 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -99,26 +99,27 @@ axes.coerceRef = function(containerIn, containerOut, gd, attr, dflt, extraOption * - for other types: coerce them to numbers */ axes.coercePosition = function(containerOut, gd, coerce, axRef, attr, dflt) { - var ax, pos; + var cleanPos, pos; if(axRef === 'paper' || axRef === 'pixel') { - ax = { cleanPos: Lib.num }; + cleanPos = Lib.ensureNumber; pos = coerce(attr, dflt); } else { - ax = axes.getFromId(gd, axRef); + var ax = axes.getFromId(gd, axRef); dflt = ax.fraction2r(dflt); pos = coerce(attr, dflt); + cleanPos = ax.cleanPos; } - containerOut[attr] = ax.cleanPos(pos); + containerOut[attr] = cleanPos(pos); }; axes.cleanPosition = function(pos, gd, axRef) { - var ax = (axRef === 'paper' || axRef === 'pixel') ? - { cleanPos: Lib.num } : - axes.getFromId(gd, axRef); + var cleanPos = (axRef === 'paper' || axRef === 'pixel') ? + Lib.ensureNumber : + axes.getFromId(gd, axRef).cleanPos; - return ax.cleanPos(pos); + return cleanPos(pos); }; axes.getDataToCoordFunc = function(gd, trace, target, targetArray) { diff --git a/src/plots/cartesian/set_convert.js b/src/plots/cartesian/set_convert.js index b5f76eec5e1..1469d99a387 100644 --- a/src/plots/cartesian/set_convert.js +++ b/src/plots/cartesian/set_convert.js @@ -16,7 +16,7 @@ var Lib = require('../../lib'); var cleanNumber = Lib.cleanNumber; var ms2DateTime = Lib.ms2DateTime; var dateTime2ms = Lib.dateTime2ms; -var num = Lib.num; +var ensureNumber = Lib.ensureNumber; var numConstants = require('../../constants/numerical'); var FP_SAFE = numConstants.FP_SAFE; @@ -159,8 +159,8 @@ module.exports = function setConvert(ax, fullLayout) { function p2l(px) { return (px - ax._b) / ax._m; } // conversions among c/l/p are fairly simple - do them together for all axis types - ax.c2l = (ax.type === 'log') ? toLog : num; - ax.l2c = (ax.type === 'log') ? fromLog : num; + ax.c2l = (ax.type === 'log') ? toLog : ensureNumber; + ax.l2c = (ax.type === 'log') ? fromLog : ensureNumber; ax.l2p = l2p; ax.p2l = p2l; @@ -176,12 +176,12 @@ module.exports = function setConvert(ax, fullLayout) { if(['linear', '-'].indexOf(ax.type) !== -1) { // all are data vals, but d and r need cleaning ax.d2r = ax.r2d = ax.d2c = ax.r2c = ax.d2l = ax.r2l = cleanNumber; - ax.c2d = ax.c2r = ax.l2d = ax.l2r = num; + ax.c2d = ax.c2r = ax.l2d = ax.l2r = ensureNumber; ax.d2p = ax.r2p = function(v) { return ax.l2p(cleanNumber(v)); }; ax.p2d = ax.p2r = p2l; - ax.cleanPos = num; + ax.cleanPos = ensureNumber; } else if(ax.type === 'log') { // d and c are data vals, r and l are logged (but d and r need cleaning) @@ -189,7 +189,7 @@ module.exports = function setConvert(ax, fullLayout) { ax.r2d = ax.r2c = function(v) { return fromLog(cleanNumber(v)); }; ax.d2c = ax.r2l = cleanNumber; - ax.c2d = ax.l2r = num; + ax.c2d = ax.l2r = ensureNumber; ax.c2r = toLog; ax.l2d = fromLog; @@ -200,7 +200,7 @@ module.exports = function setConvert(ax, fullLayout) { ax.r2p = function(v) { return ax.l2p(cleanNumber(v)); }; ax.p2r = p2l; - ax.cleanPos = num; + ax.cleanPos = ensureNumber; } else if(ax.type === 'date') { // r and d are date strings, l and c are ms @@ -233,7 +233,7 @@ module.exports = function setConvert(ax, fullLayout) { ax.d2r = ax.d2l_noadd = getCategoryIndex; - ax.l2r = ax.r2c = ax.c2r = num; + ax.l2r = ax.r2c = ax.c2r = ensureNumber; ax.r2l = getCategoryIndex; ax.d2p = function(v) { return ax.l2p(getCategoryIndex(v)); }; @@ -243,7 +243,7 @@ module.exports = function setConvert(ax, fullLayout) { ax.cleanPos = function(v) { if(typeof v === 'string' && v !== '') return v; - return num(v); + return ensureNumber(v); }; } From 4434fadf634c3e0a877706fea38dc9358bce9d09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 31 May 2017 12:06:53 -0400 Subject: [PATCH 7/8] add test case :lock: axis categories in x/y/z column heatmaps --- test/jasmine/tests/heatmap_test.js | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/test/jasmine/tests/heatmap_test.js b/test/jasmine/tests/heatmap_test.js index 48cb7b13715..57c21b77a15 100644 --- a/test/jasmine/tests/heatmap_test.js +++ b/test/jasmine/tests/heatmap_test.js @@ -168,8 +168,8 @@ describe('heatmap convertColumnXYZ', function() { }; } - var xa = makeMockAxis(), - ya = makeMockAxis(); + var xa = makeMockAxis(); + var ya = makeMockAxis(); it('should convert x/y/z columns to z(x,y)', function() { trace = { @@ -305,8 +305,13 @@ describe('heatmap calc', function() { Plots.supplyDefaults(gd); var fullTrace = gd._fullData[0]; + var fullLayout = gd._fullLayout; - return Heatmap.calc(gd, fullTrace)[0]; + var out = Heatmap.calc(gd, fullTrace)[0]; + out._xcategories = fullLayout.xaxis._categories; + out._ycategories = fullLayout.yaxis._categories; + + return out; } it('should fill in bricks if x/y not given', function() { @@ -404,6 +409,25 @@ describe('heatmap calc', function() { expect(out.y).toBeCloseToArray([-0.5, 0.5]); expect(out.z).toBeCloseTo2DArray([[17, 18, 19]]); }); + + it('should handle the category x/y/z/ column case', function() { + var out = _calc({ + x: ['a', 'a', 'a', 'b', 'b', 'b', 'c', 'c', 'c'], + y: ['A', 'B', 'C', 'A', 'B', 'C', 'A', 'B', 'C'], + z: [0, 50, 100, 50, 0, 255, 100, 510, 1010] + }); + + expect(out.x).toBeCloseToArray([-0.5, 0.5, 1.5, 2.5]); + expect(out.y).toBeCloseToArray([-0.5, 0.5, 1.5, 2.5]); + expect(out.z).toBeCloseTo2DArray([ + [0, 50, 100], + [50, 0, 510], + [100, 255, 1010] + ]); + + expect(out._xcategories).toEqual(['a', 'b', 'c']); + expect(out._ycategories).toEqual(['A', 'B', 'C']); + }); }); describe('heatmap plot', function() { From 0fafe14a465621844da55ff4cfdd15b6e8d541c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 31 May 2017 14:26:20 -0400 Subject: [PATCH 8/8] add test case :lock: dates in x/y/z column heatmaps --- test/jasmine/tests/heatmap_test.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/jasmine/tests/heatmap_test.js b/test/jasmine/tests/heatmap_test.js index 57c21b77a15..a1b2c89cad7 100644 --- a/test/jasmine/tests/heatmap_test.js +++ b/test/jasmine/tests/heatmap_test.js @@ -428,6 +428,28 @@ describe('heatmap calc', function() { expect(out._xcategories).toEqual(['a', 'b', 'c']); expect(out._ycategories).toEqual(['A', 'B', 'C']); }); + + it('should handle the date x/y/z/ column case', function() { + var out = _calc({ + x: [ + '2016-01-01', '2016-01-01', '2016-01-01', + '2017-01-01', '2017-01-01', '2017-01-01', + '2017-06-01', '2017-06-01', '2017-06-01' + ], + y: [0, 1, 2, 0, 1, 2, 0, 1, 2], + z: [0, 50, 100, 50, 0, 255, 100, 510, 1010] + }); + + expect(out.x).toBeCloseToArray([ + 1435795200000, 1467417600000, 1489752000000, 1502798400000 + ]); + expect(out.y).toBeCloseToArray([-0.5, 0.5, 1.5, 2.5]); + expect(out.z).toBeCloseTo2DArray([ + [0, 50, 100], + [50, 0, 510], + [100, 255, 1010] + ]); + }); }); describe('heatmap plot', function() {