Skip to content

Pass MouseEvent to the user #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 27 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
580d5fe
events: plotly_click, plotly_hover, plotly_unhover
n-riesco Mar 15, 2017
fe4239b
events: update tests
n-riesco Mar 15, 2017
7a90972
events: test modified clicks
n-riesco Mar 16, 2017
aea866e
events: do not use `new MouseEvent(evt)`
n-riesco Mar 16, 2017
e809b23
events: try-catch `new MouseEvent(type, evt)`
n-riesco Mar 16, 2017
2e6fb16
test: update helper function click
n-riesco Mar 17, 2017
550a257
events: test plotly_click in a pie plot
n-riesco Mar 17, 2017
d7e56cf
events: fix plotly_click in pie plots
n-riesco Mar 17, 2017
40dc522
events: test plotly_hover in pie plots
n-riesco Mar 17, 2017
85f72ff
events: add event to plotly_hover
n-riesco Mar 17, 2017
5f5bb9e
events: test plotly_unhover in pie plots
n-riesco Mar 17, 2017
8d5697b
events: add event to plotly_unhover
n-riesco Mar 17, 2017
9cf50c2
events: test plotly events in ternary plots
n-riesco Mar 17, 2017
27eb8c3
events: fix plotly events in geo plots
n-riesco Mar 17, 2017
3584bba
events: test plotly events in geo plots
n-riesco Mar 17, 2017
60ee153
events: fix plotly events in mapbox plots
n-riesco Mar 20, 2017
dcdda6f
events: test plotly events in mapbox plots
n-riesco Mar 20, 2017
194243d
events: define variable hasUserCalledHover
n-riesco Mar 20, 2017
20ff1a0
events: do not hard-code HOVERMINTIME
n-riesco Mar 20, 2017
3c41961
events: move geo tests to get_test.js
n-riesco Mar 20, 2017
00baba0
event: fix regression in pie interactions
n-riesco Mar 20, 2017
5cdf009
events: move pie tests to hover_pie_test.js
n-riesco Mar 20, 2017
76e382e
events: move ternary tests to ternary_test.js
n-riesco Mar 20, 2017
f679bbf
pie: test issue #902
n-riesco Mar 21, 2017
91cc175
pie: emit `plotly_hover` even if hoverinfo is none
n-riesco Mar 21, 2017
499c393
pie: test issue #1456
n-riesco Mar 21, 2017
9e10119
pie: Fix undefined trace in 'plotly_click'
n-riesco Mar 21, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions src/components/dragelement/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,22 @@ dragElement.init = function init(options) {
if(options.doneFn) options.doneFn(gd._dragged, numClicks, e);

if(!gd._dragged) {
var e2 = document.createEvent('MouseEvents');
e2.initEvent('click', true, true);
var e2;

try {
e2 = new MouseEvent('click', e);
}
catch(err) {
e2 = document.createEvent('MouseEvents');
e2.initMouseEvent('click',
e.bubbles, e.cancelable,
e.view, e.detail,
e.screenX, e.screenY,
e.clientX, e.clientY,
e.ctrlKey, e.altKey, e.shiftKey, e.metaKey,
e.button, e.relatedTarget);
}

initialTarget.dispatchEvent(e2);
}

Expand Down
5 changes: 4 additions & 1 deletion src/components/dragelement/unhover.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ unhover.raw = function unhoverRaw(gd, evt) {
gd._hoverdata = undefined;

if(evt.target && oldhoverdata) {
gd.emit('plotly_unhover', {points: oldhoverdata});
gd.emit('plotly_unhover', {
event: evt,
points: oldhoverdata
});
}
};
29 changes: 16 additions & 13 deletions src/plots/cartesian/graph_interact.js
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ fx.hover = function(gd, evt, subplot) {
function hover(gd, evt, subplot) {
if(subplot === 'pie') {
gd.emit('plotly_hover', {
event: evt.originalEvent,
points: [evt]
});
return;
Expand Down Expand Up @@ -413,12 +414,17 @@ function hover(gd, evt, subplot) {

// [x|y]px: the pixels (from top left) of the mouse location
// on the currently selected plot area
var xpx, ypx;
var hasUserCalledHover = ('xpx' in evt || 'ypx' in evt),
xpx, ypx;

// mouse event? ie is there a target element with
// clientX and clientY values?
if(evt.target && ('clientX' in evt) && ('clientY' in evt)) {
if(hasUserCalledHover) {
if('xpx' in evt) xpx = evt.xpx;
else xpx = xaArray[0]._length / 2;

if('ypx' in evt) ypx = evt.ypx;
else ypx = yaArray[0]._length / 2;
}
else {
// fire the beforehover event and quit if it returns false
// note that we're only calling this on real mouse events, so
// manual calls to fx.hover will always run.
Expand All @@ -437,13 +443,6 @@ function hover(gd, evt, subplot) {
return dragElement.unhoverRaw(gd, evt);
}
}
else {
if('xpx' in evt) xpx = evt.xpx;
else xpx = xaArray[0]._length / 2;

if('ypx' in evt) ypx = evt.ypx;
else ypx = yaArray[0]._length / 2;
}

if('xval' in evt) xvalArray = flat(subplots, evt.xval);
else xvalArray = p2c(xaArray, xpx);
Expand Down Expand Up @@ -624,10 +623,14 @@ function hover(gd, evt, subplot) {
if(!hoverChanged(gd, evt, oldhoverdata)) return;

if(oldhoverdata) {
gd.emit('plotly_unhover', { points: oldhoverdata });
gd.emit('plotly_unhover', {
event: evt,
points: oldhoverdata
});
}

gd.emit('plotly_hover', {
event: evt,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great. This will work for all subplots that call Fx.hover if I remember correctly that means: cartesian, ternary, geo and mapbox subplots. @n-riesco would you mind double checking?

Now, at the very least, we should make sure event is passed in pie chart hover data. Pie hover is a little different but shouldn't make it too hard to implement.

This leaves our gl2d, gl3d and parcoords (and polar but who cares about polar?). I'll be ok leaving them out of the first iteration. We should instead spent some time down the road merging their hover logic with Fx.hover.

Copy link
Owner Author

@n-riesco n-riesco Mar 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@etpinard It doesn't work for pie, ternary, geo or mapbox, because when they call to Fx.click(gd, evt) they make up an evt.

I reckon that to make it work for all these types of subplots, first I need to locate where they call Fx.click, Fx.hover and Fx.unhover and then see if it's possible to send the original event.

For example, here: why do we need to set the event to {target: true}? Is is OK if we just do Fx.click(gd, d3.event);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is is OK if we just do Fx.click(gd, d3.event);

That sounds ok to me, yes.

points: gd._hoverdata,
xaxes: xaArray,
yaxes: yaArray,
Expand Down Expand Up @@ -1350,7 +1353,7 @@ function hoverChanged(gd, evt, oldhoverdata) {
fx.click = function(gd, evt) {
var annotationsDone = Registry.getComponentMethod('annotations', 'onClick')(gd, gd._hoverdata);

function emitClick() { gd.emit('plotly_click', {points: gd._hoverdata}); }
function emitClick() { gd.emit('plotly_click', {points: gd._hoverdata, event: evt}); }

if(gd._hoverdata && evt && evt.target) {
if(annotationsDone && annotationsDone.then) {
Expand Down
10 changes: 4 additions & 6 deletions src/plots/geo/geo.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,9 @@ proto.plot = function(geoCalcData, fullLayout, promises) {

if(!lonlat || isNaN(lonlat[0]) || isNaN(lonlat[1])) return;

var evt = {
target: true,
xpx: mouse[0],
ypx: mouse[1]
};
var evt = d3.event;
evt.xpx = mouse[0];
evt.ypx = mouse[1];

_this.xaxis.c2p = function() { return mouse[0]; };
_this.xaxis.p2c = function() { return lonlat[0]; };
Expand All @@ -110,7 +108,7 @@ proto.plot = function(geoCalcData, fullLayout, promises) {
});

_this.framework.on('click', function() {
Fx.click(_this.graphDiv, { target: true });
Fx.click(_this.graphDiv, d3.event);
});

topojsonNameNew = topojsonUtils.getTopojsonName(geoLayout);
Expand Down
4 changes: 2 additions & 2 deletions src/plots/mapbox/mapbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,8 @@ proto.createMap = function(calcData, fullLayout, resolve, reject) {
Fx.hover(gd, evt, self.id);
});

map.on('click', function() {
Fx.click(gd, { target: true });
map.on('click', function(evt) {
Fx.click(gd, evt.originalEvent);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice here. Thanks!

});

function unhover() {
Expand Down
9 changes: 7 additions & 2 deletions src/traces/pie/plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ module.exports = function plot(gd, cdpie) {
hasHoverData = false;

function handleMouseOver(evt) {
evt.originalEvent = d3.event;

// in case fullLayout or fullData has changed without a replot
var fullLayout2 = gd._fullLayout,
trace2 = gd._fullData[trace.index],
Expand All @@ -97,6 +99,7 @@ module.exports = function plot(gd, cdpie) {
// or if hover is turned off
if(gd._dragging || fullLayout2.hovermode === false ||
hoverinfo === 'none' || hoverinfo === 'skip' || !hoverinfo) {
Fx.hover(gd, evt, 'pie');
return;
}

Expand Down Expand Up @@ -132,7 +135,9 @@ module.exports = function plot(gd, cdpie) {
}

function handleMouseOut(evt) {
evt.originalEvent = d3.event;
gd.emit('plotly_unhover', {
event: d3.event,
points: [evt]
});

Expand All @@ -144,8 +149,8 @@ module.exports = function plot(gd, cdpie) {

function handleClick() {
gd._hoverdata = [pt];
gd._hoverdata.trace = cd.trace;
Fx.click(gd, { target: true });
gd._hoverdata.trace = cd0.trace;
Fx.click(gd, d3.event);
}

slicePath.enter().append('path')
Expand Down
9 changes: 5 additions & 4 deletions test/jasmine/assets/click.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
var mouseEvent = require('./mouse_event');

module.exports = function click(x, y) {
mouseEvent('mousemove', x, y);
mouseEvent('mousedown', x, y);
mouseEvent('mouseup', x, y);
module.exports = function click(x, y, opts) {
mouseEvent('mousemove', x, y, opts);
mouseEvent('mousedown', x, y, opts);
mouseEvent('mouseup', x, y, opts);
mouseEvent('click', x, y, opts);
};
12 changes: 12 additions & 0 deletions test/jasmine/assets/mouse_event.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,18 @@ module.exports = function(type, x, y, opts) {
if(opts && opts.buttons) {
fullOpts.buttons = opts.buttons;
}
if(opts && opts.altKey) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Thanks!

fullOpts.altKey = opts.altKey;
}
if(opts && opts.ctrlKey) {
fullOpts.ctrlKey = opts.ctrlKey;
}
if(opts && opts.metaKey) {
fullOpts.metaKey = opts.metaKey;
}
if(opts && opts.shiftKey) {
fullOpts.shiftKey = opts.shiftKey;
}

var el = (opts && opts.element) || document.elementFromPoint(x, y),
ev;
Expand Down
127 changes: 114 additions & 13 deletions test/jasmine/tests/click_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,30 @@ var customMatchers = require('../assets/custom_matchers');
var click = require('../assets/click');
var doubleClickRaw = require('../assets/double_click');

function move(fromX, fromY, toX, toY, delay) {
return new Promise(function(resolve) {
mouseEvent('mousemove', fromX, fromY);

setTimeout(function() {
mouseEvent('mousemove', toX, toY);
resolve();
}, delay || DBLCLICKDELAY / 4);
});
}

function drag(fromX, fromY, toX, toY, delay) {
return new Promise(function(resolve) {
mouseEvent('mousemove', fromX, fromY);
mouseEvent('mousedown', fromX, fromY);
mouseEvent('mousemove', toX, toY);

setTimeout(function() {
mouseEvent('mouseup', toX, toY);
resolve();
}, delay || DBLCLICKDELAY / 4);
});
}


describe('Test click interactions:', function() {
var mock = require('@mocks/14.json');
Expand All @@ -39,19 +63,6 @@ describe('Test click interactions:', function() {

afterEach(destroyGraphDiv);

function drag(fromX, fromY, toX, toY, delay) {
return new Promise(function(resolve) {
mouseEvent('mousemove', fromX, fromY);
mouseEvent('mousedown', fromX, fromY);
mouseEvent('mousemove', toX, toY);

setTimeout(function() {
mouseEvent('mouseup', toX, toY);
resolve();
}, delay || DBLCLICKDELAY / 4);
});
}

function doubleClick(x, y) {
return doubleClickRaw(x, y).then(function() {
return Plotly.Plots.previousPromises(gd);
Expand Down Expand Up @@ -87,6 +98,55 @@ describe('Test click interactions:', function() {
expect(pt.pointNumber).toEqual(11);
expect(pt.x).toEqual(0.125);
expect(pt.y).toEqual(2.125);

var evt = futureData.event;
expect(evt.clientX).toEqual(pointPos[0]);
expect(evt.clientY).toEqual(pointPos[1]);
});
});

describe('modified click events', function() {
var clickOpts = {
altKey: true,
ctrlKey: true,
metaKey: true,
shiftKey: true
},
futureData;

beforeEach(function(done) {
Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(done);

gd.on('plotly_click', function(data) {
futureData = data;
});
});

it('should not be trigged when not on data points', function() {
click(blankPos[0], blankPos[1], clickOpts);
expect(futureData).toBe(undefined);
});

it('should contain the correct fields', function() {
click(pointPos[0], pointPos[1], clickOpts);
expect(futureData.points.length).toEqual(1);

var pt = futureData.points[0];
expect(Object.keys(pt)).toEqual([
'data', 'fullData', 'curveNumber', 'pointNumber',
'x', 'y', 'xaxis', 'yaxis'
]);
expect(pt.curveNumber).toEqual(0);
expect(pt.pointNumber).toEqual(11);
expect(pt.x).toEqual(0.125);
expect(pt.y).toEqual(2.125);

var evt = futureData.event;
expect(evt.clientX).toEqual(pointPos[0]);
expect(evt.clientY).toEqual(pointPos[1]);
Object.getOwnPropertyNames(clickOpts).forEach(function(opt) {
expect(evt[opt]).toEqual(clickOpts[opt], opt);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

});
});
});

Expand Down Expand Up @@ -191,6 +251,46 @@ describe('Test click interactions:', function() {
expect(pt.pointNumber).toEqual(11);
expect(pt.x).toEqual(0.125);
expect(pt.y).toEqual(2.125);

var evt = futureData.event;
expect(evt.clientX).toEqual(pointPos[0]);
expect(evt.clientY).toEqual(pointPos[1]);
Copy link

@etpinard etpinard Mar 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could a test case that has a ctrlKey pressed? ... to really 🔒 down the requirement,

});
});

describe('plotly_unhover event with hoverinfo set to none', function() {
var futureData;

beforeEach(function(done) {

var modifiedMockCopy = Lib.extendDeep({}, mockCopy);
modifiedMockCopy.data[0].hoverinfo = 'none';
Plotly.plot(gd, modifiedMockCopy.data, modifiedMockCopy.layout)
.then(done);

gd.on('plotly_unhover', function(data) {
futureData = data;
});
});

it('should contain the correct fields despite hoverinfo: "none"', function(done) {
move(pointPos[0], pointPos[1], blankPos[0], blankPos[1]).then(function() {
expect(futureData.points.length).toEqual(1);

var pt = futureData.points[0];
expect(Object.keys(pt)).toEqual([
'data', 'fullData', 'curveNumber', 'pointNumber',
'x', 'y', 'xaxis', 'yaxis'
]);
expect(pt.curveNumber).toEqual(0);
expect(pt.pointNumber).toEqual(11);
expect(pt.x).toEqual(0.125);
expect(pt.y).toEqual(2.125);

var evt = futureData.event;
expect(evt.clientX).toEqual(blankPos[0]);
expect(evt.clientY).toEqual(blankPos[1]);
}).then(done);
});
});

Expand Down Expand Up @@ -817,6 +917,7 @@ describe('Test click interactions:', function() {
});
});


describe('dragbox', function() {

afterEach(destroyGraphDiv);
Expand Down
Loading