From 43b3e56e97d90a6769258ffd002a157322601971 Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Tue, 14 Feb 2017 13:35:08 -0500 Subject: [PATCH 1/7] Pass customdata through to scatter calcdata --- src/traces/scatter/arrays_to_calcdata.js | 1 + src/traces/scatter/attributes.js | 4 ++++ src/traces/scatter/defaults.js | 1 + test/jasmine/tests/calcdata_test.js | 15 +++++++++++++++ 4 files changed, 21 insertions(+) diff --git a/src/traces/scatter/arrays_to_calcdata.js b/src/traces/scatter/arrays_to_calcdata.js index 7cfcf57d2a3..8b81c42bcb7 100644 --- a/src/traces/scatter/arrays_to_calcdata.js +++ b/src/traces/scatter/arrays_to_calcdata.js @@ -16,6 +16,7 @@ var Lib = require('../../lib'); module.exports = function arraysToCalcdata(cd, trace) { Lib.mergeArray(trace.text, cd, 'tx'); + Lib.mergeArray(trace.customdata, cd, 'data'); Lib.mergeArray(trace.textposition, cd, 'tp'); if(trace.textfont) { Lib.mergeArray(trace.textfont.size, cd, 'ts'); diff --git a/src/traces/scatter/attributes.js b/src/traces/scatter/attributes.js index 0354006573d..5280abbe6d1 100644 --- a/src/traces/scatter/attributes.js +++ b/src/traces/scatter/attributes.js @@ -56,6 +56,10 @@ module.exports = { 'where `y0` is the starting coordinate and `dy` the step.' ].join(' ') }, + customdata: { + valType: 'data_array', + description: 'When markers present, assigns extra data to each scatter point DOM element' + }, dy: { valType: 'number', dflt: 1, diff --git a/src/traces/scatter/defaults.js b/src/traces/scatter/defaults.js index 6b71dbd4f6f..c0115bb2ab7 100644 --- a/src/traces/scatter/defaults.js +++ b/src/traces/scatter/defaults.js @@ -36,6 +36,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout return; } + coerce('customdata'); coerce('text'); coerce('mode', defaultMode); coerce('ids'); diff --git a/test/jasmine/tests/calcdata_test.js b/test/jasmine/tests/calcdata_test.js index 9ac9dda0e98..5449531d5fe 100644 --- a/test/jasmine/tests/calcdata_test.js +++ b/test/jasmine/tests/calcdata_test.js @@ -862,4 +862,19 @@ describe('calculated data and points', function() { }); }); }); + + describe('customdata', function() { + it('should pass customdata to the calcdata points', function() { + Plotly.plot(gd, [{ + x: [0, 1, 2, 3], + y: [4, 5, 6, 7], + customdata: ['a', 'b', 'c', 'd'] + }], {}); + + expect(gd.calcdata[0][0]).toEqual(jasmine.objectContaining({data: 'a'})); + expect(gd.calcdata[0][1]).toEqual(jasmine.objectContaining({data: 'b'})); + expect(gd.calcdata[0][2]).toEqual(jasmine.objectContaining({data: 'c'})); + expect(gd.calcdata[0][3]).toEqual(jasmine.objectContaining({data: 'd'})); + }); + }); }); From cdfbfa53a2633d3130b841cc41787b3467cf0884 Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Tue, 14 Feb 2017 13:37:16 -0500 Subject: [PATCH 2/7] Remove marker caveat --- src/traces/scatter/attributes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/traces/scatter/attributes.js b/src/traces/scatter/attributes.js index 5280abbe6d1..fc1412e9f05 100644 --- a/src/traces/scatter/attributes.js +++ b/src/traces/scatter/attributes.js @@ -58,7 +58,7 @@ module.exports = { }, customdata: { valType: 'data_array', - description: 'When markers present, assigns extra data to each scatter point DOM element' + description: 'Assigns extra data to each scatter point DOM element' }, dy: { valType: 'number', From 89b56b78ac900c9ac5a94123dc583aa4511bbab4 Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Wed, 15 Feb 2017 11:53:12 -0500 Subject: [PATCH 3/7] Explicitly test passing object as customdata --- test/jasmine/tests/calcdata_test.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/jasmine/tests/calcdata_test.js b/test/jasmine/tests/calcdata_test.js index 5449531d5fe..d34664bfd1a 100644 --- a/test/jasmine/tests/calcdata_test.js +++ b/test/jasmine/tests/calcdata_test.js @@ -866,15 +866,14 @@ describe('calculated data and points', function() { describe('customdata', function() { it('should pass customdata to the calcdata points', function() { Plotly.plot(gd, [{ - x: [0, 1, 2, 3], - y: [4, 5, 6, 7], - customdata: ['a', 'b', 'c', 'd'] + x: [0, 1, 3], + y: [4, 5, 7], + customdata: ['a', 'b', {foo: 'bar'}] }], {}); expect(gd.calcdata[0][0]).toEqual(jasmine.objectContaining({data: 'a'})); expect(gd.calcdata[0][1]).toEqual(jasmine.objectContaining({data: 'b'})); - expect(gd.calcdata[0][2]).toEqual(jasmine.objectContaining({data: 'c'})); - expect(gd.calcdata[0][3]).toEqual(jasmine.objectContaining({data: 'd'})); + expect(gd.calcdata[0][2]).toEqual(jasmine.objectContaining({data: {foo: 'bar'}})); }); }); }); From 8ce3ded46c61494266a27df92e09e71c1c90301f Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Wed, 15 Feb 2017 12:36:52 -0500 Subject: [PATCH 4/7] Add class for customdata --- src/traces/scatter/style.js | 13 ++++++++--- test/jasmine/tests/scatter_test.js | 35 ++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/traces/scatter/style.js b/src/traces/scatter/style.js index 78339d3acf7..8315dca6bb9 100644 --- a/src/traces/scatter/style.js +++ b/src/traces/scatter/style.js @@ -24,9 +24,16 @@ module.exports = function style(gd) { s.selectAll('g.points') .each(function(d) { - d3.select(this).selectAll('path.point') - .call(Drawing.pointStyle, d.trace || d[0].trace); - d3.select(this).selectAll('text') + var el = d3.select(this); + var pt = el.selectAll('path.point'); + + pt.call(Drawing.pointStyle, d.trace || d[0].trace); + + pt.each(function(cd) { + d3.select(this).classed('plotly-customdata', cd.data !== null && cd.data !== undefined); + }); + + el.selectAll('text') .call(Drawing.textPointStyle, d.trace || d[0].trace); }); diff --git a/test/jasmine/tests/scatter_test.js b/test/jasmine/tests/scatter_test.js index 1dcb3abbee3..82358f36e49 100644 --- a/test/jasmine/tests/scatter_test.js +++ b/test/jasmine/tests/scatter_test.js @@ -1,8 +1,14 @@ +var d3 = require('d3'); var Scatter = require('@src/traces/scatter'); var makeBubbleSizeFn = require('@src/traces/scatter/make_bubble_size_func'); var linePoints = require('@src/traces/scatter/line_points'); var Lib = require('@src/lib'); +var Plotly = require('@lib/index'); +var createGraphDiv = require('../assets/create_graph_div'); +var destroyGraphDiv = require('../assets/destroy_graph_div'); +var fail = require('../assets/fail_test'); + describe('Test scatter', function() { 'use strict'; @@ -326,3 +332,32 @@ describe('Test scatter', function() { }); }); + +describe('end-to-end scatter tests', function() { + var gd; + + beforeEach(function() { + gd = createGraphDiv(); + }); + + afterEach(destroyGraphDiv); + + it('should add a plotly-customdata class to points with custom data', function(done) { + Plotly.plot(gd, [{ + x: [1, 2, 3, 4, 5, 6, 7], + y: [2, 3, 4, 5, 6, 7, 8], + customdata: [null, undefined, 0, false, {foo: 'bar'}, 'a'] + }]).then(function() { + var points = d3.selectAll('g.scatterlayer').selectAll('.point'); + + // Rather than just duplicating the logic, let's be explicit about + // what's expected. Specifially, only null and undefined (the default) + // do *not* add the class. + var expected = [false, false, true, true, true, true, false]; + + points.each(function(cd, i) { + expect(d3.select(this).classed('plotly-customdata')).toBe(expected[i]); + }); + }).catch(fail).then(done); + }); +}); From 2f78e4d04c3250086c26c21970a4cbc314ca62e6 Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Wed, 15 Feb 2017 13:16:04 -0500 Subject: [PATCH 5/7] Skip customdata loop if no customdata provided --- src/traces/scatter/style.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/traces/scatter/style.js b/src/traces/scatter/style.js index 8315dca6bb9..d5799c669af 100644 --- a/src/traces/scatter/style.js +++ b/src/traces/scatter/style.js @@ -26,15 +26,18 @@ module.exports = function style(gd) { .each(function(d) { var el = d3.select(this); var pt = el.selectAll('path.point'); + var trace = d.trace || d[0].trace; - pt.call(Drawing.pointStyle, d.trace || d[0].trace); + pt.call(Drawing.pointStyle, trace); - pt.each(function(cd) { - d3.select(this).classed('plotly-customdata', cd.data !== null && cd.data !== undefined); - }); + if(trace.customdata) { + pt.each(function(cd) { + d3.select(this).classed('plotly-customdata', cd.data !== null && cd.data !== undefined); + }); + } el.selectAll('text') - .call(Drawing.textPointStyle, d.trace || d[0].trace); + .call(Drawing.textPointStyle, trace); }); s.selectAll('g.trace path.js-line') From 3d992f0bad2010ffdb948d4fd2e0dde92507ce1c Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Wed, 15 Feb 2017 13:17:57 -0500 Subject: [PATCH 6/7] Change variable name for clarity --- src/traces/scatter/style.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/traces/scatter/style.js b/src/traces/scatter/style.js index d5799c669af..ea9a66b6ded 100644 --- a/src/traces/scatter/style.js +++ b/src/traces/scatter/style.js @@ -25,13 +25,13 @@ module.exports = function style(gd) { s.selectAll('g.points') .each(function(d) { var el = d3.select(this); - var pt = el.selectAll('path.point'); + var pts = el.selectAll('path.point'); var trace = d.trace || d[0].trace; - pt.call(Drawing.pointStyle, trace); + pts.call(Drawing.pointStyle, trace); if(trace.customdata) { - pt.each(function(cd) { + pts.each(function(cd) { d3.select(this).classed('plotly-customdata', cd.data !== null && cd.data !== undefined); }); } From c16cbfb44ecb5f2cd2be26ceae9251f2b683898e Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Wed, 15 Feb 2017 13:35:45 -0500 Subject: [PATCH 7/7] Test unsetting plotly-customdata via animation --- src/traces/scatter/plot.js | 7 ++++++- src/traces/scatter/style.js | 6 ------ test/jasmine/tests/scatter_test.js | 11 +++++++++++ 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/traces/scatter/plot.js b/src/traces/scatter/plot.js index b9dde96cd7a..7da03ffe2fe 100644 --- a/src/traces/scatter/plot.js +++ b/src/traces/scatter/plot.js @@ -428,9 +428,14 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition } join.each(function(d) { - var sel = transition(d3.select(this)); + var el = d3.select(this); + var sel = transition(el); Drawing.translatePoint(d, sel, xa, ya); Drawing.singlePointStyle(d, sel, trace); + + if(trace.customdata) { + el.classed('plotly-customdata', d.data !== null && d.data !== undefined); + } }); if(hasTransition) { diff --git a/src/traces/scatter/style.js b/src/traces/scatter/style.js index ea9a66b6ded..9f0c17a935d 100644 --- a/src/traces/scatter/style.js +++ b/src/traces/scatter/style.js @@ -30,12 +30,6 @@ module.exports = function style(gd) { pts.call(Drawing.pointStyle, trace); - if(trace.customdata) { - pts.each(function(cd) { - d3.select(this).classed('plotly-customdata', cd.data !== null && cd.data !== undefined); - }); - } - el.selectAll('text') .call(Drawing.textPointStyle, trace); }); diff --git a/test/jasmine/tests/scatter_test.js b/test/jasmine/tests/scatter_test.js index 82358f36e49..210c418341e 100644 --- a/test/jasmine/tests/scatter_test.js +++ b/test/jasmine/tests/scatter_test.js @@ -358,6 +358,17 @@ describe('end-to-end scatter tests', function() { points.each(function(cd, i) { expect(d3.select(this).classed('plotly-customdata')).toBe(expected[i]); }); + + return Plotly.animate(gd, [{ + data: [{customdata: []}] + }], {frame: {redraw: false, duration: 0}}); + }).then(function() { + var points = d3.selectAll('g.scatterlayer').selectAll('.point'); + + points.each(function() { + expect(d3.select(this).classed('plotly-customdata')).toBe(false); + }); + }).catch(fail).then(done); }); });