From 858a3388a316544ee298ef86d65d70c83a160426 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 25 May 2016 19:16:46 -0400 Subject: [PATCH 1/6] generalize Fx.hover to non-cartesian cases: - don't assume that fullLayout._plots is defined - don't assume that fullLayout._plots[''] is defined - use fullLayout[spId]._subplot.{x,y}axis as subplot x/y axes in non-cartesian cases - perf: replace two .map calls with one for loop --- src/plots/cartesian/graph_interact.js | 58 ++++++++++++++++++--------- 1 file changed, 38 insertions(+), 20 deletions(-) diff --git a/src/plots/cartesian/graph_interact.js b/src/plots/cartesian/graph_interact.js index 02169649378..fdb28ea99f5 100644 --- a/src/plots/cartesian/graph_interact.js +++ b/src/plots/cartesian/graph_interact.js @@ -310,27 +310,45 @@ function hover(gd, evt, subplot) { if(!subplot) subplot = 'xy'; + // if the user passed in an array of subplots, + // use those instead of finding overlayed plots + var subplots = Array.isArray(subplot) ? subplot : [subplot]; + var fullLayout = gd._fullLayout, - plotinfo = fullLayout._plots[subplot], - - //If the user passed in an array of subplots, use those instead of finding overlayed plots - subplots = Array.isArray(subplot) ? - subplot : - // list of all overlaid subplots to look at - [subplot].concat(plotinfo.overlays - .map(function(pi) { return pi.id; })), - - xaArray = subplots.map(function(spId) { - var ternary = (gd._fullLayout[spId] || {})._ternary; - if(ternary) return ternary.xaxis; - return Axes.getFromId(gd, spId, 'x'); - }), - yaArray = subplots.map(function(spId) { - var ternary = (gd._fullLayout[spId] || {})._ternary; - if(ternary) return ternary.yaxis; - return Axes.getFromId(gd, spId, 'y'); - }), - hovermode = evt.hovermode || fullLayout.hovermode; + plots = fullLayout._plots || [], + plotinfo = plots[subplot]; + + // list of all overlaid subplots to look at + if(plotinfo) { + var overlayedSubplots = plotinfo.overlays.map(function(pi) { + return pi.id; + }); + + subplots = subplots.concat(overlayedSubplots); + } + + var len = subplots.length, + xaArray = new Array(len), + yaArray = new Array(len); + + for(var i = 0; i < len; i++) { + var spId = subplots[i]; + + // 'cartesian' case + var plotObj = plots[spId]; + if(plotObj) { + xaArray[i] = plotObj.xaxis; + yaArray[i] = plotObj.yaxis; + continue; + } + + // other subplot types + var _subplot = fullLayout[spId]._subplot; + xaArray[i] = _subplot.xaxis; + yaArray[i] = _subplot.yaxis; + } + + var hovermode = evt.hovermode || fullLayout.hovermode; if(['x', 'y', 'closest'].indexOf(hovermode) === -1 || !gd.calcdata || gd.querySelector('.zoombox') || gd._dragging) { From 27683c25e692374b21f5b54c38413e575ef159e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 25 May 2016 19:17:32 -0400 Subject: [PATCH 2/6] store ref to ternary subplot object in fullLayout.ternary?._subplot --- src/plots/ternary/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plots/ternary/index.js b/src/plots/ternary/index.js index 2db0226490d..99e9b22cd61 100644 --- a/src/plots/ternary/index.js +++ b/src/plots/ternary/index.js @@ -50,7 +50,7 @@ exports.plot = function plotTernary(gd) { fullLayout ); - fullLayout[ternaryId]._ternary = ternary; + fullLayout[ternaryId]._subplot = ternary; } ternary.plot(fullTernaryData, fullLayout, gd._promises); From 39b8acf54814c6cb4b33ac797a5027637edf7e07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 25 May 2016 19:18:08 -0400 Subject: [PATCH 3/6] don't hack into fullLayout._plots in ternary code --- src/plots/ternary/ternary.js | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/plots/ternary/ternary.js b/src/plots/ternary/ternary.js index 7939463fbd1..b9895cacae5 100644 --- a/src/plots/ternary/ternary.js +++ b/src/plots/ternary/ternary.js @@ -675,19 +675,6 @@ proto.initInteractions = function() { dragger.onclick = function(evt) { fx.click(gd, evt); }; - - // make a fake plotinfo for fx.hover - // it hardly uses it, could probably be refactored out... - // but specifying subplot by name does seem nice for js applications - // that want to hook into this. - if(!gd._fullLayout._plots) gd._fullLayout._plots = {}; - gd._fullLayout._plots[_this.id] = { - overlays: [], - xaxis: _this.xaxis, - yaxis: _this.yaxis, - x: function() { return _this.xaxis; }, - y: function() { return _this.yaxis; } - }; }; function removeZoombox(gd) { From 92da0e3906991711b75dfa0b4963b4196de47d50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 25 May 2016 19:22:12 -0400 Subject: [PATCH 4/6] sub ternary._ternary -> ternary._subplot --- src/plots/ternary/index.js | 4 ++-- test/jasmine/tests/scatterternary_test.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/plots/ternary/index.js b/src/plots/ternary/index.js index 99e9b22cd61..d55cec7c58a 100644 --- a/src/plots/ternary/index.js +++ b/src/plots/ternary/index.js @@ -38,7 +38,7 @@ exports.plot = function plotTernary(gd) { for(var i = 0; i < ternaryIds.length; i++) { var ternaryId = ternaryIds[i], fullTernaryData = Plots.getSubplotData(fullData, 'ternary', ternaryId), - ternary = fullLayout[ternaryId]._ternary; + ternary = fullLayout[ternaryId]._subplot; // If ternary is not instantiated, create one! if(ternary === undefined) { @@ -62,7 +62,7 @@ exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) for(var i = 0; i < oldTernaryKeys.length; i++) { var oldTernaryKey = oldTernaryKeys[i]; - var oldTernary = oldFullLayout[oldTernaryKey]._ternary; + var oldTernary = oldFullLayout[oldTernaryKey]._subplot; if(!newFullLayout[oldTernaryKey] && !!oldTernary) { oldTernary.plotContainer.remove(); diff --git a/test/jasmine/tests/scatterternary_test.js b/test/jasmine/tests/scatterternary_test.js index e2d6c293085..470ec336cb4 100644 --- a/test/jasmine/tests/scatterternary_test.js +++ b/test/jasmine/tests/scatterternary_test.js @@ -285,7 +285,7 @@ describe('scatterternary hover', function() { beforeEach(function() { var cd = gd.calcdata, - ternary = gd._fullLayout.ternary._ternary; + ternary = gd._fullLayout.ternary._subplot; pointData = { index: false, From 76ba24bf9738ec7d1a713123de1fd85bfd481062 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 27 May 2016 11:59:33 -0400 Subject: [PATCH 5/6] remove nodes inside scatterlayer in scatterternary plot step --- src/traces/scatterternary/plot.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/traces/scatterternary/plot.js b/src/traces/scatterternary/plot.js index 81eccb22518..ca866ce7038 100644 --- a/src/traces/scatterternary/plot.js +++ b/src/traces/scatterternary/plot.js @@ -13,11 +13,16 @@ var scatterPlot = require('../scatter/plot'); module.exports = function plot(ternary, data) { + var plotContainer = ternary.plotContainer; + + // remove all nodes inside the scatter layer + plotContainer.select('.scatterlayer').selectAll('*').remove(); + // mimic cartesian plotinfo var plotinfo = { x: function() { return ternary.xaxis; }, y: function() { return ternary.yaxis; }, - plot: ternary.plotContainer + plot: plotContainer }; var calcdata = new Array(data.length), From 7ff5ca9bbcba68bc651c566cd015cf4fb73246b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 27 May 2016 12:22:45 -0400 Subject: [PATCH 6/6] add hover label on overlaid axis test case --- test/jasmine/tests/hover_label_test.js | 31 ++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index 0567ea2ffa2..8dd124a8a51 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -574,3 +574,34 @@ describe('hover info on stacked subplots', function() { }); }); }); + + +describe('hover info on overlaid subplots', function() { + 'use strict'; + + afterEach(destroyGraphDiv); + + it('should respond to hover', function(done) { + var mock = require('@mocks/autorange-tozero-rangemode.json'); + + Plotly.plot(createGraphDiv(), mock.data, mock.layout).then(function() { + mouseEvent('mousemove', 775, 352); + + var axisText = d3.selectAll('g.axistext'), + hoverText = d3.selectAll('g.hovertext'); + + expect(axisText.size()).toEqual(1, 'with 1 label on axis'); + expect(hoverText.size()).toEqual(2, 'with 2 labels on the overlaid pts'); + + expect(axisText.select('text').html()).toEqual('1', 'with correct axis label'); + + var textNodes = hoverText.selectAll('text'); + + expect(textNodes[0][0].innerHTML).toEqual('Take Rate', 'with correct hover labels'); + expect(textNodes[0][1].innerHTML).toEqual('0.35', 'with correct hover labels'); + expect(textNodes[1][0].innerHTML).toEqual('Revenue', 'with correct hover labels'); + expect(textNodes[1][1].innerHTML).toEqual('2,352.5', 'with correct hover labels'); + + }).then(done); + }); +});