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.

Sign up for GitHub

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

Pass MouseEvent to the user #9

Show file tree
Hide file tree
Changes from 2 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
3 changes: 1 addition & 2 deletions src/components/dragelement/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,7 @@ 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 = new MouseEvent('click', e);
Copy link

Choose a reason for hiding this comment

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

Oh. I think that will break click in IE9 and even up to IE11.

I'd be ok with something like:

var e2

try {
  e2 = new MouseEvent('click', e);
} catch(e) {
  e2  = document.createEvent('MouseEvents');
  e2.initEvent('click', true, true);
}

and hence only passing MouseEvent in browsers that supports it.

Copy link

Choose a reason for hiding this comment

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

@n-riesco what do you think?

Copy link
Owner Author

Choose a reason for hiding this comment

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

@etpinard How about initMouseEvent?

MDN
MSDN

initialTarget.dispatchEvent(e2);
}

Expand Down
5 changes: 4 additions & 1 deletion src/components/dragelement/unhover.js
View file
Open in desktop
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
});
}
};
8 changes: 6 additions & 2 deletions src/plots/cartesian/graph_interact.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,10 +624,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,
Copy link

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 •
edited
Loading

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);

Copy link

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 +1354,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
55 changes: 55 additions & 0 deletions test/jasmine/tests/click_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,17 @@ describe('Test click interactions:', function() {

afterEach(destroyGraphDiv);

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);
Expand Down Expand Up @@ -87,6 +98,10 @@ 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]);
});
});

Expand Down Expand Up @@ -191,6 +206,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 •
edited
Loading

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