From 2c76d79c18b564bd0383a304c416cc527c3c0cac Mon Sep 17 00:00:00 2001 From: archmoj Date: Sat, 21 Mar 2020 18:04:31 -0400 Subject: [PATCH 1/7] enable unified hovermode via template --- src/plots/cartesian/layout_defaults.js | 5 +++-- test/jasmine/tests/hover_label_test.js | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/plots/cartesian/layout_defaults.js b/src/plots/cartesian/layout_defaults.js index 9e163329c7b..fb34f40e3ab 100644 --- a/src/plots/cartesian/layout_defaults.js +++ b/src/plots/cartesian/layout_defaults.js @@ -249,8 +249,9 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { handleTypeDefaults(axLayoutIn, axLayoutOut, coerce, defaultOptions); handleAxisDefaults(axLayoutIn, axLayoutOut, coerce, defaultOptions, layoutOut); - var unifiedHover = layoutIn.hovermode && ['x unified', 'y unified'].indexOf(layoutIn.hovermode) !== -1; - var unifiedSpike = unifiedHover && axLetter === layoutIn.hovermode.charAt(0); + var hovermode = layoutOut.hovermode || layoutIn.hovermode; + var unifiedHover = hovermode && ['x unified', 'y unified'].indexOf(hovermode) !== -1; + var unifiedSpike = unifiedHover && axLetter === hovermode.charAt(0); var spikecolor = coerce2('spikecolor', unifiedHover ? axLayoutOut.color : undefined); var spikethickness = coerce2('spikethickness', unifiedHover ? 1.5 : undefined); var spikedash = coerce2('spikedash', unifiedHover ? 'dot' : undefined); diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index 0f5e7f17cf2..8c58c670bc9 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -4093,6 +4093,24 @@ describe('hovermode: (x|y)unified', function() { .then(done); }); + it('unified hover modes should work for x/y cartesian traces via template', function(done) { + var mockCopy = Lib.extendDeep({}, mock); + delete mockCopy.layout.hovermode; + mockCopy.layout.template = { + layout: { + hovermode: 'y unified' + } + }; + Plotly.newPlot(gd, mockCopy) + .then(function(gd) { + _hover(gd, { yval: 6 }); + + assertLabel({title: '6', items: ['trace 0 : 2', 'trace 1 : 5']}); + }) + .catch(failTest) + .then(done); + }); + it('x unified should work for x/y cartesian traces with legendgroup', function(done) { var mockLegendGroup = require('@mocks/legendgroup.json'); var mockCopy = Lib.extendDeep({}, mockLegendGroup); From 3c883ff95361465c03d821da7b88f4f705407d82 Mon Sep 17 00:00:00 2001 From: archmoj Date: Sat, 21 Mar 2020 18:48:02 -0400 Subject: [PATCH 2/7] refactor check for unified hover modes --- src/components/fx/helpers.js | 28 +++++++++++++++++--------- src/components/fx/hover.js | 6 +++--- src/components/fx/layout_defaults.js | 7 ++----- src/components/modebar/manage.js | 3 ++- src/plots/cartesian/layout_defaults.js | 3 ++- 5 files changed, 28 insertions(+), 19 deletions(-) diff --git a/src/components/fx/helpers.js b/src/components/fx/helpers.js index ce1011edb2f..a74de37a277 100644 --- a/src/components/fx/helpers.js +++ b/src/components/fx/helpers.js @@ -12,13 +12,13 @@ var Lib = require('../../lib'); // look for either subplot or xaxis and yaxis attributes // does not handle splom case -exports.getSubplot = function getSubplot(trace) { +exports.getSubplot = function(trace) { return trace.subplot || (trace.xaxis + trace.yaxis) || trace.geo; }; // is trace in given list of subplots? // does handle splom case -exports.isTraceInSubplots = function isTraceInSubplots(trace, subplots) { +exports.isTraceInSubplots = function(trace, subplots) { if(trace.type === 'splom') { var xaxes = trace.xaxes || []; var yaxes = trace.yaxes || []; @@ -36,7 +36,7 @@ exports.isTraceInSubplots = function isTraceInSubplots(trace, subplots) { }; // convenience functions for mapping all relevant axes -exports.flat = function flat(subplots, v) { +exports.flat = function(subplots, v) { var out = new Array(subplots.length); for(var i = 0; i < subplots.length; i++) { out[i] = v; @@ -44,7 +44,7 @@ exports.flat = function flat(subplots, v) { return out; }; -exports.p2c = function p2c(axArray, v) { +exports.p2c = function(axArray, v) { var out = new Array(axArray.length); for(var i = 0; i < axArray.length; i++) { out[i] = axArray[i].p2c(v); @@ -52,12 +52,12 @@ exports.p2c = function p2c(axArray, v) { return out; }; -exports.getDistanceFunction = function getDistanceFunction(mode, dx, dy, dxy) { +exports.getDistanceFunction = function(mode, dx, dy, dxy) { if(mode === 'closest') return dxy || exports.quadrature(dx, dy); return mode.charAt(0) === 'x' ? dx : dy; }; -exports.getClosest = function getClosest(cd, distfn, pointData) { +exports.getClosest = function(cd, distfn, pointData) { // do we already have a point number? (array mode only) if(pointData.index !== false) { if(pointData.index >= 0 && pointData.index < cd.length) { @@ -87,11 +87,11 @@ exports.getClosest = function getClosest(cd, distfn, pointData) { * @param {number} v1: signed difference between the current position and the right edge * @param {number} passVal: the value to return on success */ -exports.inbox = function inbox(v0, v1, passVal) { +exports.inbox = function(v0, v1, passVal) { return (v0 * v1 < 0 || v0 === 0) ? passVal : Infinity; }; -exports.quadrature = function quadrature(dx, dy) { +exports.quadrature = function(dx, dy) { return function(di) { var x = dx(di); var y = dy(di); @@ -114,7 +114,7 @@ exports.quadrature = function quadrature(dx, dy) { * @param {object} cd * @return {object} */ -exports.makeEventData = function makeEventData(pt, trace, cd) { +exports.makeEventData = function(pt, trace, cd) { // hover uses 'index', select uses 'pointNumber' var pointNumber = 'index' in pt ? pt.index : pt.pointNumber; @@ -238,3 +238,13 @@ function getPointData(val, pointNumber) { return val[pointNumber]; } } + +var unifiedHoverMode = { + 'x unified': true, + 'y unified': true +}; + +exports.isUnifiedHover = function(hovermode) { + if(typeof hovermode !== 'string') return false; + return !!unifiedHoverMode[hovermode]; +}; diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 6556c22d8f7..780a884a41e 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -394,7 +394,7 @@ function _hover(gd, evt, subplot, noHoverEvent) { // within one trace mode can sometimes be overridden mode = hovermode; - if(['x unified', 'y unified'].indexOf(mode) !== -1) { + if(helpers.isUnifiedHover(mode)) { mode = mode.charAt(0); } @@ -715,7 +715,7 @@ function _hover(gd, evt, subplot, noHoverEvent) { var hoverLabels = createHoverText(hoverData, labelOpts, gd); - if(['x unified', 'y unified'].indexOf(hovermode) === -1) { + if(!helpers.isUnifiedHover(hovermode)) { hoverAvoidOverlaps(hoverLabels, rotateLabels ? 'xa' : 'ya', fullLayout); alignHoverText(hoverLabels, rotateLabels); } @@ -976,7 +976,7 @@ function createHoverText(hoverData, opts, gd) { } // Show a single hover label - if(['x unified', 'y unified'].indexOf(hovermode) !== -1) { + if(helpers.isUnifiedHover(hovermode)) { // Delete leftover hover labels from other hovermodes container.selectAll('g.hovertext').remove(); diff --git a/src/components/fx/layout_defaults.js b/src/components/fx/layout_defaults.js index 70931e3e730..8b2616e63ce 100644 --- a/src/components/fx/layout_defaults.js +++ b/src/components/fx/layout_defaults.js @@ -9,6 +9,7 @@ 'use strict'; var Lib = require('../../lib'); +var isUnifiedHover = require('./helpers').isUnifiedHover; var layoutAttributes = require('./layout_attributes'); module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { @@ -35,12 +36,8 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { var hoverMode = coerce('hovermode', hovermodeDflt); if(hoverMode) { - var dflt; - if(['x unified', 'y unified'].indexOf(hoverMode) !== -1) { - dflt = -1; - } coerce('hoverdistance'); - coerce('spikedistance', dflt); + coerce('spikedistance', isUnifiedHover(hoverMode) ? -1 : undefined); } // if only mapbox or geo subplots is present on graph, diff --git a/src/components/modebar/manage.js b/src/components/modebar/manage.js index 884a3c2c3ad..267f2117392 100644 --- a/src/components/modebar/manage.js +++ b/src/components/modebar/manage.js @@ -12,6 +12,7 @@ var axisIds = require('../../plots/cartesian/axis_ids'); var scatterSubTypes = require('../../traces/scatter/subtypes'); var Registry = require('../../registry'); +var isUnifiedHover = require('../fx/helpers').isUnifiedHover; var createModeBar = require('./modebar'); var modeBarButtons = require('./buttons'); @@ -85,7 +86,7 @@ function getButtonGroups(gd) { var hasPolar = fullLayout._has('polar'); var hasSankey = fullLayout._has('sankey'); var allAxesFixed = areAllAxesFixed(fullLayout); - var hasUnifiedHoverLabel = ['x unified', 'y unified'].indexOf(fullLayout.hovermode) !== -1; + var hasUnifiedHoverLabel = isUnifiedHover(fullLayout.hovermode); var groups = []; diff --git a/src/plots/cartesian/layout_defaults.js b/src/plots/cartesian/layout_defaults.js index fb34f40e3ab..ed43f14dae7 100644 --- a/src/plots/cartesian/layout_defaults.js +++ b/src/plots/cartesian/layout_defaults.js @@ -11,6 +11,7 @@ var Lib = require('../../lib'); var Color = require('../../components/color'); +var isUnifiedHover = require('../../components/fx/helpers').isUnifiedHover; var Template = require('../../plot_api/plot_template'); var basePlotLayoutAttributes = require('../layout_attributes'); @@ -250,7 +251,7 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { handleAxisDefaults(axLayoutIn, axLayoutOut, coerce, defaultOptions, layoutOut); var hovermode = layoutOut.hovermode || layoutIn.hovermode; - var unifiedHover = hovermode && ['x unified', 'y unified'].indexOf(hovermode) !== -1; + var unifiedHover = isUnifiedHover(hovermode); var unifiedSpike = unifiedHover && axLetter === hovermode.charAt(0); var spikecolor = coerce2('spikecolor', unifiedHover ? axLayoutOut.color : undefined); var spikethickness = coerce2('spikethickness', unifiedHover ? 1.5 : undefined); From 9c07a91346e8fd19dbd3ef3b79132eca364ac910 Mon Sep 17 00:00:00 2001 From: archmoj Date: Sat, 21 Mar 2020 19:00:28 -0400 Subject: [PATCH 3/7] make a similar function in helpers to test x or y hover modes --- src/components/fx/helpers.js | 10 ++++++++++ src/components/fx/hover.js | 9 ++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/components/fx/helpers.js b/src/components/fx/helpers.js index a74de37a277..28d67cf857a 100644 --- a/src/components/fx/helpers.js +++ b/src/components/fx/helpers.js @@ -239,6 +239,11 @@ function getPointData(val, pointNumber) { } } +var xyHoverMode = { + x: true, + y: true +}; + var unifiedHoverMode = { 'x unified': true, 'y unified': true @@ -248,3 +253,8 @@ exports.isUnifiedHover = function(hovermode) { if(typeof hovermode !== 'string') return false; return !!unifiedHoverMode[hovermode]; }; + +exports.isXYhover = function(hovermode) { + if(typeof hovermode !== 'string') return false; + return !!xyHoverMode[hovermode]; +}; diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 780a884a41e..c30114408d4 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -45,8 +45,6 @@ var YSHIFTY = Math.sin(YA_RADIANS); var HOVERARROWSIZE = constants.HOVERARROWSIZE; var HOVERTEXTPAD = constants.HOVERTEXTPAD; -var XY = {x: 1, y: 1}; - // fx.hover: highlight data on hover // evt can be a mousemove event, or an object with data about what points // to hover on @@ -631,9 +629,10 @@ function _hover(gd, evt, subplot, noHoverEvent) { hoverData.sort(function(d1, d2) { return d1.distance - d2.distance; }); // If in compare mode, select every point at position - if(hoverData[0].length !== 0 && - XY[mode] && - hoverData[0].trace.type !== 'splom' // TODO: add support for splom + if( + helpers.isXYhover(mode) && + hoverData[0].length !== 0 && + hoverData[0].trace.type !== 'splom' // TODO: add support for splom ) { var hd = hoverData[0]; var cd0 = hd.cd[hd.index]; From 11a4e2758889bc897ff8b37eead496239b821e44 Mon Sep 17 00:00:00 2001 From: archmoj Date: Tue, 24 Mar 2020 13:22:43 -0400 Subject: [PATCH 4/7] make a separate function to coerce clickmode and hovermode --- src/components/fx/hovermode_defaults.js | 54 +++++++++++++++++++++++++ src/components/fx/layout_defaults.js | 40 +++--------------- 2 files changed, 59 insertions(+), 35 deletions(-) create mode 100644 src/components/fx/hovermode_defaults.js diff --git a/src/components/fx/hovermode_defaults.js b/src/components/fx/hovermode_defaults.js new file mode 100644 index 00000000000..42ffe7fde0f --- /dev/null +++ b/src/components/fx/hovermode_defaults.js @@ -0,0 +1,54 @@ +/** +* Copyright 2012-2020, Plotly, Inc. +* All rights reserved. +* +* This source code is licensed under the MIT license found in the +* LICENSE file in the root directory of this source tree. +*/ + +'use strict'; + +var Lib = require('../../lib'); +var layoutAttributes = require('./layout_attributes'); + +module.exports = function handleHoverModeDefaults(layoutIn, layoutOut, fullData) { + function coerce(attr, dflt) { + // don't coerce if it is already coerced in other place e.g. in cartesian defaults + if(layoutOut[attr] !== undefined) return layoutOut[attr]; + + return Lib.coerce(layoutIn, layoutOut, layoutAttributes, attr, dflt); + } + + var clickmode = coerce('clickmode'); + + var hovermodeDflt; + if(layoutOut._has('cartesian')) { + if(clickmode.indexOf('select') > -1) { + hovermodeDflt = 'closest'; + } else { + // flag for 'horizontal' plots: + // determines the state of the mode bar 'compare' hovermode button + layoutOut._isHoriz = isHoriz(fullData, layoutOut); + hovermodeDflt = layoutOut._isHoriz ? 'y' : 'x'; + } + } else hovermodeDflt = 'closest'; + + return coerce('hovermode', hovermodeDflt); +}; + +function isHoriz(fullData, fullLayout) { + var stackOpts = fullLayout._scatterStackOpts || {}; + + for(var i = 0; i < fullData.length; i++) { + var trace = fullData[i]; + var subplot = trace.xaxis + trace.yaxis; + var subplotStackOpts = stackOpts[subplot] || {}; + var groupOpts = subplotStackOpts[trace.stackgroup] || {}; + + if(trace.orientation !== 'h' && groupOpts.orientation !== 'h') { + return false; + } + } + + return true; +} diff --git a/src/components/fx/layout_defaults.js b/src/components/fx/layout_defaults.js index 8b2616e63ce..80755253736 100644 --- a/src/components/fx/layout_defaults.js +++ b/src/components/fx/layout_defaults.js @@ -11,35 +11,22 @@ var Lib = require('../../lib'); var isUnifiedHover = require('./helpers').isUnifiedHover; var layoutAttributes = require('./layout_attributes'); +var handleHoverModeDefaults = require('./hovermode_defaults'); module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { function coerce(attr, dflt) { return Lib.coerce(layoutIn, layoutOut, layoutAttributes, attr, dflt); } - var clickmode = coerce('clickmode'); - - var dragMode = coerce('dragmode'); - if(dragMode === 'select') coerce('selectdirection'); - - var hovermodeDflt; - if(layoutOut._has('cartesian')) { - if(clickmode.indexOf('select') > -1) { - hovermodeDflt = 'closest'; - } else { - // flag for 'horizontal' plots: - // determines the state of the mode bar 'compare' hovermode button - layoutOut._isHoriz = isHoriz(fullData, layoutOut); - hovermodeDflt = layoutOut._isHoriz ? 'y' : 'x'; - } - } else hovermodeDflt = 'closest'; - - var hoverMode = coerce('hovermode', hovermodeDflt); + var hoverMode = handleHoverModeDefaults(layoutIn, layoutOut, fullData); if(hoverMode) { coerce('hoverdistance'); coerce('spikedistance', isUnifiedHover(hoverMode) ? -1 : undefined); } + var dragMode = coerce('dragmode'); + if(dragMode === 'select') coerce('selectdirection'); + // if only mapbox or geo subplots is present on graph, // reset 'zoom' dragmode to 'pan' until 'zoom' is implemented, // so that the correct modebar button is active @@ -54,20 +41,3 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { layoutOut.dragmode = 'pan'; } }; - -function isHoriz(fullData, fullLayout) { - var stackOpts = fullLayout._scatterStackOpts || {}; - - for(var i = 0; i < fullData.length; i++) { - var trace = fullData[i]; - var subplot = trace.xaxis + trace.yaxis; - var subplotStackOpts = stackOpts[subplot] || {}; - var groupOpts = subplotStackOpts[trace.stackgroup] || {}; - - if(trace.orientation !== 'h' && groupOpts.orientation !== 'h') { - return false; - } - } - - return true; -} From 3c47bf9586b40062728db9fc928639e9bf5b16ff Mon Sep 17 00:00:00 2001 From: archmoj Date: Tue, 24 Mar 2020 13:45:17 -0400 Subject: [PATCH 5/7] use coerce hovermode in cartesian defaults --- src/plots/cartesian/layout_defaults.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plots/cartesian/layout_defaults.js b/src/plots/cartesian/layout_defaults.js index ed43f14dae7..7c8ced6436d 100644 --- a/src/plots/cartesian/layout_defaults.js +++ b/src/plots/cartesian/layout_defaults.js @@ -12,6 +12,7 @@ var Lib = require('../../lib'); var Color = require('../../components/color'); var isUnifiedHover = require('../../components/fx/helpers').isUnifiedHover; +var handleHoverModeDefaults = require('../../components/fx/hovermode_defaults'); var Template = require('../../plot_api/plot_template'); var basePlotLayoutAttributes = require('../layout_attributes'); @@ -250,7 +251,7 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { handleTypeDefaults(axLayoutIn, axLayoutOut, coerce, defaultOptions); handleAxisDefaults(axLayoutIn, axLayoutOut, coerce, defaultOptions, layoutOut); - var hovermode = layoutOut.hovermode || layoutIn.hovermode; + var hovermode = handleHoverModeDefaults(layoutIn, layoutOut, fullData); var unifiedHover = isUnifiedHover(hovermode); var unifiedSpike = unifiedHover && axLetter === hovermode.charAt(0); var spikecolor = coerce2('spikecolor', unifiedHover ? axLayoutOut.color : undefined); From dc704aef1405be7a4fb77193288c2dca872d65e5 Mon Sep 17 00:00:00 2001 From: archmoj Date: Tue, 24 Mar 2020 13:47:10 -0400 Subject: [PATCH 6/7] move hovermode coerce outside the loop --- src/plots/cartesian/layout_defaults.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/plots/cartesian/layout_defaults.js b/src/plots/cartesian/layout_defaults.js index 7c8ced6436d..3e411ae0574 100644 --- a/src/plots/cartesian/layout_defaults.js +++ b/src/plots/cartesian/layout_defaults.js @@ -207,6 +207,9 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { } } + var hovermode = handleHoverModeDefaults(layoutIn, layoutOut, fullData); + var unifiedHover = isUnifiedHover(hovermode); + // first pass creates the containers, determines types, and handles most of the settings for(i = 0; i < axNames.length; i++) { axName = axNames[i]; @@ -251,8 +254,6 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { handleTypeDefaults(axLayoutIn, axLayoutOut, coerce, defaultOptions); handleAxisDefaults(axLayoutIn, axLayoutOut, coerce, defaultOptions, layoutOut); - var hovermode = handleHoverModeDefaults(layoutIn, layoutOut, fullData); - var unifiedHover = isUnifiedHover(hovermode); var unifiedSpike = unifiedHover && axLetter === hovermode.charAt(0); var spikecolor = coerce2('spikecolor', unifiedHover ? axLayoutOut.color : undefined); var spikethickness = coerce2('spikethickness', unifiedHover ? 1.5 : undefined); From abf3a557080ae16b25ca0da57d88ba956a535b75 Mon Sep 17 00:00:00 2001 From: archmoj Date: Tue, 24 Mar 2020 13:55:38 -0400 Subject: [PATCH 7/7] improve test for spikelines --- test/jasmine/tests/hover_label_test.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index 8c58c670bc9..342301787d9 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -4103,6 +4103,16 @@ describe('hovermode: (x|y)unified', function() { }; Plotly.newPlot(gd, mockCopy) .then(function(gd) { + expect(gd._fullLayout.hovermode).toBe('y unified'); + var ax = gd._fullLayout.yaxis; + expect(ax.showspike).toBeTrue; + expect(ax.spikemode).toBe('across'); + expect(ax.spikethickness).toBe(1.5); + expect(ax.spikedash).toBe('dot'); + expect(ax.spikecolor).toBe('#444'); + expect(ax.spikesnap).toBe('hovered data'); + expect(gd._fullLayout.xaxis.showspike).toBeFalse; + _hover(gd, { yval: 6 }); assertLabel({title: '6', items: ['trace 0 : 2', 'trace 1 : 5']});