diff --git a/src/plots/ternary/ternary.js b/src/plots/ternary/ternary.js index 96d7587f275..1fd6d7094f7 100644 --- a/src/plots/ternary/ternary.js +++ b/src/plots/ternary/ternary.js @@ -659,9 +659,9 @@ proto.initInteractions = function() { Plotly.relayout(gd, attrs); } - dragElement.init(dragOptions); - // finally, set up hover and click + // these event handlers must already be set before dragElement.init + // so it can stash them and override them. dragger.onmousemove = function(evt) { fx.hover(gd, evt, _this.id); gd._fullLayout._lasthover = dragger; @@ -677,6 +677,8 @@ proto.initInteractions = function() { dragger.onclick = function(evt) { fx.click(gd, evt); }; + + dragElement.init(dragOptions); }; function removeZoombox(gd) { diff --git a/src/traces/scatter/hover.js b/src/traces/scatter/hover.js index 373d68d9e3c..cb23c268fa1 100644 --- a/src/traces/scatter/hover.js +++ b/src/traces/scatter/hover.js @@ -107,6 +107,11 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { } if(inside) { + // constrain ymin/max to the visible plot, so the label goes + // at the middle of the piece you can see + ymin = Math.max(ymin, 0); + ymax = Math.min(ymax, ya._length); + // 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. @@ -128,6 +133,10 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { } } + // constrain xmin/max to the visible plot now too + xmin = Math.max(xmin, 0); + xmax = Math.min(xmax, xa._length); + // get only fill or line color for the hover color var color = Color.defaultLine; if(Color.opacity(trace.fillcolor)) color = trace.fillcolor; diff --git a/src/traces/scatterternary/hover.js b/src/traces/scatterternary/hover.js index 67de2f3b9b0..176786ed050 100644 --- a/src/traces/scatterternary/hover.js +++ b/src/traces/scatterternary/hover.js @@ -17,12 +17,29 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { var scatterPointData = scatterHover(pointData, xval, yval, hovermode); if(!scatterPointData || scatterPointData[0].index === false) return; + var newPointData = scatterPointData[0]; + // 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; + // unchanged from what scatter gives us - except that it needs to + // be constrained to the trianglular plot area, not just the rectangular + // area defined by the synthetic x and y axes + // TODO: in some cases the vertical middle of the shape is not within + // the triangular viewport at all, so the label can become disconnected + // from the shape entirely. But calculating what portion of the shape + // is actually visible, as constrained by the diagonal axis lines, is not + // so easy and anyway we lost the information we would have needed to do + // this inside scatterHover. + if(newPointData.index === undefined) { + var yFracUp = 1 - (newPointData.y0 / pointData.ya._length), + xLen = pointData.xa._length, + xMin = xLen * yFracUp / 2, + xMax = xLen - xMin; + newPointData.x0 = Math.max(Math.min(newPointData.x0, xMax), xMin); + newPointData.x1 = Math.max(Math.min(newPointData.x1, xMax), xMin); + return scatterPointData; + } - var newPointData = scatterPointData[0], - cdi = newPointData.cd[newPointData.index]; + var cdi = newPointData.cd[newPointData.index]; newPointData.a = cdi.a; newPointData.b = cdi.b; diff --git a/test/jasmine/assets/click.js b/test/jasmine/assets/click.js new file mode 100644 index 00000000000..e2cc43e444b --- /dev/null +++ b/test/jasmine/assets/click.js @@ -0,0 +1,7 @@ +var mouseEvent = require('./mouse_event'); + +module.exports = function click(x, y) { + mouseEvent('mousemove', x, y); + mouseEvent('mousedown', x, y); + mouseEvent('mouseup', x, y); +}; diff --git a/test/jasmine/assets/double_click.js b/test/jasmine/assets/double_click.js new file mode 100644 index 00000000000..e92375588a8 --- /dev/null +++ b/test/jasmine/assets/double_click.js @@ -0,0 +1,13 @@ +var click = require('./click'); +var DBLCLICKDELAY = require('@src/plots/cartesian/constants').DBLCLICKDELAY; + +module.exports = function doubleClick(x, y) { + return new Promise(function(resolve) { + click(x, y); + + setTimeout(function() { + click(x, y); + setTimeout(function() { resolve(); }, DBLCLICKDELAY / 2); + }, DBLCLICKDELAY / 2); + }); +}; diff --git a/test/jasmine/tests/click_test.js b/test/jasmine/tests/click_test.js index c08b56ea711..8dd517cdd15 100644 --- a/test/jasmine/tests/click_test.js +++ b/test/jasmine/tests/click_test.js @@ -8,6 +8,12 @@ var mouseEvent = require('../assets/mouse_event'); var getRectCenter = require('../assets/get_rect_center'); var customMatchers = require('../assets/custom_matchers'); +// cartesian click events events use the hover data +// from the mousemove events and then simulate +// a click event on mouseup +var click = require('../assets/click'); +var doubleClick = require('../assets/double_click'); + describe('Test click interactions:', function() { var mock = require('@mocks/14.json'); @@ -31,26 +37,6 @@ describe('Test click interactions:', function() { afterEach(destroyGraphDiv); - // cartesian click events events use the hover data - // from the mousemove events and then simulate - // a click event on mouseup - function click(x, y) { - mouseEvent('mousemove', x, y); - mouseEvent('mousedown', x, y); - mouseEvent('mouseup', x, y); - } - - function doubleClick(x, y) { - return new Promise(function(resolve) { - click(x, y); - - setTimeout(function() { - click(x, y); - setTimeout(function() { resolve(); }, DBLCLICKDELAY / 2); - }, DBLCLICKDELAY / 2); - }); - } - function drag(fromX, fromY, toX, toY, delay) { return new Promise(function(resolve) { mouseEvent('mousemove', fromX, fromY); diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index 2b3951fb8cb..8f4c71e0eae 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -8,6 +8,7 @@ var Lib = require('@src/lib'); var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); var mouseEvent = require('../assets/mouse_event'); +var doubleClick = require('../assets/double_click'); describe('hover info', function() { 'use strict'; @@ -693,8 +694,9 @@ describe('hover on fill', function() { it('should work for scatterternary too', function(done) { var mock = Lib.extendDeep({}, require('@mocks/ternary_fill.json')); + var gd = createGraphDiv(); - Plotly.plot(createGraphDiv(), mock.data, mock.layout).then(function() { + Plotly.plot(gd, mock.data, mock.layout).then(function() { // 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], @@ -706,6 +708,22 @@ describe('hover on fill', function() { return assertLabelsCorrect([237, 218], [266.75, 265], 'trace 1'); }).then(function() { return assertLabelsCorrect([237, 251], [247.7, 254], 'trace 0'); + }).then(function() { + // zoom in to test clipping of large out-of-viewport shapes + return Plotly.relayout(gd, { + 'ternary.aaxis.min': 0.5, + 'ternary.baxis.min': 0.25 + }); + }).then(function() { + // this particular one has a hover label disconnected from the shape itself + // so if we ever fix this, the test will have to be fixed too. + return assertLabelsCorrect([295, 218], [275.1, 166], 'trace 2'); + }).then(function() { + // trigger an autoscale redraw, which goes through dragElement + return doubleClick(237, 251); + }).then(function() { + // then make sure we can still select a *different* item afterward + return assertLabelsCorrect([237, 218], [266.75, 265], 'trace 1'); }).then(done); }); }); diff --git a/test/jasmine/tests/select_test.js b/test/jasmine/tests/select_test.js index a4fd9bb4997..885d993afb3 100644 --- a/test/jasmine/tests/select_test.js +++ b/test/jasmine/tests/select_test.js @@ -2,7 +2,7 @@ var d3 = require('d3'); var Plotly = require('@lib/index'); var Lib = require('@src/lib'); -var DBLCLICKDELAY = require('@src/plots/cartesian/constants').DBLCLICKDELAY; +var doubleClick = require('../assets/double_click'); var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); @@ -35,23 +35,6 @@ describe('select box and lasso', function() { mouseEvent('mouseup', path[len - 1][0], path[len - 1][1]); } - // cartesian click events events use the hover data - // from the mousemove events and then simulate - // a click event on mouseup - function click(x, y) { - mouseEvent('mousemove', x, y); - mouseEvent('mousedown', x, y); - mouseEvent('mouseup', x, y); - } - - function doubleClick(x, y, cb) { - click(x, y); - setTimeout(function() { - click(x, y); - cb(); - }, DBLCLICKDELAY / 2); - } - function assertRange(actual, expected) { var PRECISION = 4; @@ -104,7 +87,7 @@ describe('select box and lasso', function() { drag([[x0, y0], [x1, y1]]); - doubleClick(x2, y2, done); + doubleClick(x2, y2).then(done); }); }); @@ -153,7 +136,7 @@ describe('select box and lasso', function() { drag([[x0, y0], [x1, y1]]); - doubleClick(x2, y2, done); + doubleClick(x2, y2).then(done); }); }); @@ -225,7 +208,7 @@ describe('select box and lasso', function() { y: [0.10209191961595454, 24.512223978291406] }, 'with the correct selected range'); - doubleClick(250, 200, function() { + doubleClick(250, 200).then(function() { expect(doubleClickData).toBe(null, 'with the correct deselect data'); done(); }); @@ -283,7 +266,7 @@ describe('select box and lasso', function() { y: 2.75 }], 'with the correct selected points'); - doubleClick(250, 200, function() { + doubleClick(250, 200).then(function() { expect(doubleClickData).toBe(null, 'with the correct deselect data'); done(); }); diff --git a/test/jasmine/tests/ternary_test.js b/test/jasmine/tests/ternary_test.js index c00df866914..c563a500b1b 100644 --- a/test/jasmine/tests/ternary_test.js +++ b/test/jasmine/tests/ternary_test.js @@ -1,6 +1,5 @@ var Plotly = require('@lib'); var Lib = require('@src/lib'); -var DBLCLICKDELAY = require('@src/plots/cartesian/constants').DBLCLICKDELAY; var supplyLayoutDefaults = require('@src/plots/ternary/layout/defaults'); @@ -8,6 +7,8 @@ var d3 = require('d3'); var createGraphDiv = require('../assets/create_graph_div'); 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 customMatchers = require('../assets/custom_matchers'); @@ -238,23 +239,6 @@ describe('ternary plots', function() { return d3.select('.hoverlayer').selectAll('g'); } - function click(x, y) { - mouseEvent('mousemove', x, y); - mouseEvent('mousedown', x, y); - mouseEvent('mouseup', x, y); - } - - function doubleClick(x, y) { - return new Promise(function(resolve) { - click(x, y); - - setTimeout(function() { - click(x, y); - resolve(); - }, DBLCLICKDELAY / 2); - }); - } - function drag(path) { var len = path.length;