Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit a19cf2f

Browse filesBrowse files
alexcjohnsonalexcjohnson
authored
Merge pull request plotly#688 from plotly/fix-ternary-hover-after-zoom
fix ternary hover after zoom/pan events
2 parents 9675483 + a0a12d8 commit a19cf2f
Copy full SHA for a19cf2f

File tree

Expand file treeCollapse file tree

9 files changed

+86
-67
lines changed
Filter options
Expand file treeCollapse file tree

9 files changed

+86
-67
lines changed

‎src/plots/ternary/ternary.js

Copy file name to clipboardExpand all lines: src/plots/ternary/ternary.js
+4-2Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -659,9 +659,9 @@ proto.initInteractions = function() {
659659
Plotly.relayout(gd, attrs);
660660
}
661661

662-
dragElement.init(dragOptions);
663-
664662
// finally, set up hover and click
663+
// these event handlers must already be set before dragElement.init
664+
// so it can stash them and override them.
665665
dragger.onmousemove = function(evt) {
666666
fx.hover(gd, evt, _this.id);
667667
gd._fullLayout._lasthover = dragger;
@@ -677,6 +677,8 @@ proto.initInteractions = function() {
677677
dragger.onclick = function(evt) {
678678
fx.click(gd, evt);
679679
};
680+
681+
dragElement.init(dragOptions);
680682
};
681683

682684
function removeZoombox(gd) {

‎src/traces/scatter/hover.js

Copy file name to clipboardExpand all lines: src/traces/scatter/hover.js
+9Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,11 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
107107
}
108108

109109
if(inside) {
110+
// constrain ymin/max to the visible plot, so the label goes
111+
// at the middle of the piece you can see
112+
ymin = Math.max(ymin, 0);
113+
ymax = Math.min(ymax, ya._length);
114+
110115
// find the overall left-most and right-most points of the
111116
// polygon(s) we're inside at their combined vertical midpoint.
112117
// This is where we will draw the hover label.
@@ -128,6 +133,10 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
128133
}
129134
}
130135

136+
// constrain xmin/max to the visible plot now too
137+
xmin = Math.max(xmin, 0);
138+
xmax = Math.min(xmax, xa._length);
139+
131140
// get only fill or line color for the hover color
132141
var color = Color.defaultLine;
133142
if(Color.opacity(trace.fillcolor)) color = trace.fillcolor;

‎src/traces/scatterternary/hover.js

Copy file name to clipboardExpand all lines: src/traces/scatterternary/hover.js
+21-4Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,29 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
1717
var scatterPointData = scatterHover(pointData, xval, yval, hovermode);
1818
if(!scatterPointData || scatterPointData[0].index === false) return;
1919

20+
var newPointData = scatterPointData[0];
21+
2022
// if hovering on a fill, we don't show any point data so the label is
21-
// unchanged from what scatter gives us.
22-
if(scatterPointData[0].index === undefined) return scatterPointData;
23+
// unchanged from what scatter gives us - except that it needs to
24+
// be constrained to the trianglular plot area, not just the rectangular
25+
// area defined by the synthetic x and y axes
26+
// TODO: in some cases the vertical middle of the shape is not within
27+
// the triangular viewport at all, so the label can become disconnected
28+
// from the shape entirely. But calculating what portion of the shape
29+
// is actually visible, as constrained by the diagonal axis lines, is not
30+
// so easy and anyway we lost the information we would have needed to do
31+
// this inside scatterHover.
32+
if(newPointData.index === undefined) {
33+
var yFracUp = 1 - (newPointData.y0 / pointData.ya._length),
34+
xLen = pointData.xa._length,
35+
xMin = xLen * yFracUp / 2,
36+
xMax = xLen - xMin;
37+
newPointData.x0 = Math.max(Math.min(newPointData.x0, xMax), xMin);
38+
newPointData.x1 = Math.max(Math.min(newPointData.x1, xMax), xMin);
39+
return scatterPointData;
40+
}
2341

24-
var newPointData = scatterPointData[0],
25-
cdi = newPointData.cd[newPointData.index];
42+
var cdi = newPointData.cd[newPointData.index];
2643

2744
newPointData.a = cdi.a;
2845
newPointData.b = cdi.b;

‎test/jasmine/assets/click.js

Copy file name to clipboard
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
var mouseEvent = require('./mouse_event');
2+
3+
module.exports = function click(x, y) {
4+
mouseEvent('mousemove', x, y);
5+
mouseEvent('mousedown', x, y);
6+
mouseEvent('mouseup', x, y);
7+
};

