From 6c07e2eba3105a4ce327d00868ff0961f410cf39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 8 May 2017 17:41:40 -0400 Subject: [PATCH 1/3] use BADNUM instead of false to denote bad calcdata items - unfinished business from https://github.com/plotly/plotly.js/pull/1564/commits/b5cd57308c60b584ba47ba93b35bb02efdd15418 --- src/traces/scatter/calc.js | 3 ++- test/jasmine/tests/calcdata_test.js | 15 ++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/traces/scatter/calc.js b/src/traces/scatter/calc.js index 1708af0144b..e5c64209128 100644 --- a/src/traces/scatter/calc.js +++ b/src/traces/scatter/calc.js @@ -12,6 +12,7 @@ var isNumeric = require('fast-isnumeric'); var Axes = require('../../plots/cartesian/axes'); +var BADNUM = require('../../constants/numerical').BADNUM; var subTypes = require('./subtypes'); var calcColorscale = require('./colorscale_calc'); @@ -114,7 +115,7 @@ module.exports = function calc(gd, trace) { var cd = new Array(serieslen); for(i = 0; i < serieslen; i++) { cd[i] = (isNumeric(x[i]) && isNumeric(y[i])) ? - {x: x[i], y: y[i]} : {x: false, y: false}; + {x: x[i], y: y[i]} : {x: BADNUM, y: BADNUM}; if(trace.ids) { cd[i].id = String(trace.ids[i]); diff --git a/test/jasmine/tests/calcdata_test.js b/test/jasmine/tests/calcdata_test.js index d34664bfd1a..2f1ef1f2f59 100644 --- a/test/jasmine/tests/calcdata_test.js +++ b/test/jasmine/tests/calcdata_test.js @@ -1,5 +1,6 @@ var Plotly = require('@lib/index'); +var BADNUM = require('@src/constants/numerical').BADNUM; var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); @@ -18,15 +19,15 @@ describe('calculated data and points', function() { it('should exclude null and undefined points when false', function() { Plotly.plot(gd, [{ x: [1, 2, 3, undefined, 5], y: [1, null, 3, 4, 5]}], {}); - expect(gd.calcdata[0][1]).toEqual(jasmine.objectContaining({ x: false, y: false})); - expect(gd.calcdata[0][3]).toEqual(jasmine.objectContaining({ x: false, y: false})); + expect(gd.calcdata[0][1]).toEqual(jasmine.objectContaining({ x: BADNUM, y: BADNUM})); + expect(gd.calcdata[0][3]).toEqual(jasmine.objectContaining({ x: BADNUM, y: BADNUM})); }); it('should exclude null and undefined points as categories when false', function() { Plotly.plot(gd, [{ x: [1, 2, 3, undefined, 5], y: [1, null, 3, 4, 5] }], { xaxis: { type: 'category' }}); - expect(gd.calcdata[0][1]).toEqual(jasmine.objectContaining({ x: false, y: false})); - expect(gd.calcdata[0][3]).toEqual(jasmine.objectContaining({ x: false, y: false})); + expect(gd.calcdata[0][1]).toEqual(jasmine.objectContaining({ x: BADNUM, y: BADNUM})); + expect(gd.calcdata[0][3]).toEqual(jasmine.objectContaining({ x: BADNUM, y: BADNUM})); }); }); @@ -192,9 +193,9 @@ describe('calculated data and points', function() { }}); expect(gd.calcdata[0][0]).toEqual(jasmine.objectContaining({x: 4, y: 15})); - expect(gd.calcdata[0][1]).toEqual(jasmine.objectContaining({ x: false, y: false})); + expect(gd.calcdata[0][1]).toEqual(jasmine.objectContaining({ x: BADNUM, y: BADNUM})); expect(gd.calcdata[0][2]).toEqual(jasmine.objectContaining({x: 3, y: 12})); - expect(gd.calcdata[0][3]).toEqual(jasmine.objectContaining({ x: false, y: false})); + expect(gd.calcdata[0][3]).toEqual(jasmine.objectContaining({ x: BADNUM, y: BADNUM})); expect(gd.calcdata[0][4]).toEqual(jasmine.objectContaining({x: 2, y: 14})); }); @@ -269,7 +270,7 @@ describe('calculated data and points', function() { }}); expect(gd.calcdata[0][0]).toEqual(jasmine.objectContaining({x: 6, y: 15})); - expect(gd.calcdata[0][1]).toEqual(jasmine.objectContaining({x: false, y: false})); + expect(gd.calcdata[0][1]).toEqual(jasmine.objectContaining({x: BADNUM, y: BADNUM})); expect(gd.calcdata[0][2]).toEqual(jasmine.objectContaining({x: 5, y: 12})); expect(gd.calcdata[0][3]).toEqual(jasmine.objectContaining({x: 0, y: 13})); expect(gd.calcdata[0][4]).toEqual(jasmine.objectContaining({x: 3, y: 14})); From 4c48ec6ad43f9a148dd8fcb8134b2fec33876b26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 8 May 2017 17:42:26 -0400 Subject: [PATCH 2/3] skip over BADNUM in scatter select routine - so that BADNUM items are not included in the 'plotly_selected' event data. --- src/traces/scatter/select.js | 6 ++++++ test/jasmine/tests/select_test.js | 27 +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/traces/scatter/select.js b/src/traces/scatter/select.js index 8ad3fc12030..215b6da4930 100644 --- a/src/traces/scatter/select.js +++ b/src/traces/scatter/select.js @@ -10,6 +10,7 @@ 'use strict'; var subtypes = require('./subtypes'); +var BADNUM = require('../../constants/numerical').BADNUM; var DESELECTDIM = 0.2; @@ -40,6 +41,11 @@ module.exports = function selectPoints(searchInfo, polygon) { di = cd[i]; x = xa.c2p(di.x); y = ya.c2p(di.y); + + if(x === BADNUM || y === BADNUM) { + continue; + } + if(polygon.contains([x, y])) { selection.push({ curveNumber: curveNumber, diff --git a/test/jasmine/tests/select_test.js b/test/jasmine/tests/select_test.js index f2be564a41d..9b9876a2db1 100644 --- a/test/jasmine/tests/select_test.js +++ b/test/jasmine/tests/select_test.js @@ -322,4 +322,31 @@ describe('select box and lasso', function() { done(); }); }); + + it('should skip over BADNUM items', function(done) { + var data = [{ + mode: 'markers', + x: [null, undefined, NaN, 0, 'NA'], + y: [NaN, null, undefined, 0, 'NA'] + }]; + var layout = { + dragmode: 'select', + width: 400, + heigth: 400, + }; + var gd = createGraphDiv(); + var pts; + + Plotly.plot(gd, data, layout).then(function() { + gd.on('plotly_selected', function(data) { + pts = data.points; + }); + + drag([[100, 100], [300, 300]]); + expect(pts.length).toEqual(1); + expect(pts[0].x).toEqual(0); + expect(pts[0].y).toEqual(0); + }) + .then(done); + }); }); From 80cee013a0a6b70092fc1ecb3ac7ba6ed812d81d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 9 May 2017 10:36:12 -0400 Subject: [PATCH 3/3] move BADNUM checks in polygon contains - also make sure that 'lasso' dragmode is tested --- src/lib/polygon.js | 5 +++-- src/traces/scatter/select.js | 5 ----- test/jasmine/tests/select_test.js | 8 ++++++++ 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/lib/polygon.js b/src/lib/polygon.js index befd593e275..d30d1fda104 100644 --- a/src/lib/polygon.js +++ b/src/lib/polygon.js @@ -10,6 +10,7 @@ 'use strict'; var dot = require('./matrix').dot; +var BADNUM = require('../constants/numerical').BADNUM; var polygon = module.exports = {}; @@ -73,7 +74,7 @@ polygon.tester = function tester(ptsIn) { var x = pt[0], y = pt[1]; - if(x < xmin || x > xmax || y < ymin || y > ymax) { + if(x === BADNUM || x < xmin || x > xmax || y === BADNUM || y < ymin || y > ymax) { // pt is outside the bounding box of polygon return false; } @@ -86,7 +87,7 @@ polygon.tester = function tester(ptsIn) { var x = pt[0], y = pt[1]; - if(x < xmin || x > xmax || y < ymin || y > ymax) { + if(x === BADNUM || x < xmin || x > xmax || y === BADNUM || y < ymin || y > ymax) { // pt is outside the bounding box of polygon return false; } diff --git a/src/traces/scatter/select.js b/src/traces/scatter/select.js index 215b6da4930..222d2b22566 100644 --- a/src/traces/scatter/select.js +++ b/src/traces/scatter/select.js @@ -10,7 +10,6 @@ 'use strict'; var subtypes = require('./subtypes'); -var BADNUM = require('../../constants/numerical').BADNUM; var DESELECTDIM = 0.2; @@ -42,10 +41,6 @@ module.exports = function selectPoints(searchInfo, polygon) { x = xa.c2p(di.x); y = ya.c2p(di.y); - if(x === BADNUM || y === BADNUM) { - continue; - } - if(polygon.contains([x, y])) { selection.push({ curveNumber: curveNumber, diff --git a/test/jasmine/tests/select_test.js b/test/jasmine/tests/select_test.js index 9b9876a2db1..cbad8d1504c 100644 --- a/test/jasmine/tests/select_test.js +++ b/test/jasmine/tests/select_test.js @@ -346,6 +346,14 @@ describe('select box and lasso', function() { expect(pts.length).toEqual(1); expect(pts[0].x).toEqual(0); expect(pts[0].y).toEqual(0); + + return Plotly.relayout(gd, 'dragmode', 'lasso'); + }) + .then(function() { + drag([[100, 100], [100, 300], [300, 300], [300, 100], [100, 100]]); + expect(pts.length).toEqual(1); + expect(pts[0].x).toEqual(0); + expect(pts[0].y).toEqual(0); }) .then(done); });