From 05e3687e5f1aaa0e02823fd4379958b4f3bb19a9 Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Mon, 16 Jan 2017 10:05:59 -0500 Subject: [PATCH 01/10] Cache the last mousemove event --- src/plots/cartesian/graph_interact.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/plots/cartesian/graph_interact.js b/src/plots/cartesian/graph_interact.js index 909b10d6411..6c561517fb9 100644 --- a/src/plots/cartesian/graph_interact.js +++ b/src/plots/cartesian/graph_interact.js @@ -114,7 +114,9 @@ fx.init = function(gd) { var maindrag = dragBox(gd, plotinfo, 0, 0, xa._length, ya._length, 'ns', 'ew'); + gd._lastMoveEvt; maindrag.onmousemove = function(evt) { + gd._lastMoveEvt = evt; fx.hover(gd, evt, subplot); fullLayout._lasthover = maindrag; fullLayout._hoversubplot = subplot; @@ -130,10 +132,13 @@ fx.init = function(gd) { maindrag.onmouseout = function(evt) { if(gd._dragging) return; + gd._lastMoveEvt = null; + dragElement.unhover(gd, evt); }; maindrag.onclick = function(evt) { + if(gd._lastMoveEvt) fx.hover(gd, gd._lastMoveEvt, subplot); fx.click(gd, evt); }; From 5881ad5279319abaaa70fe780fe92dae2fa18006 Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Mon, 30 Jan 2017 10:49:51 -0500 Subject: [PATCH 02/10] Rehover --- src/plots/cartesian/graph_interact.js | 12 ++++++------ src/plots/plots.js | 2 ++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/plots/cartesian/graph_interact.js b/src/plots/cartesian/graph_interact.js index 6c561517fb9..596d2b60ac9 100644 --- a/src/plots/cartesian/graph_interact.js +++ b/src/plots/cartesian/graph_interact.js @@ -114,10 +114,13 @@ fx.init = function(gd) { var maindrag = dragBox(gd, plotinfo, 0, 0, xa._length, ya._length, 'ns', 'ew'); - gd._lastMoveEvt; maindrag.onmousemove = function(evt) { - gd._lastMoveEvt = evt; - fx.hover(gd, evt, subplot); + // This is on `gd._fullLayout`, *not* fullLayout because the reference + // changes by the time this is called again. + gd._fullLayout._rehover = function() { + fx.hover(gd, evt, subplot); + }; + gd._fullLayout._rehover(); fullLayout._lasthover = maindrag; fullLayout._hoversubplot = subplot; }; @@ -132,13 +135,10 @@ fx.init = function(gd) { maindrag.onmouseout = function(evt) { if(gd._dragging) return; - gd._lastMoveEvt = null; - dragElement.unhover(gd, evt); }; maindrag.onclick = function(evt) { - if(gd._lastMoveEvt) fx.hover(gd, gd._lastMoveEvt, subplot); fx.click(gd, evt); }; diff --git a/src/plots/plots.js b/src/plots/plots.js index a1d853d40ef..4e7012f5edb 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -2026,4 +2026,6 @@ plots.doCalcdata = function(gd, traces) { calcdata[i] = cd; } + + if(gd._fullLayout._rehover) gd._fullLayout._rehover(); }; From 18b36a6482068578a47b404bd2ec7a3f3a5c3420 Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Mon, 13 Feb 2017 12:40:55 -0500 Subject: [PATCH 03/10] Manually trigger rehover behavior --- src/plot_api/plot_api.js | 38 ++++++++++++++++++++------- src/plots/cartesian/graph_interact.js | 17 +++++++++++- src/plots/plots.js | 12 +++++++-- 3 files changed, 54 insertions(+), 13 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 0e26c4b8aa9..f5a2b330bb0 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -343,7 +343,7 @@ Plotly.plot = function(gd, data, layout, config) { gd.emit('plotly_afterplot'); } - Lib.syncOrAsync([ + var seq = [ Plots.previousPromises, addFrames, drawFramework, @@ -353,8 +353,14 @@ Plotly.plot = function(gd, data, layout, config) { subroutines.layoutStyles, drawAxes, drawData, - finalDraw - ], gd, cleanUp); + finalDraw, + ]; + + if(gd._fullLayout._rehover) { + seq.push(function() { Plots.rehover(gd); }); + } + + Lib.syncOrAsync(seq, gd, cleanUp); // even if everything we did was synchronous, return a promise // so that the caller doesn't care which route we took @@ -1206,8 +1212,7 @@ Plotly.restyle = function restyle(gd, astr, val, traces) { if(flags.fullReplot) { seq.push(Plotly.plot); - } - else { + } else { seq.push(Plots.previousPromises); Plots.supplyDefaults(gd); @@ -1216,6 +1221,10 @@ Plotly.restyle = function restyle(gd, astr, val, traces) { if(flags.docolorbars) seq.push(subroutines.doColorBars); } + if(gd._fullLayout._rehover) { + seq.push(function() { Plots.rehover(gd); }); + } + Queue.add(gd, restyle, [gd, specs.undoit, specs.traces], restyle, [gd, specs.redoit, specs.traces] @@ -1696,9 +1705,11 @@ Plotly.relayout = function relayout(gd, astr, val) { } var aobj = {}; - if(typeof astr === 'string') aobj[astr] = val; - else if(Lib.isPlainObject(astr)) aobj = astr; - else { + if(typeof astr === 'string') { + aobj[astr] = val; + } else if(Lib.isPlainObject(astr)) { + aobj = astr; + } else { Lib.warn('Relayout fail.', astr, val); return Promise.reject(); } @@ -1716,8 +1727,7 @@ Plotly.relayout = function relayout(gd, astr, val) { if(flags.layoutReplot) { seq.push(subroutines.layoutReplot); - } - else if(Object.keys(aobj).length) { + } else if(Object.keys(aobj).length) { seq.push(Plots.previousPromises); Plots.supplyDefaults(gd); @@ -1727,6 +1737,10 @@ Plotly.relayout = function relayout(gd, astr, val) { if(flags.domodebar) seq.push(subroutines.doModeBar); } + if(gd._fullLayout._rehover) { + seq.push(function() { Plots.rehover(gd); }); + } + Queue.add(gd, relayout, [gd, specs.undoit], relayout, [gd, specs.redoit] @@ -2122,6 +2136,10 @@ Plotly.update = function update(gd, traceUpdate, layoutUpdate, traces) { if(relayoutFlags.domodebar) seq.push(subroutines.doModeBar); } + if(gd._fullLayout._rehover) { + seq.push(function() { Plots.rehover(gd); }); + } + Queue.add(gd, update, [gd, restyleSpecs.undoit, relayoutSpecs.undoit, restyleSpecs.traces], update, [gd, restyleSpecs.redoit, relayoutSpecs.redoit, restyleSpecs.traces] diff --git a/src/plots/cartesian/graph_interact.js b/src/plots/cartesian/graph_interact.js index 596d2b60ac9..c63b84dce61 100644 --- a/src/plots/cartesian/graph_interact.js +++ b/src/plots/cartesian/graph_interact.js @@ -118,9 +118,19 @@ fx.init = function(gd) { // This is on `gd._fullLayout`, *not* fullLayout because the reference // changes by the time this is called again. gd._fullLayout._rehover = function() { - fx.hover(gd, evt, subplot); + if(gd._fullLayout._hoversubplot === plotinfo.id) { + fx.hover(gd, evt, subplot); + } }; + + // Track the hovered subplot. This prevents rehover from accidetally + // reapplying a hover label after the mouse has left the plot or if + // the mouse has entered another subplot. + gd._fullLayout._hoversubplot = plotinfo.id; + gd._fullLayout._rehover(); + + fx.hover(gd, evt, subplot); fullLayout._lasthover = maindrag; fullLayout._hoversubplot = subplot; }; @@ -135,6 +145,11 @@ fx.init = function(gd) { maindrag.onmouseout = function(evt) { if(gd._dragging) return; + // When the mouse leaves this maindrag, unset the hovered subplot. + // This may cause problems if it leaves the subplot directly *onto* + // another subplot, but that's a tiny corner case at the moment. + gd._fullLayout._hoversubplot = null; + dragElement.unhover(gd, evt); }; diff --git a/src/plots/plots.js b/src/plots/plots.js index 4e7012f5edb..e8adbaf0e28 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1905,7 +1905,11 @@ plots.transition = function(gd, data, layout, traces, frameOpts, transitionOpts) } } - var seq = [plots.previousPromises, interruptPreviousTransitions, prepareTransitions, executeTransitions]; + function rehover() { + plots.rehover(gd); + } + + var seq = [plots.previousPromises, interruptPreviousTransitions, prepareTransitions, rehover, executeTransitions]; var transitionStarting = Lib.syncOrAsync(seq, gd); @@ -2026,6 +2030,10 @@ plots.doCalcdata = function(gd, traces) { calcdata[i] = cd; } +}; - if(gd._fullLayout._rehover) gd._fullLayout._rehover(); +plots.rehover = function(gd) { + if(gd._fullLayout._rehover) { + gd._fullLayout._rehover(); + } }; From 7df021eb88f09b6128d3a823fdab096c3abc3679 Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Mon, 13 Feb 2017 12:46:59 -0500 Subject: [PATCH 04/10] Fix bad merge --- src/plot_api/plot_api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index a1051d3aa8b..c37a55407ce 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -354,7 +354,7 @@ Plotly.plot = function(gd, data, layout, config) { seq.push(function() { Plots.rehover(gd); }); } - Lib.syncOrAsync(seq, gd, cleanUp); + Lib.syncOrAsync(seq, gd); // even if everything we did was synchronous, return a promise // so that the caller doesn't care which route we took From bfaf905ca43bdecea8b63ec24d14bc19166146d4 Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Mon, 13 Feb 2017 14:14:03 -0500 Subject: [PATCH 05/10] Simplify rehover calling slightly --- src/plot_api/plot_api.js | 17 ++++------------- src/plots/cartesian/graph_interact.js | 1 - src/plots/plots.js | 6 +----- 3 files changed, 5 insertions(+), 19 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index c37a55407ce..55c98d78910 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -348,12 +348,9 @@ Plotly.plot = function(gd, data, layout, config) { drawAxes, drawData, finalDraw, + Plots.rehover ]; - if(gd._fullLayout._rehover) { - seq.push(function() { Plots.rehover(gd); }); - } - Lib.syncOrAsync(seq, gd); // even if everything we did was synchronous, return a promise @@ -1216,9 +1213,7 @@ Plotly.restyle = function restyle(gd, astr, val, traces) { if(flags.docolorbars) seq.push(subroutines.doColorBars); } - if(gd._fullLayout._rehover) { - seq.push(function() { Plots.rehover(gd); }); - } + seq.push(Plots.rehover); Queue.add(gd, restyle, [gd, specs.undoit, specs.traces], @@ -1737,9 +1732,7 @@ Plotly.relayout = function relayout(gd, astr, val) { if(flags.docamera) seq.push(subroutines.doCamera); } - if(gd._fullLayout._rehover) { - seq.push(function() { Plots.rehover(gd); }); - } + seq.push(Plots.rehover); Queue.add(gd, relayout, [gd, specs.undoit], @@ -2141,9 +2134,7 @@ Plotly.update = function update(gd, traceUpdate, layoutUpdate, traces) { if(relayoutFlags.doCamera) seq.push(subroutines.doCamera); } - if(gd._fullLayout._rehover) { - seq.push(function() { Plots.rehover(gd); }); - } + seq.push(Plots.rehover); Queue.add(gd, update, [gd, restyleSpecs.undoit, relayoutSpecs.undoit, restyleSpecs.traces], diff --git a/src/plots/cartesian/graph_interact.js b/src/plots/cartesian/graph_interact.js index 7c771ef423a..904484fdc05 100644 --- a/src/plots/cartesian/graph_interact.js +++ b/src/plots/cartesian/graph_interact.js @@ -129,7 +129,6 @@ fx.init = function(gd) { gd._fullLayout._rehover(); - fx.hover(gd, evt, subplot); fullLayout._lasthover = maindrag; fullLayout._hoversubplot = subplot; }; diff --git a/src/plots/plots.js b/src/plots/plots.js index 7fc7917107a..3b2e3301586 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1934,11 +1934,7 @@ plots.transition = function(gd, data, layout, traces, frameOpts, transitionOpts) } } - function rehover() { - plots.rehover(gd); - } - - var seq = [plots.previousPromises, interruptPreviousTransitions, prepareTransitions, rehover, executeTransitions]; + var seq = [plots.previousPromises, interruptPreviousTransitions, prepareTransitions, plots.rehover, executeTransitions]; var transitionStarting = Lib.syncOrAsync(seq, gd); From 77aa54c70c23393d612fce9222acfa64ea0bca9f Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Mon, 13 Feb 2017 14:25:36 -0500 Subject: [PATCH 06/10] Fix mousemove actions --- src/plots/cartesian/graph_interact.js | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/plots/cartesian/graph_interact.js b/src/plots/cartesian/graph_interact.js index 904484fdc05..27c655a7e99 100644 --- a/src/plots/cartesian/graph_interact.js +++ b/src/plots/cartesian/graph_interact.js @@ -117,20 +117,17 @@ fx.init = function(gd) { // This is on `gd._fullLayout`, *not* fullLayout because the reference // changes by the time this is called again. gd._fullLayout._rehover = function() { - if(gd._fullLayout._hoversubplot === plotinfo.id) { + if(gd._fullLayout._hoversubplot === subplot) { fx.hover(gd, evt, subplot); } }; - // Track the hovered subplot. This prevents rehover from accidetally - // reapplying a hover label after the mouse has left the plot or if - // the mouse has entered another subplot. - gd._fullLayout._hoversubplot = plotinfo.id; + fx.hover(gd, evt, subplot); - gd._fullLayout._rehover(); - - fullLayout._lasthover = maindrag; - fullLayout._hoversubplot = subplot; + // Not that we have *not* used the cached fullLayout variable here + // since that may be outdated when this is called as a callback later on + gd._fullLayout._lasthover = maindrag; + gd._fullLayout._hoversubplot = subplot; }; /* From 9ad97e4cfaf27829c12595f25733b3ae3d8232f4 Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Mon, 13 Feb 2017 14:27:12 -0500 Subject: [PATCH 07/10] Fix small typo in comment --- src/plots/cartesian/graph_interact.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plots/cartesian/graph_interact.js b/src/plots/cartesian/graph_interact.js index 27c655a7e99..835873e5e39 100644 --- a/src/plots/cartesian/graph_interact.js +++ b/src/plots/cartesian/graph_interact.js @@ -124,7 +124,7 @@ fx.init = function(gd) { fx.hover(gd, evt, subplot); - // Not that we have *not* used the cached fullLayout variable here + // Note that we have *not* used the cached fullLayout variable here // since that may be outdated when this is called as a callback later on gd._fullLayout._lasthover = maindrag; gd._fullLayout._hoversubplot = subplot; From bfa55004566f1955d45343346145ee9b3da166dd Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Tue, 14 Feb 2017 13:12:38 -0500 Subject: [PATCH 08/10] Test label updates --- test/jasmine/tests/hover_label_test.js | 60 ++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index 2e76a3ab0e5..1f1ff8d11ff 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -10,6 +10,7 @@ var destroyGraphDiv = require('../assets/destroy_graph_div'); var mouseEvent = require('../assets/mouse_event'); var click = require('../assets/click'); var doubleClick = require('../assets/double_click'); +var fail = require('../assets/fail_test'); describe('hover info', function() { 'use strict'; @@ -811,3 +812,62 @@ describe('hover on fill', function() { }).then(done); }); }); + +describe('hover updates', function() { + 'use strict'; + + afterEach(destroyGraphDiv); + + function assertLabelsCorrect(mousePos, labelPos, labelText) { + return new Promise(function(resolve) { + if (mousePos) { + mouseEvent('mousemove', mousePos[0], mousePos[1]); + } + + setTimeout(function() { + var hoverText = d3.selectAll('g.hovertext'); + expect(hoverText.size()).toEqual(1); + expect(hoverText.text()).toEqual(labelText); + + var transformParts = hoverText.attr('transform').split('('); + expect(transformParts[0]).toEqual('translate'); + var transformCoords = transformParts[1].split(')')[0].split(','); + expect(+transformCoords[0]).toBeCloseTo(labelPos[0], -1, labelText + ':x'); + expect(+transformCoords[1]).toBeCloseTo(labelPos[1], -1, labelText + ':y'); + + resolve(); + }, constants.HOVERMINTIME); + }); + } + + it('should update the labels on animation', function(done) { + var mock = { + data: [ + {x: [0.5], y: [0.5], showlegend: false}, + {x: [0], y: [0], showlegend: false}, + ], + layout: { + margin: {t: 0, r: 0, b: 0, l: 0}, + width: 200, + height: 200, + xaxis: {range: [0, 1]}, + yaxis: {range: [0, 1]}, + } + }; + + var gd = createGraphDiv(); + Plotly.plot(gd, mock).then(function() { + // The label text gets concatenated together when queried. Such is life. + return assertLabelsCorrect([100, 100], [103, 100], 'trace 00.5'); + }).then(function () { + return Plotly.animate(gd, [{ + data: [{x: [0], y: [0]}, {x: [0.5], y: [0.5]}], + traces: [0, 1], + }], {frame: {redraw: false, duration: 0}}); + }).then(function () { + // No mouse event this time. Just change the data and check the label. + // Ditto on concatenation. This is "trace 1" + "0.5" + return assertLabelsCorrect(null, [103, 100], 'trace 10.5'); + }).catch(fail).then(done); + }); +}); From 6711827fcd95dc0637543ce41e963f2d9afff2cb Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Tue, 14 Feb 2017 13:13:59 -0500 Subject: [PATCH 09/10] Lint obv --- test/jasmine/tests/hover_label_test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index 1f1ff8d11ff..0d66cd5b006 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -820,7 +820,7 @@ describe('hover updates', function() { function assertLabelsCorrect(mousePos, labelPos, labelText) { return new Promise(function(resolve) { - if (mousePos) { + if(mousePos) { mouseEvent('mousemove', mousePos[0], mousePos[1]); } @@ -859,12 +859,12 @@ describe('hover updates', function() { Plotly.plot(gd, mock).then(function() { // The label text gets concatenated together when queried. Such is life. return assertLabelsCorrect([100, 100], [103, 100], 'trace 00.5'); - }).then(function () { + }).then(function() { return Plotly.animate(gd, [{ data: [{x: [0], y: [0]}, {x: [0.5], y: [0.5]}], traces: [0, 1], }], {frame: {redraw: false, duration: 0}}); - }).then(function () { + }).then(function() { // No mouse event this time. Just change the data and check the label. // Ditto on concatenation. This is "trace 1" + "0.5" return assertLabelsCorrect(null, [103, 100], 'trace 10.5'); From 554d7ed5efaf1376a610ef61884ddd30bc145fd5 Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Wed, 15 Feb 2017 14:07:00 -0500 Subject: [PATCH 10/10] Expand hover label test to verify is hidden/redisplayed on relayout --- test/jasmine/tests/hover_label_test.js | 32 +++++++++++++++++++------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index 0d66cd5b006..3561e6d6082 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -826,14 +826,18 @@ describe('hover updates', function() { setTimeout(function() { var hoverText = d3.selectAll('g.hovertext'); - expect(hoverText.size()).toEqual(1); - expect(hoverText.text()).toEqual(labelText); - - var transformParts = hoverText.attr('transform').split('('); - expect(transformParts[0]).toEqual('translate'); - var transformCoords = transformParts[1].split(')')[0].split(','); - expect(+transformCoords[0]).toBeCloseTo(labelPos[0], -1, labelText + ':x'); - expect(+transformCoords[1]).toBeCloseTo(labelPos[1], -1, labelText + ':y'); + if(labelPos) { + expect(hoverText.size()).toEqual(1); + expect(hoverText.text()).toEqual(labelText); + + var transformParts = hoverText.attr('transform').split('('); + expect(transformParts[0]).toEqual('translate'); + var transformCoords = transformParts[1].split(')')[0].split(','); + expect(+transformCoords[0]).toBeCloseTo(labelPos[0], -1, labelText + ':x'); + expect(+transformCoords[1]).toBeCloseTo(labelPos[1], -1, labelText + ':y'); + } else { + expect(hoverText.size()).toEqual(0); + } resolve(); }, constants.HOVERMINTIME); @@ -868,6 +872,18 @@ describe('hover updates', function() { // No mouse event this time. Just change the data and check the label. // Ditto on concatenation. This is "trace 1" + "0.5" return assertLabelsCorrect(null, [103, 100], 'trace 10.5'); + }).then(function() { + // Restyle to move the point out of the window: + return Plotly.relayout(gd, {'xaxis.range': [2, 3]}); + }).then(function() { + // Assert label removed: + return assertLabelsCorrect(null, null); + }).then(function() { + // Move back to the original xaxis range: + return Plotly.relayout(gd, {'xaxis.range': [0, 1]}); + }).then(function() { + // Assert label restored: + return assertLabelsCorrect(null, [103, 100], 'trace 10.5'); }).catch(fail).then(done); }); });