‎test/jasmine/assets/double_click.js

Copy file name to clipboard
+13Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
var click = require('./click');
2+
var DBLCLICKDELAY = require('@src/plots/cartesian/constants').DBLCLICKDELAY;
3+
4+
module.exports = function doubleClick(x, y) {
5+
return new Promise(function(resolve) {
6+
click(x, y);
7+
8+
setTimeout(function() {
9+
click(x, y);
10+
setTimeout(function() { resolve(); }, DBLCLICKDELAY / 2);
11+
}, DBLCLICKDELAY / 2);
12+
});
13+
};

‎test/jasmine/tests/click_test.js

Copy file name to clipboardExpand all lines: test/jasmine/tests/click_test.js
+6-20Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@ var mouseEvent = require('../assets/mouse_event');
88
var getRectCenter = require('../assets/get_rect_center');
99
var customMatchers = require('../assets/custom_matchers');
1010

11+
// cartesian click events events use the hover data
12+
// from the mousemove events and then simulate
13+
// a click event on mouseup
14+
var click = require('../assets/click');
15+
var doubleClick = require('../assets/double_click');
16+
1117

1218
describe('Test click interactions:', function() {
1319
var mock = require('@mocks/14.json');
@@ -31,26 +37,6 @@ describe('Test click interactions:', function() {
3137

3238
afterEach(destroyGraphDiv);
3339

34-
// cartesian click events events use the hover data
35-
// from the mousemove events and then simulate
36-
// a click event on mouseup
37-
function click(x, y) {
38-
mouseEvent('mousemove', x, y);
39-
mouseEvent('mousedown', x, y);
40-
mouseEvent('mouseup', x, y);
41-
}
42-
43-
function doubleClick(x, y) {
44-
return new Promise(function(resolve) {
45-
click(x, y);
46-
47-
setTimeout(function() {
48-
click(x, y);
49-
setTimeout(function() { resolve(); }, DBLCLICKDELAY / 2);
50-
}, DBLCLICKDELAY / 2);
51-
});
52-
}
53-
5440
function drag(fromX, fromY, toX, toY, delay) {
5541
return new Promise(function(resolve) {
5642
mouseEvent('mousemove', fromX, fromY);

‎test/jasmine/tests/hover_label_test.js

Copy file name to clipboardExpand all lines: test/jasmine/tests/hover_label_test.js
+19-1Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ var Lib = require('@src/lib');
88
var createGraphDiv = require('../assets/create_graph_div');
99
var destroyGraphDiv = require('../assets/destroy_graph_div');
1010
var mouseEvent = require('../assets/mouse_event');
11+
var doubleClick = require('../assets/double_click');
1112

1213
describe('hover info', function() {
1314
'use strict';
@@ -693,8 +694,9 @@ describe('hover on fill', function() {
693694

694695
it('should work for scatterternary too', function(done) {
695696
var mock = Lib.extendDeep({}, require('@mocks/ternary_fill.json'));
697+
var gd = createGraphDiv();
696698

697-
Plotly.plot(createGraphDiv(), mock.data, mock.layout).then(function() {
699+
Plotly.plot(gd, mock.data, mock.layout).then(function() {
698700
// hover over a point when that's closest, even if you're over
699701
// a fill, because by default we have hoveron='points+fills'
700702
return assertLabelsCorrect([237, 150], [240.0, 144],
@@ -706,6 +708,22 @@ describe('hover on fill', function() {
706708
return assertLabelsCorrect([237, 218], [266.75, 265], 'trace 1');
707709
}).then(function() {
708710
return assertLabelsCorrect([237, 251], [247.7, 254], 'trace 0');
711+
}).then(function() {
712+
// zoom in to test clipping of large out-of-viewport shapes
713+
return Plotly.relayout(gd, {
714+
'ternary.aaxis.min': 0.5,
715+
'ternary.baxis.min': 0.25
716+
});
717+
}).then(function() {
718+
// this particular one has a hover label disconnected from the shape itself
719+
// so if we ever fix this, the test will have to be fixed too.
720+
return assertLabelsCorrect([295, 218], [275.1, 166], 'trace 2');
721+
}).then(function() {
722+
// trigger an autoscale redraw, which goes through dragElement
723+
return doubleClick(237, 251);
724+
}).then(function() {
725+
// then make sure we can still select a *different* item afterward
726+
return assertLabelsCorrect([237, 218], [266.75, 265], 'trace 1');
709727
}).then(done);
710728
});
711729
});

‎test/jasmine/tests/select_test.js

Copy file name to clipboardExpand all lines: test/jasmine/tests/select_test.js
+5-22Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ var d3 = require('d3');
22

33
var Plotly = require('@lib/index');
44
var Lib = require('@src/lib');
5-
var DBLCLICKDELAY = require('@src/plots/cartesian/constants').DBLCLICKDELAY;
5+
var doubleClick = require('../assets/double_click');
66

77
var createGraphDiv = require('../assets/create_graph_div');
88
var destroyGraphDiv = require('../assets/destroy_graph_div');
@@ -35,23 +35,6 @@ describe('select box and lasso', function() {
3535
mouseEvent('mouseup', path[len - 1][0], path[len - 1][1]);
3636
}
3737

38-
// cartesian click events events use the hover data
39-
// from the mousemove events and then simulate
40-
// a click event on mouseup
41-
function click(x, y) {
42-
mouseEvent('mousemove', x, y);
43-
mouseEvent('mousedown', x, y);
44-
mouseEvent('mouseup', x, y);
45-
}
46-
47-
function doubleClick(x, y, cb) {
48-
click(x, y);
49-
setTimeout(function() {
50-
click(x, y);
51-
cb();
52-
}, DBLCLICKDELAY / 2);
53-
}
54-
5538
function assertRange(actual, expected) {
5639
var PRECISION = 4;
5740

@@ -104,7 +87,7 @@ describe('select box and lasso', function() {
10487

10588
drag([[x0, y0], [x1, y1]]);
10689

107-
doubleClick(x2, y2, done);
90+
doubleClick(x2, y2).then(done);
10891
});
10992
});
11093

@@ -153,7 +136,7 @@ describe('select box and lasso', function() {
153136

154137
drag([[x0, y0], [x1, y1]]);
155138

156-
doubleClick(x2, y2, done);
139+
doubleClick(x2, y2).then(done);
157140
});
158141
});
159142

@@ -225,7 +208,7 @@ describe('select box and lasso', function() {
225208
y: [0.10209191961595454, 24.512223978291406]
226209
}, 'with the correct selected range');
227210

228-
doubleClick(250, 200, function() {
211+
doubleClick(250, 200).then(function() {
229212
expect(doubleClickData).toBe(null, 'with the correct deselect data');
230213
done();
231214
});
@@ -283,7 +266,7 @@ describe('select box and lasso', function() {
283266
y: 2.75
284267
}], 'with the correct selected points');
285268

286-
doubleClick(250, 200, function() {
269+
doubleClick(250, 200).then(function() {
287270
expect(doubleClickData).toBe(null, 'with the correct deselect data');
288271
done();
289272
});

‎test/jasmine/tests/ternary_test.js

Copy file name to clipboardExpand all lines: test/jasmine/tests/ternary_test.js
+2-18Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
var Plotly = require('@lib');
22
var Lib = require('@src/lib');
3-
var DBLCLICKDELAY = require('@src/plots/cartesian/constants').DBLCLICKDELAY;
43

54
var supplyLayoutDefaults = require('@src/plots/ternary/layout/defaults');
65

76
var d3 = require('d3');
87
var createGraphDiv = require('../assets/create_graph_div');
98
var destroyGraphDiv = require('../assets/destroy_graph_div');
109
var mouseEvent = require('../assets/mouse_event');
10+
var click = require('../assets/click');
11+
var doubleClick = require('../assets/double_click');
1112
var customMatchers = require('../assets/custom_matchers');
1213

1314

@@ -238,23 +239,6 @@ describe('ternary plots', function() {
238239
return d3.select('.hoverlayer').selectAll('g');
239240
}
240241

241-
function click(x, y) {
242-
mouseEvent('mousemove', x, y);
243-
mouseEvent('mousedown', x, y);
244-
mouseEvent('mouseup', x, y);
245-
}
246-
247-
function doubleClick(x, y) {
248-
return new Promise(function(resolve) {
249-
click(x, y);
250-
251-
setTimeout(function() {
252-
click(x, y);
253-
resolve();
254-
}, DBLCLICKDELAY / 2);
255-
});
256-
}
257-
258242
function drag(path) {
259243
var len = path.length;
260244

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.