From fd17e6fc8a1ec4be7555903403fd192aef53d8e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Etienne=20T=C3=A9treault-Pinard?= Date: Tue, 18 Jul 2017 11:30:59 -0400 Subject: [PATCH 1/5] add test case for hover on traces w/ colorbar w/ set ticktext tickvals - failing on `master` during appendArrayPointValue when they are less items in 'colorbar.ticktext; or 'colorbar.tickvals' then pointNumber[0]. - test case is for surface, but same happens for other trace types with 2d coords. --- test/jasmine/tests/gl_plot_interact_test.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/jasmine/tests/gl_plot_interact_test.js b/test/jasmine/tests/gl_plot_interact_test.js index ae2b085cf16..7e06a5de92f 100644 --- a/test/jasmine/tests/gl_plot_interact_test.js +++ b/test/jasmine/tests/gl_plot_interact_test.js @@ -245,6 +245,20 @@ describe('Test gl3d plots', function() { expect(label.size()).toEqual(1); expect(label.select('text').text()).toEqual('2'); + + return Plotly.restyle(gd, { + 'colorbar.tickvals': [[25]], + 'colorbar.ticktext': [['single tick!']] + }); + }) + .then(_hover) + .then(function() { + assertEventData(1, 2, 43, 0, [1, 2], { + 'hoverinfo': 'y', + 'hoverlabel.font.color': 'cyan', + 'colorbar.tickvals': undefined, + 'colorbar.ticktext': undefined + }); }) .then(done); }); From 0aa87c9da6914bad762e36c2571886db4c93ad93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Etienne=20T=C3=A9treault-Pinard?= Date: Tue, 18 Jul 2017 11:33:45 -0400 Subject: [PATCH 2/5] fix hover for traces w/ colorbar w/ set ticktext tickvals - don't include point values of 1d array attributes when pointNumber is 2d in appendArrayPointValue. - note that 1d array attributes (e.g. `x` and `y`) can still be set in the `module.hoverPoints`. --- src/components/fx/helpers.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/components/fx/helpers.js b/src/components/fx/helpers.js index 0f6b0aa52d4..3b0c81623ae 100644 --- a/src/components/fx/helpers.js +++ b/src/components/fx/helpers.js @@ -108,9 +108,14 @@ exports.appendArrayPointValue = function(pointData, trace, pointNumber) { if(pointData[key] === undefined) { var val = Lib.nestedProperty(trace, astr).get(); - pointData[key] = Array.isArray(pointNumber) ? - val[pointNumber[0]][pointNumber[1]] : - val[pointNumber]; + + if(Array.isArray(pointNumber)) { + if(Array.isArray(val) && Array.isArray(val[0])) { + pointData[key] = val[pointNumber[0]][pointNumber[1]]; + } + } else { + pointData[key] = val[pointNumber]; + } } } }; From 0d91074aa1ef039c74045370867d3ce9001ea25c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Etienne=20T=C3=A9treault-Pinard?= Date: Tue, 18 Jul 2017 14:55:38 -0400 Subject: [PATCH 3/5] exclude values of incomplete 2d arrays from event data --- src/components/fx/helpers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/fx/helpers.js b/src/components/fx/helpers.js index 3b0c81623ae..9ce5acd160b 100644 --- a/src/components/fx/helpers.js +++ b/src/components/fx/helpers.js @@ -110,7 +110,7 @@ exports.appendArrayPointValue = function(pointData, trace, pointNumber) { var val = Lib.nestedProperty(trace, astr).get(); if(Array.isArray(pointNumber)) { - if(Array.isArray(val) && Array.isArray(val[0])) { + if(Array.isArray(val) && Array.isArray(val[pointNumber[0]])) { pointData[key] = val[pointNumber[0]][pointNumber[1]]; } } else { From aa9312313393764f63c5e7076986c45d73fa24a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Etienne=20T=C3=A9treault-Pinard?= Date: Tue, 18 Jul 2017 14:59:50 -0400 Subject: [PATCH 4/5] manually exclude colorbar 'tickvals' and 'ticktext' from array attrs - as they scale independently of coordinates arrays, so shouldn't be filtered nor included in point event data - do not add a schema field for this now. + wait until solution become clearer. --- src/plot_api/plot_schema.js | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/plot_api/plot_schema.js b/src/plot_api/plot_schema.js index c3a259f1384..27fab419cd7 100644 --- a/src/plot_api/plot_schema.js +++ b/src/plot_api/plot_schema.js @@ -142,13 +142,25 @@ exports.isValObject = function(obj) { * list of array attributes for the given trace */ exports.findArrayAttributes = function(trace) { - var arrayAttributes = [], - stack = []; + var arrayAttributes = []; + var stack = []; function callback(attr, attrName, attrs, level) { stack = stack.slice(0, level).concat([attrName]); - var splittableAttr = attr && (attr.valType === 'data_array' || attr.arrayOk === true); + var splittableAttr = ( + attr && + (attr.valType === 'data_array' || attr.arrayOk === true) && + !(stack[level - 1] === 'colorbar' && (attrName === 'ticktext' || attrName === 'tickvals')) + ); + + // Manually exclude 'colorbar.tickvals' and 'colorbar.ticktext' for now + // which are declared as `valType: 'data_array'` but scale independently of + // the coordinate arrays. + // + // Down the road, we might want to add a schema field (e.g `uncorrelatedArray: true`) + // to distinguish attributes of the likes. + if(!splittableAttr) return; var astr = toAttrString(stack); From 2050e3fb3a9831326e2303ef329ffe29f0875e1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Etienne=20T=C3=A9treault-Pinard?= Date: Tue, 18 Jul 2017 15:00:06 -0400 Subject: [PATCH 5/5] add a few more event data tests --- test/jasmine/tests/cartesian_interact_test.js | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/test/jasmine/tests/cartesian_interact_test.js b/test/jasmine/tests/cartesian_interact_test.js index 3bd985c5b9c..e59b79188dd 100644 --- a/test/jasmine/tests/cartesian_interact_test.js +++ b/test/jasmine/tests/cartesian_interact_test.js @@ -440,3 +440,83 @@ describe('axis zoom/pan and main plot zoom', function() { .then(done); }); }); + +describe('Event data:', function() { + var gd; + + beforeEach(function() { + gd = createGraphDiv(); + }); + + afterEach(destroyGraphDiv); + + function _hover(px, py) { + return new Promise(function(resolve, reject) { + gd.once('plotly_hover', function(d) { + delete gd._lastHoverTime; + resolve(d); + }); + + mouseEvent('mousemove', px, py); + + setTimeout(function() { + reject('plotly_hover did not get called!'); + }, 100); + }); + } + + it('should have correct content for *scatter* traces', function(done) { + Plotly.plot(gd, [{ + y: [1, 2, 1], + marker: { + color: [20, 30, 10], + colorbar: { + tickvals: [25], + ticktext: ['one single tick'] + } + } + }], { + width: 500, + height: 500 + }) + .then(function() { return _hover(200, 200); }) + .then(function(d) { + var pt = d.points[0]; + + expect(pt.y).toBe(2, 'y'); + expect(pt['marker.color']).toBe(30, 'marker.color'); + expect('marker.colorbar.tickvals' in pt).toBe(false, 'marker.colorbar.tickvals'); + expect('marker.colorbar.ticktext' in pt).toBe(false, 'marker.colorbar.ticktext'); + }) + .catch(fail) + .then(done); + }); + + it('should have correct content for *heatmap* traces', function(done) { + Plotly.plot(gd, [{ + type: 'heatmap', + z: [[1, 2, 1], [2, 3, 1]], + colorbar: { + tickvals: [2], + ticktext: ['one single tick'] + }, + text: [['incomplete array']], + ids: [['incomplete array']] + }], { + width: 500, + height: 500 + }) + .then(function() { return _hover(200, 200); }) + .then(function(d) { + var pt = d.points[0]; + + expect(pt.z).toBe(3, 'z'); + expect(pt.text).toBe(undefined, 'undefined text items are included'); + expect('id' in pt).toBe(false, 'undefined ids items are not included'); + expect('marker.colorbar.tickvals' in pt).toBe(false, 'marker.colorbar.tickvals'); + expect('marker.colorbar.ticktext' in pt).toBe(false, 'marker.colorbar.ticktext'); + }) + .catch(fail) + .then(done); + }); +});