From 43e8d304bebafc16c5bef8566df311f640960824 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Mon, 20 Jun 2016 11:22:57 +0200 Subject: [PATCH 01/16] hoveron=fills initial commit --- src/traces/scatter/attributes.js | 11 +++ src/traces/scatter/defaults.js | 8 ++ src/traces/scatter/hover.js | 148 +++++++++++++++++++++---------- src/traces/scatter/plot.js | 14 ++- 4 files changed, 133 insertions(+), 48 deletions(-) diff --git a/src/traces/scatter/attributes.js b/src/traces/scatter/attributes.js index 68d7db06cca..4e2bba6b271 100644 --- a/src/traces/scatter/attributes.js +++ b/src/traces/scatter/attributes.js @@ -92,6 +92,17 @@ module.exports = { 'then the default is *lines+markers*. Otherwise, *lines*.' ].join(' ') }, + hoveron: { + valType: 'enumerated', + values: ['points', 'fills'], + role: 'info', + description: [ + 'Do the hover effects highlight individual points (markers or', + 'line points) or do they highlight filled regions?', + 'If the fill is *toself* or *tonext* and there are no markers', + 'or text, then the default is *fills*, otherwise it is *points*.' + ].join(' ') + }, line: { color: { valType: 'color', diff --git a/src/traces/scatter/defaults.js b/src/traces/scatter/defaults.js index 3690160053f..5e8f42931f5 100644 --- a/src/traces/scatter/defaults.js +++ b/src/traces/scatter/defaults.js @@ -53,8 +53,11 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout handleTextDefaults(traceIn, traceOut, layout, coerce); } + var dfltHoverOn = ''; + if(subTypes.hasMarkers(traceOut) || subTypes.hasText(traceOut)) { coerce('marker.maxdisplayed'); + dfltHoverOn = 'points'; } coerce('fill'); @@ -63,6 +66,11 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout if(!subTypes.hasLines(traceOut)) handleLineShapeDefaults(traceIn, traceOut, coerce); } + if(!dfltHoverOn && (traceOut.fill === 'tonext' || traceOut.fill === 'toself')) { + dfltHoverOn = 'fills'; + } + coerce('hoveron', dfltHoverOn || 'points'); + errorBarsSupplyDefaults(traceIn, traceOut, defaultColor, {axis: 'y'}); errorBarsSupplyDefaults(traceIn, traceOut, defaultColor, {axis: 'x', inherit: 'y'}); }; diff --git a/src/traces/scatter/hover.js b/src/traces/scatter/hover.js index 737a6509149..c73c657a311 100644 --- a/src/traces/scatter/hover.js +++ b/src/traces/scatter/hover.js @@ -9,9 +9,11 @@ 'use strict'; +var Lib = require('../../lib'); var Fx = require('../../plots/cartesian/graph_interact'); var ErrorBars = require('../../components/errorbars'); var getTraceColor = require('./get_trace_color'); +var Color = require('../../components/color'); module.exports = function hoverPoints(pointData, xval, yval, hovermode) { @@ -19,51 +21,103 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { trace = cd[0].trace, xa = pointData.xa, ya = pointData.ya, - dx = function(di) { - // scatter points: d.mrc is the calculated marker radius - // adjust the distance so if you're inside the marker it - // always will show up regardless of point size, but - // prioritize smaller points - var rad = Math.max(3, di.mrc || 0); - return Math.max(Math.abs(xa.c2p(di.x) - xa.c2p(xval)) - rad, 1 - 3 / rad); - }, - dy = function(di) { - var rad = Math.max(3, di.mrc || 0); - return Math.max(Math.abs(ya.c2p(di.y) - ya.c2p(yval)) - rad, 1 - 3 / rad); - }, - dxy = function(di) { - var rad = Math.max(3, di.mrc || 0), - dx = Math.abs(xa.c2p(di.x) - xa.c2p(xval)), - dy = Math.abs(ya.c2p(di.y) - ya.c2p(yval)); - return Math.max(Math.sqrt(dx * dx + dy * dy) - rad, 1 - 3 / rad); - }, - distfn = Fx.getDistanceFunction(hovermode, dx, dy, dxy); - - Fx.getClosest(cd, distfn, pointData); - - // skip the rest (for this trace) if we didn't find a close point - if(pointData.index === false) return; - - // the closest data point - var di = cd[pointData.index], - xc = xa.c2p(di.x, true), - yc = ya.c2p(di.y, true), - rad = di.mrc || 1; - - pointData.color = getTraceColor(trace, di); - - pointData.x0 = xc - rad; - pointData.x1 = xc + rad; - pointData.xLabelVal = di.x; - - pointData.y0 = yc - rad; - pointData.y1 = yc + rad; - pointData.yLabelVal = di.y; - - if(di.tx) pointData.text = di.tx; - else if(trace.text) pointData.text = trace.text; - - ErrorBars.hoverInfo(di, trace, pointData); - - return [pointData]; + xpx = xa.c2p(xval), + ypx = ya.c2p(yval), + pt = [xpx, ypx]; + + // even if hoveron is 'fills', only use it if we have polygons too + if(trace.hoveron === 'fills' && trace._polygons) { + var polygons = trace._polygons, + inside = false, + x0 = Infinity, + x1 = -Infinity, + y0 = Infinity, + y1 = -Infinity; + + for(var i = 0; i < polygons.length; i++) { + var polygon = polygons[i]; + // TODO: this is not going to work right for curved edges, it will + // act as though they're straight. That's probably going to need + // the elements themselves to capture the events. Worth it? + if(polygon.contains(pt)) { + inside = !inside; + // TODO: need better than just the overall bounding box + x0 = Math.min(x0, polygon.xmin); + x1 = Math.max(x1, polygon.xmax); + y0 = Math.min(y0, polygon.ymin); + y1 = Math.max(y1, polygon.ymax); + } + } + + if(inside) { + // get only fill or line color for the hover color + var color = Color.defaultLine; + if(Color.opacity(trace.fillcolor)) color = trace.fillcolor; + else if(Color.opacity((trace.line || {}).color)) { + color = trace.line.color; + } + + Lib.extendFlat(pointData, { + // never let a 2D override 1D type as closest point + distance: Fx.MAXDIST + 10, + x0: x0, + x1: x1, + y0: y0, + y1: y1, + color: color + }); + + delete pointData.index; + return [pointData]; + } + } + else { + var dx = function(di) { + // scatter points: d.mrc is the calculated marker radius + // adjust the distance so if you're inside the marker it + // always will show up regardless of point size, but + // prioritize smaller points + var rad = Math.max(3, di.mrc || 0); + return Math.max(Math.abs(xa.c2p(di.x) - xa.c2p(xval)) - rad, 1 - 3 / rad); + }, + dy = function(di) { + var rad = Math.max(3, di.mrc || 0); + return Math.max(Math.abs(ya.c2p(di.y) - ya.c2p(yval)) - rad, 1 - 3 / rad); + }, + dxy = function(di) { + var rad = Math.max(3, di.mrc || 0), + dx = Math.abs(xa.c2p(di.x) - xa.c2p(xval)), + dy = Math.abs(ya.c2p(di.y) - ya.c2p(yval)); + return Math.max(Math.sqrt(dx * dx + dy * dy) - rad, 1 - 3 / rad); + }, + distfn = Fx.getDistanceFunction(hovermode, dx, dy, dxy); + + Fx.getClosest(cd, distfn, pointData); + + // skip the rest (for this trace) if we didn't find a close point + if(pointData.index === false) return; + + // the closest data point + var di = cd[pointData.index], + xc = xa.c2p(di.x, true), + yc = ya.c2p(di.y, true), + rad = di.mrc || 1; + + pointData.color = getTraceColor(trace, di); + + pointData.x0 = xc - rad; + pointData.x1 = xc + rad; + pointData.xLabelVal = di.x; + + pointData.y0 = yc - rad; + pointData.y1 = yc + rad; + pointData.yLabelVal = di.y; + + if(di.tx) pointData.text = di.tx; + else if(trace.text) pointData.text = trace.text; + + ErrorBars.hoverInfo(di, trace, pointData); + + return [pointData]; + } }; diff --git a/src/traces/scatter/plot.js b/src/traces/scatter/plot.js index 2044c854fa1..05aa3dd8034 100644 --- a/src/traces/scatter/plot.js +++ b/src/traces/scatter/plot.js @@ -15,6 +15,8 @@ var Lib = require('../../lib'); var Drawing = require('../../components/drawing'); var ErrorBars = require('../../components/errorbars'); +var polygonTester = require('../../lib/polygon').tester; + var subTypes = require('./subtypes'); var arraysToCalcdata = require('./arrays_to_calcdata'); var linePoints = require('./line_points'); @@ -125,12 +127,22 @@ module.exports = function plot(gd, plotinfo, cdscatter) { linear: line.shape === 'linear' }); + // since we already have the pixel segments here, use them to make + // polygons for hover on fill + // TODO: can we skip this if hoveron!=fills? That would mean we + // need to redraw when you change hoveron... + trace._polygons = new Array(segments.length); + var i; + for(i = 0; i < segments.length; i++) { + trace._polygons[i] = polygonTester(segments[i]); + } + if(segments.length) { var pt0 = segments[0][0], lastSegment = segments[segments.length - 1], pt1 = lastSegment[lastSegment.length - 1]; - for(var i = 0; i < segments.length; i++) { + for(i = 0; i < segments.length; i++) { var pts = segments[i]; thispath = pathfn(pts); thisrevpath = revpathfn(pts); From 8aa43970d93e331feb2bbce95a7922711a6230fb Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Mon, 20 Jun 2016 11:54:36 +0200 Subject: [PATCH 02/16] fix bug in heatmap + non-heatmap hover if you make this plot, you could never hover on the scatter points: Plotly.newPlot(gd, [ {x:[1,2,3], y: [1,4,7], z:[[1,2,3],[4,5,6],[7,8,9]], type:'heatmap'}, {x:[1,2,3], y:[1,4,7]} ],{hovermode:'closest'}) Fx.MAXDIST had moved so sorting point distance was broken with heatmaps. --- src/traces/heatmap/hover.js | 9 +++++---- src/traces/scatter/hover.js | 3 ++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/traces/heatmap/hover.js b/src/traces/heatmap/hover.js index 64bab8457b2..700edb2ac49 100644 --- a/src/traces/heatmap/hover.js +++ b/src/traces/heatmap/hover.js @@ -10,12 +10,13 @@ 'use strict'; var Fx = require('../../plots/cartesian/graph_interact'); +var constants = require('../../plots/cartesian/constants'); var Lib = require('../../lib'); module.exports = function hoverPoints(pointData, xval, yval, hovermode, contour) { // never let a heatmap override another type as closest point - if(pointData.distance < Fx.MAXDIST) return; + if(pointData.distance < constants.MAXDIST) return; var cd0 = pointData.cd[0], trace = cd0.trace, @@ -46,8 +47,8 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode, contour) return; } } - else if(Fx.inbox(xval - x[0], xval - x[x.length - 1]) > Fx.MAXDIST || - Fx.inbox(yval - y[0], yval - y[y.length - 1]) > Fx.MAXDIST) { + else if(Fx.inbox(xval - x[0], xval - x[x.length - 1]) > constants.MAXDIST || + Fx.inbox(yval - y[0], yval - y[y.length - 1]) > constants.MAXDIST) { return; } else { @@ -99,7 +100,7 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode, contour) return [Lib.extendFlat(pointData, { index: [ny, nx], // never let a 2D override 1D type as closest point - distance: Fx.MAXDIST + 10, + distance: constants.MAXDIST + 10, x0: x0, x1: x1, y0: y0, diff --git a/src/traces/scatter/hover.js b/src/traces/scatter/hover.js index c73c657a311..e4b031646ec 100644 --- a/src/traces/scatter/hover.js +++ b/src/traces/scatter/hover.js @@ -11,6 +11,7 @@ var Lib = require('../../lib'); var Fx = require('../../plots/cartesian/graph_interact'); +var constants = require('../../plots/cartesian/constants'); var ErrorBars = require('../../components/errorbars'); var getTraceColor = require('./get_trace_color'); var Color = require('../../components/color'); @@ -59,7 +60,7 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { Lib.extendFlat(pointData, { // never let a 2D override 1D type as closest point - distance: Fx.MAXDIST + 10, + distance: constants.MAXDIST + 10, x0: x0, x1: x1, y0: y0, From 876bd1d5563edf4226aeed10e90980ff5759d3af Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Mon, 20 Jun 2016 16:38:59 +0200 Subject: [PATCH 03/16] put fill hover label at the right (or left) middle. Finds the outer crossing points at the vertical midline. Note that this will miss curves, if the lines are smoothed, and unlike the contains test there isn't a clean way (far as I know) to get it using browser machinery. --- src/traces/scatter/hover.js | 58 +++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 15 deletions(-) diff --git a/src/traces/scatter/hover.js b/src/traces/scatter/hover.js index e4b031646ec..f944b5fd3ef 100644 --- a/src/traces/scatter/hover.js +++ b/src/traces/scatter/hover.js @@ -29,28 +29,50 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { // even if hoveron is 'fills', only use it if we have polygons too if(trace.hoveron === 'fills' && trace._polygons) { var polygons = trace._polygons, + polygonsIn = [], inside = false, - x0 = Infinity, - x1 = -Infinity, - y0 = Infinity, - y1 = -Infinity; - - for(var i = 0; i < polygons.length; i++) { - var polygon = polygons[i]; + xmin = Infinity, + xmax = -Infinity, + ymin = Infinity, + ymax = -Infinity, + i, j, polygon, pts, xCross, x0, x1, y0, y1; + + for(i = 0; i < polygons.length; i++) { + polygon = polygons[i]; // TODO: this is not going to work right for curved edges, it will // act as though they're straight. That's probably going to need // the elements themselves to capture the events. Worth it? if(polygon.contains(pt)) { inside = !inside; // TODO: need better than just the overall bounding box - x0 = Math.min(x0, polygon.xmin); - x1 = Math.max(x1, polygon.xmax); - y0 = Math.min(y0, polygon.ymin); - y1 = Math.max(y1, polygon.ymax); + polygonsIn.push(polygon); + ymin = Math.min(ymin, polygon.ymin); + ymax = Math.max(ymax, polygon.ymax); } } if(inside) { + // find the overall left-most and right-most points of the + // polygon(s) we're inside at their combined vertical midpoint. + // This is where we will draw the hover label. + // Note that this might not be the vertical midpoint of the + // whole trace, if it's disjoint. + var yAvg = (ymin + ymax) / 2; + for(i = 0; i < polygonsIn.length; i++) { + pts = polygonsIn[i].pts; + for(j = 1; j < pts.length; j++) { + y0 = pts[j - 1][1]; + y1 = pts[j][1]; + if((y0 > yAvg) !== (y1 >= yAvg)) { + x0 = pts[j - 1][0]; + x1 = pts[j][0]; + xCross = x0 + (x1 - x0) * (yAvg - y0) / (y1 - y0); + xmin = Math.min(xmin, xCross); + xmax = Math.max(xmax, xCross); + } + } + } + // get only fill or line color for the hover color var color = Color.defaultLine; if(Color.opacity(trace.fillcolor)) color = trace.fillcolor; @@ -61,14 +83,20 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { Lib.extendFlat(pointData, { // never let a 2D override 1D type as closest point distance: constants.MAXDIST + 10, - x0: x0, - x1: x1, - y0: y0, - y1: y1, + x0: xmin, + x1: xmax, + y0: yAvg, + y1: yAvg, color: color }); delete pointData.index; + + if(trace.text && !Array.isArray(trace.text)) { + pointData.text = String(trace.text); + } + else pointData.text = trace.name; + return [pointData]; } } From 2018a74b02df9cadebaae2e0c1d63a15af1d694d Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Mon, 20 Jun 2016 16:51:27 +0200 Subject: [PATCH 04/16] prev trace changes fill hover the same way as it changes fill itself --- src/traces/scatter/plot.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/traces/scatter/plot.js b/src/traces/scatter/plot.js index 05aa3dd8034..d2388f40c22 100644 --- a/src/traces/scatter/plot.js +++ b/src/traces/scatter/plot.js @@ -43,6 +43,7 @@ module.exports = function plot(gd, plotinfo, cdscatter) { // BUILD LINES AND FILLS var prevpath = '', + prevPolygons = [], ownFillEl3, ownFillDir, tonext, nexttonext; scattertraces.each(function(d) { @@ -131,8 +132,9 @@ module.exports = function plot(gd, plotinfo, cdscatter) { // polygons for hover on fill // TODO: can we skip this if hoveron!=fills? That would mean we // need to redraw when you change hoveron... - trace._polygons = new Array(segments.length); - var i; + var thisPolygons = trace._polygons = new Array(segments.length), + i; + for(i = 0; i < segments.length; i++) { trace._polygons[i] = polygonTester(segments[i]); } @@ -197,8 +199,10 @@ module.exports = function plot(gd, plotinfo, cdscatter) { // existing curve or off the end of it tonext.attr('d', fullpath + 'L' + prevpath.substr(1) + 'Z'); } + trace._polygons = trace._polygons.concat(prevPolygons); } prevpath = revpath; + prevPolygons = thisPolygons; } }); From c7a9dd58f2ef2c12c0ab0317be7598d4f3bbb411 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Mon, 20 Jun 2016 17:03:38 +0200 Subject: [PATCH 05/16] couple of perf and style tweaks to regular point hover --- src/traces/scatter/hover.js | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/traces/scatter/hover.js b/src/traces/scatter/hover.js index f944b5fd3ef..61ec08d0deb 100644 --- a/src/traces/scatter/hover.js +++ b/src/traces/scatter/hover.js @@ -107,16 +107,16 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { // always will show up regardless of point size, but // prioritize smaller points var rad = Math.max(3, di.mrc || 0); - return Math.max(Math.abs(xa.c2p(di.x) - xa.c2p(xval)) - rad, 1 - 3 / rad); + return Math.max(Math.abs(xa.c2p(di.x) - xpx) - rad, 1 - 3 / rad); }, dy = function(di) { var rad = Math.max(3, di.mrc || 0); - return Math.max(Math.abs(ya.c2p(di.y) - ya.c2p(yval)) - rad, 1 - 3 / rad); + return Math.max(Math.abs(ya.c2p(di.y) - ypx) - rad, 1 - 3 / rad); }, dxy = function(di) { var rad = Math.max(3, di.mrc || 0), - dx = Math.abs(xa.c2p(di.x) - xa.c2p(xval)), - dy = Math.abs(ya.c2p(di.y) - ya.c2p(yval)); + dx = xa.c2p(di.x) - xpx, + dy = ya.c2p(di.y) - ypx; return Math.max(Math.sqrt(dx * dx + dy * dy) - rad, 1 - 3 / rad); }, distfn = Fx.getDistanceFunction(hovermode, dx, dy, dxy); @@ -132,15 +132,17 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { yc = ya.c2p(di.y, true), rad = di.mrc || 1; - pointData.color = getTraceColor(trace, di); + Lib.extendFlat(pointData, { + color: getTraceColor(trace, di), - pointData.x0 = xc - rad; - pointData.x1 = xc + rad; - pointData.xLabelVal = di.x; + x0: xc - rad, + x1: xc + rad, + xLabelVal: di.x, - pointData.y0 = yc - rad; - pointData.y1 = yc + rad; - pointData.yLabelVal = di.y; + y0: yc - rad, + y1: yc + rad, + yLabelVal: di.y + }); if(di.tx) pointData.text = di.tx; else if(trace.text) pointData.text = trace.text; From 5552c77b6227dd2b5b682cba94134d84cce45664 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Mon, 20 Jun 2016 19:09:44 +0200 Subject: [PATCH 06/16] test hoveron=fills label position and text --- test/jasmine/tests/hover_label_test.js | 39 ++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index ec61cd122cd..6ccb0d67223 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -652,3 +652,42 @@ describe('hover after resizing', function() { }).then(done); }); }); + +describe('hover on fill', function() { + 'use strict'; + + afterEach(destroyGraphDiv); + + function assertLabelsCorrect(mousePos, labelPos, labelText) { + return new Promise(function(resolve) { + 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], 0, labelText + ':x'); + expect(+transformCoords[1]).toBeCloseTo(labelPos[1], 0, labelText + ':y'); + + resolve(); + }, constants.HOVERMINTIME); + }); + } + + it('should always show one label in the right place', function(done) { + var mock = require('@mocks/scatter_fill_self_next.json'); + mock.data.forEach(function(trace) { trace.hoveron = 'fills'; }); + + Plotly.plot(createGraphDiv(), mock.data, mock.layout).then(function() { + return assertLabelsCorrect([250, 150], [252.575, 133.8], 'trace 2'); + }).then(function() { + return assertLabelsCorrect([250, 300], [234.125, 210], 'trace 1'); + }).then(function() { + return assertLabelsCorrect([155, 260], [160.325, 248.1], 'trace 0'); + }).then(done); + }); +}); \ No newline at end of file From ffd56c733fbdbd48a04945656b647fe9c0007e68 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Mon, 20 Jun 2016 21:05:21 +0200 Subject: [PATCH 07/16] fix and test hoveron=fills for scatterternary --- src/traces/scatterternary/attributes.js | 1 + src/traces/scatterternary/defaults.js | 8 ++++++++ src/traces/scatterternary/hover.js | 4 ++++ test/jasmine/tests/hover_label_test.js | 13 +++++++++++++ 4 files changed, 26 insertions(+) diff --git a/src/traces/scatterternary/attributes.js b/src/traces/scatterternary/attributes.js index 5dd23f7a674..fb28327b348 100644 --- a/src/traces/scatterternary/attributes.js +++ b/src/traces/scatterternary/attributes.js @@ -117,6 +117,7 @@ module.exports = { hoverinfo: extendFlat({}, plotAttrs.hoverinfo, { flags: ['a', 'b', 'c', 'text', 'name'] }), + hoveron: scatterAttrs.hoveron, _nestedModules: { 'marker.colorbar': 'Colorbar' } diff --git a/src/traces/scatterternary/defaults.js b/src/traces/scatterternary/defaults.js index 169d922a958..a3cd0cc2cb5 100644 --- a/src/traces/scatterternary/defaults.js +++ b/src/traces/scatterternary/defaults.js @@ -81,8 +81,11 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout handleTextDefaults(traceIn, traceOut, layout, coerce); } + var dfltHoverOn = ''; + if(subTypes.hasMarkers(traceOut) || subTypes.hasText(traceOut)) { coerce('marker.maxdisplayed'); + dfltHoverOn = 'points'; } coerce('fill'); @@ -92,4 +95,9 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout } coerce('hoverinfo', (layout._dataLength === 1) ? 'a+b+c+text' : undefined); + + if(!dfltHoverOn && (traceOut.fill === 'tonext' || traceOut.fill === 'toself')) { + dfltHoverOn = 'fills'; + } + coerce('hoveron', dfltHoverOn || 'points'); }; diff --git a/src/traces/scatterternary/hover.js b/src/traces/scatterternary/hover.js index 212b7d18d83..e231bc39508 100644 --- a/src/traces/scatterternary/hover.js +++ b/src/traces/scatterternary/hover.js @@ -17,6 +17,10 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { var scatterPointData = scatterHover(pointData, xval, yval, hovermode); if(!scatterPointData || scatterPointData[0].index === false) return; + // if hoveron='fills', we don't show any point data so the label is + // unchanged from what scatter gives us. + if(scatterPointData[0].index === undefined) return scatterPointData; + var newPointData = scatterPointData[0], cdi = newPointData.cd[newPointData.index]; diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index 6ccb0d67223..03f360b0994 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -690,4 +690,17 @@ describe('hover on fill', function() { return assertLabelsCorrect([155, 260], [160.325, 248.1], 'trace 0'); }).then(done); }); + + it('should work for scatterternary too', function(done) { + var mock = require('@mocks/ternary_fill.json'); + mock.data.forEach(function(trace) { trace.hoveron = 'fills'; }); + + Plotly.plot(createGraphDiv(), mock.data, mock.layout).then(function() { + return assertLabelsCorrect([245, 171], [249.7, 166], 'trace 2'); + }).then(function() { + return assertLabelsCorrect([245, 226], [268.75, 265], 'trace 1'); + }).then(function() { + return assertLabelsCorrect([245, 259], [249.7, 254], 'trace 0'); + }).then(done); + }); }); \ No newline at end of file From dddb6e123857bca8bc6319319788662767b5d5f3 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Tue, 21 Jun 2016 03:27:25 +0200 Subject: [PATCH 08/16] lock test graphDiv to position 0,0 and other jasmine reliability tweaks fixes #666 --- test/jasmine/assets/create_graph_div.js | 6 ++++ test/jasmine/tests/click_test.js | 34 +++++++++--------- test/jasmine/tests/hover_label_test.js | 32 ++++++++--------- test/jasmine/tests/hover_pie_test.js | 20 +++++------ test/jasmine/tests/modebar_test.js | 6 ++-- test/jasmine/tests/range_slider_test.js | 48 ++++++++++++++----------- test/jasmine/tests/select_test.js | 14 ++++---- test/jasmine/tests/ternary_test.js | 4 +-- 8 files changed, 90 insertions(+), 74 deletions(-) diff --git a/test/jasmine/assets/create_graph_div.js b/test/jasmine/assets/create_graph_div.js index 0788b7f5dbc..9791d46018c 100644 --- a/test/jasmine/assets/create_graph_div.js +++ b/test/jasmine/assets/create_graph_div.js @@ -4,5 +4,11 @@ module.exports = function createGraphDiv() { var gd = document.createElement('div'); gd.id = 'graph'; document.body.appendChild(gd); + + // force the graph to be at position 0,0 no matter what + gd.style.position = 'fixed'; + gd.style.left = 0; + gd.style.top = 0; + return gd; }; diff --git a/test/jasmine/tests/click_test.js b/test/jasmine/tests/click_test.js index c9f70b4dcd9..f68166e04e7 100644 --- a/test/jasmine/tests/click_test.js +++ b/test/jasmine/tests/click_test.js @@ -14,8 +14,8 @@ describe('Test click interactions:', function() { var mockCopy, gd; - var pointPos = [351, 223], - blankPos = [70, 363]; + var pointPos = [344, 216], + blankPos = [63, 356]; var autoRangeX = [-3.011967491973726, 2.1561305597186564], autoRangeY = [-0.9910086301469277, 1.389382716298284]; @@ -46,8 +46,8 @@ describe('Test click interactions:', function() { setTimeout(function() { click(x, y); - resolve(); - }, DBLCLICKDELAY / 2); + setTimeout(function() { resolve(); }, DBLCLICKDELAY / 2); + }, DBLCLICKDELAY / 4); }); } @@ -530,8 +530,8 @@ describe('Test click interactions:', function() { return drag(100, 100, 200, 200, DBLCLICKDELAY / 2); }).then(function() { - expect(gd.layout.xaxis.range).toBeCloseToArray([-2.70624901567643, -1.9783478816352495]); - expect(gd.layout.yaxis.range).toBeCloseToArray([0.5007032802920716, 1.2941670624404753]); + expect(gd.layout.xaxis.range).toBeCloseToArray([-2.6480169249531356,-1.920115790911955]); + expect(gd.layout.yaxis.range).toBeCloseToArray([0.4372261777201992,1.2306899598686027]); done(); }); @@ -697,14 +697,14 @@ describe('Test click interactions:', function() { expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); expect(gd.layout.yaxis.range).toBeCloseToArray(autoRangeY); - drag(100, 100, 400, 300).then(function() { - expect(gd.layout.xaxis.range).toBeCloseToArray([-2.70624901, -0.52254561]); - expect(gd.layout.yaxis.range).toBeCloseToArray([-0.29276050, 1.294167062]); + drag(93, 93, 393, 293).then(function() { + expect(gd.layout.xaxis.range).toBeCloseToArray([-2.69897000,-0.515266602]); + expect(gd.layout.yaxis.range).toBeCloseToArray([-0.30069513,1.2862324246]); - return drag(100, 100, 400, 300); + return drag(93, 93, 393, 293); }).then(function() { - expect(gd.layout.xaxis.range).toBeCloseToArray([-2.57707219, -1.65438061]); - expect(gd.layout.yaxis.range).toBeCloseToArray([0.172738250, 1.230689959]); + expect(gd.layout.xaxis.range).toBeCloseToArray([-2.56671754,-1.644025966]); + expect(gd.layout.yaxis.range).toBeCloseToArray([0.159513853,1.2174655634]); done(); }); @@ -721,8 +721,8 @@ describe('Test click interactions:', function() { var plot = gd._fullLayout._plots.xy.plot; - mouseEvent('mousemove', 400, 250); - mouseEvent('scroll', 400, 250, { deltaX: 0, deltaY: -1000 }); + mouseEvent('mousemove', 393, 243); + mouseEvent('scroll', 393, 243, { deltaX: 0, deltaY: -1000 }); var transform = plot.attr('transform'); @@ -735,7 +735,7 @@ describe('Test click interactions:', function() { var translate = Lib.getTranslate(mockEl), scale = Lib.getScale(mockEl); - expect([translate.x, translate.y]).toBeCloseToArray([62.841, 99.483]); + expect([translate.x, translate.y]).toBeCloseToArray([61.070, 97.712]); expect([scale.x, scale.y]).toBeCloseToArray([1.221, 1.221]); }); }); @@ -751,11 +751,11 @@ describe('Test click interactions:', function() { expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); expect(gd.layout.yaxis.range).toBeCloseToArray(autoRangeY); - drag(100, 100, 400, 300).then(function() { + drag(93, 93, 393, 293).then(function() { expect(gd.layout.xaxis.range).toBeCloseToArray([-5.19567089, -0.02757284]); expect(gd.layout.yaxis.range).toBeCloseToArray([0.595918934, 2.976310280]); - return drag(100, 100, 400, 300); + return drag(93, 93, 393, 293); }).then(function() { expect(gd.layout.xaxis.range).toBeCloseToArray([-7.37937429, -2.21127624]); expect(gd.layout.yaxis.range).toBeCloseToArray([2.182846498, 4.563237844]); diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index 03f360b0994..5fb6dec6817 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -406,7 +406,7 @@ describe('hover info', function() { it('should display the correct format when ticklabels true', function() { Plotly.plot(this.gd, data, layout); - mouseEvent('mousemove', 310, 220); + mouseEvent('mousemove', 303, 213); var hovers = d3.selectAll('g.hovertext'); @@ -417,7 +417,7 @@ describe('hover info', function() { it('should display the correct format when ticklabels false', function() { layout.yaxis.showticklabels = false; Plotly.plot(this.gd, data, layout); - mouseEvent('mousemove', 310, 220); + mouseEvent('mousemove', 303, 213); var hovers = d3.selectAll('g.hovertext'); @@ -445,27 +445,27 @@ describe('hover info', function() { }); it('should show text labels', function() { - mouseEvent('mousemove', 115, 310); + mouseEvent('mousemove', 108, 303); var hovers = d3.selectAll('g.hovertext'); expect(hovers.size()).toEqual(1); expect(hovers.select('text')[0][0].textContent).toEqual('test'); }); it('should show number labels', function() { - mouseEvent('mousemove', 370, 180); + mouseEvent('mousemove', 363, 173); var hovers = d3.selectAll('g.hovertext'); expect(hovers.size()).toEqual(1); expect(hovers.select('text')[0][0].textContent).toEqual('42'); }); it('should not show null text labels', function() { - mouseEvent('mousemove', 236, 246); + mouseEvent('mousemove', 229, 239); var hovers = d3.selectAll('g.hovertext'); expect(hovers.size()).toEqual(0); }); it('should not show undefined text labels', function() { - mouseEvent('mousemove', 500, 115); + mouseEvent('mousemove', 493, 108); var hovers = d3.selectAll('g.hovertext'); expect(hovers.size()).toEqual(0); }); @@ -586,7 +586,7 @@ describe('hover info on overlaid subplots', function() { var mock = require('@mocks/autorange-tozero-rangemode.json'); Plotly.plot(createGraphDiv(), mock.data, mock.layout).then(function() { - mouseEvent('mousemove', 775, 352); + mouseEvent('mousemove', 768, 345); var axisText = d3.selectAll('g.axistext'), hoverText = d3.selectAll('g.hovertext'); @@ -630,8 +630,8 @@ describe('hover after resizing', function() { layout = { width: 600, height: 500 }, gd = createGraphDiv(); - var pos0 = [311, 409], - pos1 = [407, 128]; + var pos0 = [305, 403], + pos1 = [401, 122]; Plotly.plot(gd, data, layout).then(function() { return assertLabelCount(pos0, 1, 'before resize, showing pt label'); @@ -683,11 +683,11 @@ describe('hover on fill', function() { mock.data.forEach(function(trace) { trace.hoveron = 'fills'; }); Plotly.plot(createGraphDiv(), mock.data, mock.layout).then(function() { - return assertLabelsCorrect([250, 150], [252.575, 133.8], 'trace 2'); + return assertLabelsCorrect([242, 142], [249.175, 133.8], 'trace 2'); }).then(function() { - return assertLabelsCorrect([250, 300], [234.125, 210], 'trace 1'); + return assertLabelsCorrect([242, 292], [231.125, 210], 'trace 1'); }).then(function() { - return assertLabelsCorrect([155, 260], [160.325, 248.1], 'trace 0'); + return assertLabelsCorrect([147, 252], [158.925, 248.1], 'trace 0'); }).then(done); }); @@ -696,11 +696,11 @@ describe('hover on fill', function() { mock.data.forEach(function(trace) { trace.hoveron = 'fills'; }); Plotly.plot(createGraphDiv(), mock.data, mock.layout).then(function() { - return assertLabelsCorrect([245, 171], [249.7, 166], 'trace 2'); + return assertLabelsCorrect([237, 163], [247.7, 166], 'trace 2'); }).then(function() { - return assertLabelsCorrect([245, 226], [268.75, 265], 'trace 1'); + return assertLabelsCorrect([237, 218], [266.75, 265], 'trace 1'); }).then(function() { - return assertLabelsCorrect([245, 259], [249.7, 254], 'trace 0'); + return assertLabelsCorrect([237, 251], [247.7, 254], 'trace 0'); }).then(done); }); -}); \ No newline at end of file +}); diff --git a/test/jasmine/tests/hover_pie_test.js b/test/jasmine/tests/hover_pie_test.js index e7472b33951..75610e36834 100644 --- a/test/jasmine/tests/hover_pie_test.js +++ b/test/jasmine/tests/hover_pie_test.js @@ -54,8 +54,8 @@ describe('pie hovering', function() { unhoverData = data; }); - mouseEvent('mouseover', width / 2, height / 2); - mouseEvent('mouseout', width / 2, height / 2); + mouseEvent('mouseover', width / 2 - 7, height / 2 - 7); + mouseEvent('mouseout', width / 2 - 7, height / 2 - 7); expect(hoverData.points.length).toEqual(1); expect(unhoverData.points.length).toEqual(1); @@ -82,9 +82,9 @@ describe('pie hovering', function() { hoverData.push(data); }); - mouseEvent('mouseover', 180, 140); + mouseEvent('mouseover', 173, 133); setTimeout(function() { - mouseEvent('mouseover', 240, 200); + mouseEvent('mouseover', 233, 193); expect(count).toEqual(2); expect(hoverData[0]).not.toEqual(hoverData[1]); done(); @@ -100,11 +100,11 @@ describe('pie hovering', function() { unhoverData.push(data); }); - mouseEvent('mouseover', 180, 140); - mouseEvent('mouseout', 180, 140); + mouseEvent('mouseover', 173, 133); + mouseEvent('mouseout', 173, 133); setTimeout(function() { - mouseEvent('mouseover', 240, 200); - mouseEvent('mouseout', 240, 200); + mouseEvent('mouseover', 233, 193); + mouseEvent('mouseout', 233, 193); expect(count).toEqual(2); expect(unhoverData[0]).not.toEqual(unhoverData[1]); done(); @@ -130,7 +130,7 @@ describe('pie hovering', function() { Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(function() { - mouseEvent('mouseover', 230, 150); + mouseEvent('mouseover', 223, 143); var labels = Plotly.d3.selectAll('.hovertext .nums .line'); @@ -152,7 +152,7 @@ describe('pie hovering', function() { Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(function() { - mouseEvent('mouseover', 230, 150); + mouseEvent('mouseover', 223, 143); var labels = Plotly.d3.selectAll('.hovertext .nums .line'); diff --git a/test/jasmine/tests/modebar_test.js b/test/jasmine/tests/modebar_test.js index b8a5c460e12..3c4c884169e 100644 --- a/test/jasmine/tests/modebar_test.js +++ b/test/jasmine/tests/modebar_test.js @@ -660,7 +660,9 @@ describe('ModeBar', function() { yaxis2: { anchor: 'x2', range: [0, 4] - } + }, + width: 600, + height: 500 }; gd = createGraphDiv(); @@ -696,7 +698,7 @@ describe('ModeBar', function() { buttonZoomIn.click(); buttonAutoScale.click(); - assertRange(gd._fullLayout.xaxis.range, [-0.1375913, 2.137591]); + assertRange(gd._fullLayout.xaxis.range, [-0.1584327, 2.1584327]); assertRange(gd._fullLayout.yaxis.range, [0.92675159, 2.073248]); assertRange(gd._fullLayout.xaxis2.range, [-0.5, 2.5]); assertRange(gd._fullLayout.yaxis2.range, [0, 2.105263]); diff --git a/test/jasmine/tests/range_slider_test.js b/test/jasmine/tests/range_slider_test.js index a603c21b324..54b6c3038e6 100644 --- a/test/jasmine/tests/range_slider_test.js +++ b/test/jasmine/tests/range_slider_test.js @@ -12,6 +12,8 @@ describe('the range slider', function() { rangeSlider, children; + var sliderY = 393; + describe('when specified as visible', function() { beforeEach(function(done) { @@ -57,7 +59,7 @@ describe('the range slider', function() { dataMinStart = rangeSlider.getAttribute('data-min'), diff = end - start; - slide(start, 400, end, 400).then(function() { + slide(start, sliderY, end, sliderY).then(function() { var maskMin = children[2], handleMin = children[5]; @@ -67,19 +69,25 @@ describe('the range slider', function() { }).then(done); }); + function testTranslate1D(node, val) { + var transformParts = node.getAttribute('transform').split('('); + expect(transformParts[0]).toEqual('translate'); + expect(+transformParts[1].split(')')[0]).toBeCloseTo(val, 0); + } + it('should react to resizing the maximum handle', function(done) { - var start = 705, - end = 500, + var start = 695, + end = 490, dataMaxStart = rangeSlider.getAttribute('data-max'), diff = end - start; - slide(start, 400, end, 400).then(function() { + slide(start, sliderY, end, sliderY).then(function() { var maskMax = children[3], handleMax = children[6]; - expect(rangeSlider.getAttribute('data-max')).toEqual(String(+dataMaxStart + diff)); - expect(maskMax.getAttribute('width')).toEqual(String(-diff)); - expect(handleMax.getAttribute('transform')).toBe('translate(' + (+dataMaxStart + diff) + ')'); + expect(+rangeSlider.getAttribute('data-max')).toBeCloseTo(+dataMaxStart + diff, 0); + expect(+maskMax.getAttribute('width')).toBeCloseTo(-diff); + testTranslate1D(handleMax, +dataMaxStart + diff) }).then(done); }); @@ -89,13 +97,13 @@ describe('the range slider', function() { dataMinStart = rangeSlider.getAttribute('data-min'), diff = end - start; - slide(start, 400, end, 400).then(function() { + slide(start, sliderY, end, sliderY).then(function() { var maskMin = children[2], handleMin = children[5]; - expect(rangeSlider.getAttribute('data-min')).toEqual(String(+dataMinStart + diff)); - expect(maskMin.getAttribute('width')).toEqual(String(diff)); - expect(handleMin.getAttribute('transform')).toEqual('translate(' + (+dataMinStart + diff - 3) + ')'); + expect(+rangeSlider.getAttribute('data-min')).toBeCloseTo(String(+dataMinStart + diff)); + expect(+maskMin.getAttribute('width')).toBeCloseTo(String(diff)); + testTranslate1D(handleMin, +dataMinStart + diff - 3); }).then(done); }); @@ -105,13 +113,13 @@ describe('the range slider', function() { dataMaxStart = rangeSlider.getAttribute('data-max'), diff = end - start; - slide(start, 400, end, 400).then(function() { + slide(start, sliderY, end, sliderY).then(function() { var maskMax = children[3], handleMax = children[6]; - expect(rangeSlider.getAttribute('data-max')).toEqual(String(+dataMaxStart + diff)); - expect(maskMax.getAttribute('width')).toEqual(String(-diff)); - expect(handleMax.getAttribute('transform')).toEqual('translate(' + (+dataMaxStart + diff) + ')'); + expect(+rangeSlider.getAttribute('data-max')).toBeCloseTo(+dataMaxStart + diff); + expect(+maskMax.getAttribute('width')).toBeCloseTo(-diff); + testTranslate1D(handleMax, +dataMaxStart + diff); }).then(done); @@ -124,14 +132,14 @@ describe('the range slider', function() { rangeDiff2, rangeDiff3; - slide(start, 400, end, 400).then(function() { + slide(start, sliderY, end, sliderY).then(function() { rangeDiff2 = gd._fullLayout.xaxis.range[1] - gd._fullLayout.xaxis.range[0]; expect(rangeDiff2).toBeLessThan(rangeDiff1); }).then(function() { start = 400; end = 200; - return slide(start, 400, end, 400); + return slide(start, sliderY, end, sliderY); }).then(function() { rangeDiff3 = gd._fullLayout.xaxis.range[1] - gd._fullLayout.xaxis.range[0]; expect(rangeDiff3).toBeLessThan(rangeDiff2); @@ -142,8 +150,8 @@ describe('the range slider', function() { Plotly.relayout(gd, 'xaxis.range', [10, 20]) .then(function() { expect(gd._fullLayout.xaxis.range).toEqual([10, 20]); - expect(+rangeSlider.getAttribute('data-min')).toBeCloseTo(125.51, 0); - expect(+rangeSlider.getAttribute('data-max')).toBeCloseTo(251.02, 0); + expect(+rangeSlider.getAttribute('data-min')).toBeCloseTo(124.69, -1); + expect(+rangeSlider.getAttribute('data-max')).toBeCloseTo(249.39, -1); }) .then(done); }); @@ -152,7 +160,7 @@ describe('the range slider', function() { Plotly.relayout(gd, 'xaxis.range[0]', 10) .then(function() { expect(gd._fullLayout.xaxis.range[0]).toEqual(10); - expect(+rangeSlider.getAttribute('data-min')).toBeCloseTo(125.51, 0); + expect(+rangeSlider.getAttribute('data-min')).toBeCloseTo(124.69, -1); }) .then(done); }); diff --git a/test/jasmine/tests/select_test.js b/test/jasmine/tests/select_test.js index f709d3b962c..a4fd9bb4997 100644 --- a/test/jasmine/tests/select_test.js +++ b/test/jasmine/tests/select_test.js @@ -13,6 +13,9 @@ var customMatchers = require('../assets/custom_matchers'); describe('select box and lasso', function() { var mock = require('@mocks/14.json'); + var selectPath = [[93, 193], [143, 193]]; + var lassoPath = [[316, 171], [318, 239], [335, 243], [328, 169]]; + beforeEach(function() { jasmine.addMatchers(customMatchers); }); @@ -186,7 +189,7 @@ describe('select box and lasso', function() { doubleClickData = data; }); - drag([[100, 200], [150, 200]]); + drag(selectPath); expect(selectingCnt).toEqual(1, 'with the correct selecting count'); expect(selectingData.points).toEqual([{ @@ -201,7 +204,7 @@ describe('select box and lasso', function() { y: 12.5 }], 'with the correct selecting points'); assertRange(selectingData.range, { - x: [0.0019667582669138295, 0.004546754982054625], + x: [0.002000, 0.0046236], y: [0.10209191961595454, 24.512223978291406] }, 'with the correct selecting range'); @@ -218,7 +221,7 @@ describe('select box and lasso', function() { y: 12.5 }], 'with the correct selected points'); assertRange(selectedData.range, { - x: [0.0019667582669138295, 0.004546754982054625], + x: [0.002000, 0.0046236], y: [0.10209191961595454, 24.512223978291406] }, 'with the correct selected range'); @@ -262,7 +265,7 @@ describe('select box and lasso', function() { doubleClickData = data; }); - drag([[331, 178], [333, 246], [350, 250], [343, 176]]); + drag(lassoPath); expect(selectingCnt).toEqual(3, 'with the correct selecting count'); expect(selectingData.points).toEqual([{ @@ -291,9 +294,6 @@ describe('select box and lasso', function() { var mockCopy = Lib.extendDeep({}, mock); mockCopy.layout.dragmode = 'select'; - var selectPath = [[100, 200], [150, 200]]; - var lassoPath = [[331, 178], [333, 246], [350, 250], [343, 176]]; - var gd = createGraphDiv(); var selectedPtLength; diff --git a/test/jasmine/tests/ternary_test.js b/test/jasmine/tests/ternary_test.js index bf631dfd54d..2c607cb9bc6 100644 --- a/test/jasmine/tests/ternary_test.js +++ b/test/jasmine/tests/ternary_test.js @@ -186,8 +186,8 @@ describe('ternary plots', function() { it('should respond zoom drag interactions', function(done) { assertRange(gd, [0.231, 0.2, 0.11]); - drag([[390, 220], [300, 250]]); - assertRange(gd, [0.4486, 0.2480, 0.1453]); + drag([[383, 213], [293, 243]]); + assertRange(gd, [0.4435,0.2462,0.1523]); doubleClick(pointPos[0], pointPos[1]).then(function() { assertRange(gd, [0, 0, 0]); From 19311029560fcce5671d5f5511fc8ed9f36a7f84 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Tue, 21 Jun 2016 10:50:03 +0200 Subject: [PATCH 09/16] test hoveron in supplyDefaults --- test/jasmine/tests/scatter_test.js | 33 ++++++++++++++++++++++ test/jasmine/tests/scatterternary_test.js | 34 +++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/test/jasmine/tests/scatter_test.js b/test/jasmine/tests/scatter_test.js index 5a9f7406c4d..f32fc407d77 100644 --- a/test/jasmine/tests/scatter_test.js +++ b/test/jasmine/tests/scatter_test.js @@ -60,6 +60,39 @@ describe('Test scatter', function() { expect(traceOut.visible).toBe(false); }); + it('should correctly assign \'hoveron\' default', function() { + traceIn = { + x: [1, 2, 3], + y: [1, 2, 3], + mode: 'lines+markers', + fill: 'tonext' + }; + + // even with fill tonext, as long as there are markers or text + // you get points + // you need visible: true here, as that normally gets set + // outside of the module supplyDefaults + traceOut = {visible: true}; + supplyDefaults(traceIn, traceOut, defaultColor, layout); + expect(traceOut.hoveron).toBe('points'); + + // but with only lines (or just fill) and fill tonext or toself + // you get fills + traceIn.mode = 'lines'; + traceOut = {visible: true}; + supplyDefaults(traceIn, traceOut, defaultColor, layout); + expect(traceOut.hoveron).toBe('fills'); + + // with the wrong fill you always get points + // only area fills default to hoveron points. Vertical or + // horizontal fills don't have the same physical meaning, + // they're generally just filling their own slice, so they + // default to hoveron points. + traceIn.fill = 'tonexty'; + traceOut = {visible: true}; + supplyDefaults(traceIn, traceOut, defaultColor, layout); + expect(traceOut.hoveron).toBe('points'); + }); }); describe('isBubble', function() { diff --git a/test/jasmine/tests/scatterternary_test.js b/test/jasmine/tests/scatterternary_test.js index 470ec336cb4..2b840122f8c 100644 --- a/test/jasmine/tests/scatterternary_test.js +++ b/test/jasmine/tests/scatterternary_test.js @@ -158,6 +158,40 @@ describe('scatterternary defaults', function() { supplyDefaults(traceIn, traceOut, defaultColor, layout); expect(traceOut.hoverinfo).toBe('a+b+c+text'); }); + + it('should correctly assign \'hoveron\' default', function() { + traceIn = { + a: [1, 2, 3], + b: [1, 2, 3], + c: [1, 2, 3], + mode: 'lines+markers', + fill: 'tonext' + }; + + // even with fill tonext, as long as there are markers or text + // you get points + // you need visible: true here, as that normally gets set + // outside of the module supplyDefaults + traceOut = {visible: true}; + supplyDefaults(traceIn, traceOut, defaultColor, layout); + expect(traceOut.hoveron).toBe('points'); + + // but with only lines (or just fill) and fill tonext or toself + // you get fills + traceIn.mode = 'lines'; + traceOut = {visible: true}; + supplyDefaults(traceIn, traceOut, defaultColor, layout); + expect(traceOut.hoveron).toBe('fills'); + + // without a fill you always get points. For scatterternary, unlike + // scatter, every allowed fill but 'none' is an area fill (rather than + // a vertical / horizontal fill) so they all should default to + // hoveron points. + traceIn.fill = 'none'; + traceOut = {visible: true}; + supplyDefaults(traceIn, traceOut, defaultColor, layout); + expect(traceOut.hoveron).toBe('points'); + }); }); describe('scatterternary calc', function() { From 025b7888b2673d2cf7aa7ceab4355e347a436dbe Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Tue, 21 Jun 2016 12:04:13 +0200 Subject: [PATCH 10/16] more lenient test of hoveron fills label position --- test/jasmine/tests/hover_label_test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index 5fb6dec6817..defff180b7e 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -670,8 +670,8 @@ describe('hover on fill', function() { var transformParts = hoverText.attr('transform').split('('); expect(transformParts[0]).toEqual('translate'); var transformCoords = transformParts[1].split(')')[0].split(','); - expect(+transformCoords[0]).toBeCloseTo(labelPos[0], 0, labelText + ':x'); - expect(+transformCoords[1]).toBeCloseTo(labelPos[1], 0, labelText + ':y'); + expect(+transformCoords[0]).toBeCloseTo(labelPos[0], -1, labelText + ':x'); + expect(+transformCoords[1]).toBeCloseTo(labelPos[1], -1, labelText + ':y'); resolve(); }, constants.HOVERMINTIME); From 1a685d4f440e515dcacb0881ee6df6ea96dd5932 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Tue, 21 Jun 2016 12:22:39 +0200 Subject: [PATCH 11/16] appease eslint :cow2: --- test/jasmine/tests/click_test.js | 12 ++++++------ test/jasmine/tests/range_slider_test.js | 2 +- test/jasmine/tests/ternary_test.js | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/test/jasmine/tests/click_test.js b/test/jasmine/tests/click_test.js index f68166e04e7..b4b70f66e00 100644 --- a/test/jasmine/tests/click_test.js +++ b/test/jasmine/tests/click_test.js @@ -530,8 +530,8 @@ describe('Test click interactions:', function() { return drag(100, 100, 200, 200, DBLCLICKDELAY / 2); }).then(function() { - expect(gd.layout.xaxis.range).toBeCloseToArray([-2.6480169249531356,-1.920115790911955]); - expect(gd.layout.yaxis.range).toBeCloseToArray([0.4372261777201992,1.2306899598686027]); + expect(gd.layout.xaxis.range).toBeCloseToArray([-2.6480169249531356, -1.920115790911955]); + expect(gd.layout.yaxis.range).toBeCloseToArray([0.4372261777201992, 1.2306899598686027]); done(); }); @@ -698,13 +698,13 @@ describe('Test click interactions:', function() { expect(gd.layout.yaxis.range).toBeCloseToArray(autoRangeY); drag(93, 93, 393, 293).then(function() { - expect(gd.layout.xaxis.range).toBeCloseToArray([-2.69897000,-0.515266602]); - expect(gd.layout.yaxis.range).toBeCloseToArray([-0.30069513,1.2862324246]); + expect(gd.layout.xaxis.range).toBeCloseToArray([-2.69897000, -0.515266602]); + expect(gd.layout.yaxis.range).toBeCloseToArray([-0.30069513, 1.2862324246]); return drag(93, 93, 393, 293); }).then(function() { - expect(gd.layout.xaxis.range).toBeCloseToArray([-2.56671754,-1.644025966]); - expect(gd.layout.yaxis.range).toBeCloseToArray([0.159513853,1.2174655634]); + expect(gd.layout.xaxis.range).toBeCloseToArray([-2.56671754, -1.644025966]); + expect(gd.layout.yaxis.range).toBeCloseToArray([0.159513853, 1.2174655634]); done(); }); diff --git a/test/jasmine/tests/range_slider_test.js b/test/jasmine/tests/range_slider_test.js index 54b6c3038e6..2ca0ebe89c3 100644 --- a/test/jasmine/tests/range_slider_test.js +++ b/test/jasmine/tests/range_slider_test.js @@ -87,7 +87,7 @@ describe('the range slider', function() { expect(+rangeSlider.getAttribute('data-max')).toBeCloseTo(+dataMaxStart + diff, 0); expect(+maskMax.getAttribute('width')).toBeCloseTo(-diff); - testTranslate1D(handleMax, +dataMaxStart + diff) + testTranslate1D(handleMax, +dataMaxStart + diff); }).then(done); }); diff --git a/test/jasmine/tests/ternary_test.js b/test/jasmine/tests/ternary_test.js index 2c607cb9bc6..c00df866914 100644 --- a/test/jasmine/tests/ternary_test.js +++ b/test/jasmine/tests/ternary_test.js @@ -187,7 +187,7 @@ describe('ternary plots', function() { assertRange(gd, [0.231, 0.2, 0.11]); drag([[383, 213], [293, 243]]); - assertRange(gd, [0.4435,0.2462,0.1523]); + assertRange(gd, [0.4435, 0.2462, 0.1523]); doubleClick(pointPos[0], pointPos[1]).then(function() { assertRange(gd, [0, 0, 0]); From f22fac800744694f62a43d900b16f9c4221a5c5c Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Tue, 21 Jun 2016 12:31:18 +0200 Subject: [PATCH 12/16] give back a little more time in click_test doubleclick --- test/jasmine/tests/click_test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jasmine/tests/click_test.js b/test/jasmine/tests/click_test.js index b4b70f66e00..c08b56ea711 100644 --- a/test/jasmine/tests/click_test.js +++ b/test/jasmine/tests/click_test.js @@ -47,7 +47,7 @@ describe('Test click interactions:', function() { setTimeout(function() { click(x, y); setTimeout(function() { resolve(); }, DBLCLICKDELAY / 2); - }, DBLCLICKDELAY / 4); + }, DBLCLICKDELAY / 2); }); } From ba77f5bfcadd5c1c1fb8cda817ca57b4fee28a49 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Tue, 21 Jun 2016 20:40:52 +0200 Subject: [PATCH 13/16] copy hoveron fill testing mocks --- test/jasmine/tests/hover_label_test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index defff180b7e..f548fd64d2b 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -679,7 +679,7 @@ describe('hover on fill', function() { } it('should always show one label in the right place', function(done) { - var mock = require('@mocks/scatter_fill_self_next.json'); + var mock = Lib.extendDeep({}, require('@mocks/scatter_fill_self_next.json')); mock.data.forEach(function(trace) { trace.hoveron = 'fills'; }); Plotly.plot(createGraphDiv(), mock.data, mock.layout).then(function() { @@ -692,7 +692,7 @@ describe('hover on fill', function() { }); it('should work for scatterternary too', function(done) { - var mock = require('@mocks/ternary_fill.json'); + var mock = Lib.extendDeep({}, require('@mocks/ternary_fill.json')); mock.data.forEach(function(trace) { trace.hoveron = 'fills'; }); Plotly.plot(createGraphDiv(), mock.data, mock.layout).then(function() { From 1d1a0f7efa7543805f7a06ccf653b4148ecddb58 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Tue, 21 Jun 2016 21:37:17 +0200 Subject: [PATCH 14/16] lib.coerce: allow flaglist with no extras --- src/lib/coerce.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/coerce.js b/src/lib/coerce.js index c21a5410b7f..9c67cb32014 100644 --- a/src/lib/coerce.js +++ b/src/lib/coerce.js @@ -192,7 +192,7 @@ exports.valObjects = { propOut.set(dflt); return; } - if(opts.extras.indexOf(v) !== -1) { + if((opts.extras || []).indexOf(v) !== -1) { propOut.set(v); return; } From aec4c95181b86a9e39d942308c6b0bf9ae7c4a01 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Tue, 21 Jun 2016 21:37:46 +0200 Subject: [PATCH 15/16] make hoveron a flaglist with both modes enabled by default when there are points and area fills --- src/traces/scatter/attributes.js | 4 +- src/traces/scatter/defaults.js | 10 +-- src/traces/scatter/hover.js | 108 +++++++++++++------------ src/traces/scatterternary/defaults.js | 10 +-- src/traces/scatterternary/hover.js | 2 +- test/jasmine/tests/hover_label_test.js | 9 ++- 6 files changed, 76 insertions(+), 67 deletions(-) diff --git a/src/traces/scatter/attributes.js b/src/traces/scatter/attributes.js index 4e2bba6b271..28db0d1343e 100644 --- a/src/traces/scatter/attributes.js +++ b/src/traces/scatter/attributes.js @@ -93,8 +93,8 @@ module.exports = { ].join(' ') }, hoveron: { - valType: 'enumerated', - values: ['points', 'fills'], + valType: 'flaglist', + flags: ['points', 'fills'], role: 'info', description: [ 'Do the hover effects highlight individual points (markers or', diff --git a/src/traces/scatter/defaults.js b/src/traces/scatter/defaults.js index 5e8f42931f5..12c94bb8a03 100644 --- a/src/traces/scatter/defaults.js +++ b/src/traces/scatter/defaults.js @@ -53,11 +53,11 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout handleTextDefaults(traceIn, traceOut, layout, coerce); } - var dfltHoverOn = ''; + var dfltHoverOn = []; if(subTypes.hasMarkers(traceOut) || subTypes.hasText(traceOut)) { coerce('marker.maxdisplayed'); - dfltHoverOn = 'points'; + dfltHoverOn.push('points'); } coerce('fill'); @@ -66,10 +66,10 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout if(!subTypes.hasLines(traceOut)) handleLineShapeDefaults(traceIn, traceOut, coerce); } - if(!dfltHoverOn && (traceOut.fill === 'tonext' || traceOut.fill === 'toself')) { - dfltHoverOn = 'fills'; + if(traceOut.fill === 'tonext' || traceOut.fill === 'toself') { + dfltHoverOn.push('fills'); } - coerce('hoveron', dfltHoverOn || 'points'); + coerce('hoveron', dfltHoverOn.join('+') || 'points'); errorBarsSupplyDefaults(traceIn, traceOut, defaultColor, {axis: 'y'}); errorBarsSupplyDefaults(traceIn, traceOut, defaultColor, {axis: 'x', inherit: 'y'}); diff --git a/src/traces/scatter/hover.js b/src/traces/scatter/hover.js index 61ec08d0deb..373d68d9e3c 100644 --- a/src/traces/scatter/hover.js +++ b/src/traces/scatter/hover.js @@ -26,8 +26,63 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { ypx = ya.c2p(yval), pt = [xpx, ypx]; + // look for points to hover on first, then take fills only if we + // didn't find a point + if(trace.hoveron.indexOf('points') !== -1) { + var dx = function(di) { + // scatter points: d.mrc is the calculated marker radius + // adjust the distance so if you're inside the marker it + // always will show up regardless of point size, but + // prioritize smaller points + var rad = Math.max(3, di.mrc || 0); + return Math.max(Math.abs(xa.c2p(di.x) - xpx) - rad, 1 - 3 / rad); + }, + dy = function(di) { + var rad = Math.max(3, di.mrc || 0); + return Math.max(Math.abs(ya.c2p(di.y) - ypx) - rad, 1 - 3 / rad); + }, + dxy = function(di) { + var rad = Math.max(3, di.mrc || 0), + dx = xa.c2p(di.x) - xpx, + dy = ya.c2p(di.y) - ypx; + return Math.max(Math.sqrt(dx * dx + dy * dy) - rad, 1 - 3 / rad); + }, + distfn = Fx.getDistanceFunction(hovermode, dx, dy, dxy); + + Fx.getClosest(cd, distfn, pointData); + + // skip the rest (for this trace) if we didn't find a close point + if(pointData.index !== false) { + + // the closest data point + var di = cd[pointData.index], + xc = xa.c2p(di.x, true), + yc = ya.c2p(di.y, true), + rad = di.mrc || 1; + + Lib.extendFlat(pointData, { + color: getTraceColor(trace, di), + + x0: xc - rad, + x1: xc + rad, + xLabelVal: di.x, + + y0: yc - rad, + y1: yc + rad, + yLabelVal: di.y + }); + + if(di.tx) pointData.text = di.tx; + else if(trace.text) pointData.text = trace.text; + + ErrorBars.hoverInfo(di, trace, pointData); + + return [pointData]; + } + } + // even if hoveron is 'fills', only use it if we have polygons too - if(trace.hoveron === 'fills' && trace._polygons) { + if(trace.hoveron.indexOf('fills') !== -1 && trace._polygons) { var polygons = trace._polygons, polygonsIn = [], inside = false, @@ -100,55 +155,4 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { return [pointData]; } } - else { - var dx = function(di) { - // scatter points: d.mrc is the calculated marker radius - // adjust the distance so if you're inside the marker it - // always will show up regardless of point size, but - // prioritize smaller points - var rad = Math.max(3, di.mrc || 0); - return Math.max(Math.abs(xa.c2p(di.x) - xpx) - rad, 1 - 3 / rad); - }, - dy = function(di) { - var rad = Math.max(3, di.mrc || 0); - return Math.max(Math.abs(ya.c2p(di.y) - ypx) - rad, 1 - 3 / rad); - }, - dxy = function(di) { - var rad = Math.max(3, di.mrc || 0), - dx = xa.c2p(di.x) - xpx, - dy = ya.c2p(di.y) - ypx; - return Math.max(Math.sqrt(dx * dx + dy * dy) - rad, 1 - 3 / rad); - }, - distfn = Fx.getDistanceFunction(hovermode, dx, dy, dxy); - - Fx.getClosest(cd, distfn, pointData); - - // skip the rest (for this trace) if we didn't find a close point - if(pointData.index === false) return; - - // the closest data point - var di = cd[pointData.index], - xc = xa.c2p(di.x, true), - yc = ya.c2p(di.y, true), - rad = di.mrc || 1; - - Lib.extendFlat(pointData, { - color: getTraceColor(trace, di), - - x0: xc - rad, - x1: xc + rad, - xLabelVal: di.x, - - y0: yc - rad, - y1: yc + rad, - yLabelVal: di.y - }); - - if(di.tx) pointData.text = di.tx; - else if(trace.text) pointData.text = trace.text; - - ErrorBars.hoverInfo(di, trace, pointData); - - return [pointData]; - } }; diff --git a/src/traces/scatterternary/defaults.js b/src/traces/scatterternary/defaults.js index a3cd0cc2cb5..18bbcf039d3 100644 --- a/src/traces/scatterternary/defaults.js +++ b/src/traces/scatterternary/defaults.js @@ -81,11 +81,11 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout handleTextDefaults(traceIn, traceOut, layout, coerce); } - var dfltHoverOn = ''; + var dfltHoverOn = []; if(subTypes.hasMarkers(traceOut) || subTypes.hasText(traceOut)) { coerce('marker.maxdisplayed'); - dfltHoverOn = 'points'; + dfltHoverOn.push('points'); } coerce('fill'); @@ -96,8 +96,8 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout coerce('hoverinfo', (layout._dataLength === 1) ? 'a+b+c+text' : undefined); - if(!dfltHoverOn && (traceOut.fill === 'tonext' || traceOut.fill === 'toself')) { - dfltHoverOn = 'fills'; + if(traceOut.fill === 'tonext' || traceOut.fill === 'toself') { + dfltHoverOn.push('fills'); } - coerce('hoveron', dfltHoverOn || 'points'); + coerce('hoveron', dfltHoverOn.join('+') || 'points'); }; diff --git a/src/traces/scatterternary/hover.js b/src/traces/scatterternary/hover.js index e231bc39508..67de2f3b9b0 100644 --- a/src/traces/scatterternary/hover.js +++ b/src/traces/scatterternary/hover.js @@ -17,7 +17,7 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { var scatterPointData = scatterHover(pointData, xval, yval, hovermode); if(!scatterPointData || scatterPointData[0].index === false) return; - // if hoveron='fills', we don't show any point data so the label is + // if hovering on a fill, we don't show any point data so the label is // unchanged from what scatter gives us. if(scatterPointData[0].index === undefined) return scatterPointData; diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index f548fd64d2b..2b3951fb8cb 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -693,10 +693,15 @@ describe('hover on fill', function() { it('should work for scatterternary too', function(done) { var mock = Lib.extendDeep({}, require('@mocks/ternary_fill.json')); - mock.data.forEach(function(trace) { trace.hoveron = 'fills'; }); Plotly.plot(createGraphDiv(), mock.data, mock.layout).then(function() { - return assertLabelsCorrect([237, 163], [247.7, 166], 'trace 2'); + // hover over a point when that's closest, even if you're over + // a fill, because by default we have hoveron='points+fills' + return assertLabelsCorrect([237, 150], [240.0, 144], + 'trace 2Component A: 0.8Component B: 0.1Component C: 0.1'); + }).then(function() { + // the rest are hovers over fills + return assertLabelsCorrect([237, 170], [247.7, 166], 'trace 2'); }).then(function() { return assertLabelsCorrect([237, 218], [266.75, 265], 'trace 1'); }).then(function() { From 1e86eb230e54c79124a8999e5ac20f55268fe887 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Tue, 21 Jun 2016 22:46:16 +0200 Subject: [PATCH 16/16] update scatter(ternary) supplyDefaults test for hoveron flaglist --- test/jasmine/tests/scatter_test.js | 5 ++--- test/jasmine/tests/scatterternary_test.js | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/test/jasmine/tests/scatter_test.js b/test/jasmine/tests/scatter_test.js index f32fc407d77..cfc6e132353 100644 --- a/test/jasmine/tests/scatter_test.js +++ b/test/jasmine/tests/scatter_test.js @@ -68,13 +68,12 @@ describe('Test scatter', function() { fill: 'tonext' }; - // even with fill tonext, as long as there are markers or text - // you get points + // fills and markers, you get both hover types // you need visible: true here, as that normally gets set // outside of the module supplyDefaults traceOut = {visible: true}; supplyDefaults(traceIn, traceOut, defaultColor, layout); - expect(traceOut.hoveron).toBe('points'); + expect(traceOut.hoveron).toBe('points+fills'); // but with only lines (or just fill) and fill tonext or toself // you get fills diff --git a/test/jasmine/tests/scatterternary_test.js b/test/jasmine/tests/scatterternary_test.js index 2b840122f8c..21da09876f0 100644 --- a/test/jasmine/tests/scatterternary_test.js +++ b/test/jasmine/tests/scatterternary_test.js @@ -168,13 +168,12 @@ describe('scatterternary defaults', function() { fill: 'tonext' }; - // even with fill tonext, as long as there are markers or text - // you get points + // fills and markers, you get both hover types // you need visible: true here, as that normally gets set // outside of the module supplyDefaults traceOut = {visible: true}; supplyDefaults(traceIn, traceOut, defaultColor, layout); - expect(traceOut.hoveron).toBe('points'); + expect(traceOut.hoveron).toBe('points+fills'); // but with only lines (or just fill) and fill tonext or toself // you get fills