-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Hover / unhover events on the 2d WebGL plots #994
Changes from 2 commits
53c7c11
c4baeb2
359d50a
1e56d7f
2e8db74
9f77c11
2e54a99
57cec8c
a9b8b3e
c1abd3e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
var mouseEvent = require('./mouse_event'); | ||
|
||
module.exports = function hover(x, y) { | ||
mouseEvent('mousemove', x, y); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,146 @@ | ||
var Plotly = require('@lib/index'); | ||
var Lib = require('@src/lib'); | ||
|
||
var createGraphDiv = require('../assets/create_graph_div'); | ||
var destroyGraphDiv = require('../assets/destroy_graph_div'); | ||
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 hover = require('../assets/hover'); | ||
|
||
describe('Test click interactions:', function() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice tests. But unfortunately, the CircleCI runner still can't boot up Referencing #241 Previously, we ignored the gl test suites in the karma CI config, but more recently, I've been using this making it a little clearer that we're skipping a few tests on CI (I think). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @etpinard, I'll update with the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, planning to add |
||
var mock = require('@mocks/gl2d_14.json'); | ||
|
||
var mockCopy, gd; | ||
|
||
beforeAll(function() { | ||
jasmine.addMatchers(customMatchers); | ||
}); | ||
|
||
beforeEach(function() { | ||
gd = createGraphDiv(); | ||
mockCopy = Lib.extendDeep({}, mock); | ||
}); | ||
|
||
afterEach(destroyGraphDiv); | ||
|
||
describe('hover event is fired on hover', function() { | ||
var futureData; | ||
|
||
it('in general', function(done) { | ||
|
||
var modifiedMockCopy = Lib.extendDeep({}, mockCopy); | ||
|
||
Plotly.plot(gd, modifiedMockCopy.data, modifiedMockCopy.layout) | ||
|
||
.then(new Promise(function() { | ||
|
||
gd.on('plotly_hover', function(data) { | ||
futureData = data; | ||
}); | ||
|
||
hover(654.7712871743302, 316.97670766680994); | ||
|
||
window.setTimeout(function() { | ||
|
||
expect(futureData.points.length).toEqual(1); | ||
|
||
var pt = futureData.points[0]; | ||
|
||
expect(Object.keys(pt)).toEqual([ | ||
'trace', 'dataCoord', 'traceCoord', 'textLabel', 'color', | ||
'name', 'hoverinfo', 'screenCoord' | ||
]); | ||
|
||
expect(pt.traceCoord).toEqual([15.772, 0.387]); | ||
|
||
done(); | ||
}, 250); | ||
})); | ||
|
||
|
||
}); | ||
|
||
it('even when hoverinfo (== plotly tooltip) is set to none', function(done) { | ||
|
||
var modifiedMockCopy = Lib.extendDeep({}, mockCopy); | ||
modifiedMockCopy.data[0].hoverinfo = 'none'; | ||
|
||
Plotly.plot(gd, modifiedMockCopy.data, modifiedMockCopy.layout) | ||
|
||
.then(new Promise(function() { | ||
|
||
gd.on('plotly_hover', function(data) { | ||
futureData = data; | ||
}); | ||
|
||
hover(654.7712871743302, 316.97670766680994); | ||
|
||
window.setTimeout(function() { | ||
|
||
expect(futureData.points.length).toEqual(1); | ||
|
||
var pt = futureData.points[0]; | ||
|
||
expect(Object.keys(pt)).toEqual([ | ||
'trace', 'dataCoord', 'traceCoord', 'textLabel', 'color', | ||
'name', 'hoverinfo', 'screenCoord' | ||
]); | ||
|
||
expect(pt.traceCoord).toEqual([15.772, 0.387]); | ||
|
||
done(); | ||
}, 250); | ||
})); | ||
|
||
|
||
}); | ||
|
||
it('unhover happens', function(done) { | ||
|
||
var modifiedMockCopy = Lib.extendDeep({}, mockCopy); | ||
modifiedMockCopy.data[0].hoverinfo = 'none'; | ||
|
||
Plotly.plot(gd, modifiedMockCopy.data, modifiedMockCopy.layout) | ||
|
||
.then(new Promise(function() { | ||
|
||
futureData = undefined; | ||
|
||
gd.on('plotly_unhover', function() { | ||
futureData = 'emitted plotly_unhover'; | ||
}); | ||
|
||
hover(654.7712871743302, 316.97670766680994); | ||
|
||
// fairly realistic simulation of moving with the cursor | ||
window.setTimeout(function() { | ||
|
||
var x = 654, y = 316; // we start here | ||
var canceler = window.setInterval(function() { | ||
hover(x--, y++); // move the cursor | ||
}, 10); | ||
|
||
window.setTimeout(function() { | ||
window.clearInterval(canceler); // stop the mouse at some point | ||
}, 250); | ||
|
||
window.setTimeout(function() { | ||
|
||
expect(futureData).toEqual('emitted plotly_unhover'); | ||
|
||
done(); | ||
|
||
}, 250); | ||
|
||
}, 250); | ||
})); | ||
|
||
}); | ||
|
||
|
||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh nice. That looks pretty useful.
But I would prefer aiming for parity with cartesian event data as listed here for now.
Referencing #988
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about it but it didn't look like all the properties were available, so I went for what's there already. I asked your feedback on a feel that you might want something else :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@etpinard just to clarify, you want to be as close to the data contents of the event emission in the SVG version as possible, correct? E.g.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes exactly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@etpinard any advice on how to approach this? I just reviewed the code that needs to execute for this data to be available: around 350 lines in fx.hover
Do you think it's possible to make it work just by invoking it from within
scene2d
, or it's doable but maybe some parts of it need to be refactored / conditionalized, or I should perhaps extract out some useful small subset of it?It looks like quite a large piece of interdependent functionality, those hundreds of lines in that block are there for a reason, and I'd rather not waste time speculatively in the wrong direction if you perhaps have some conviction about which way to go. My wishful thinking is that it feels directly reusable to you without much change :-)
In the absence of this, maybe we can even break the task apart for quick mergeability, e.g. putting in the most essential data first, something similar to what we have now, and making a separate PR for data enrichment that's implemented as DRY as possible, perhaps with a scarily ambitious refactoring of
function hover() {...}
:-)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's definitively possible. That's how I got mapbox hover labels to work - see here.
But, I would call that outside the scope of this PR.
I think you already have all the info required to produce the same event data object within
scene2d
, correct?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all, thanks for pointing me to the Mapbox analogue!
Re your question, if you mean the availability of inputs to
Fx.hover
from withinscene2d.js
:-
gd
is availablesubplot
just equals to a constantxy
forscene2d.js
I believeevt
isn't available because the DOM mouse event capture is done deeper in thegl-vis
stack, but with some work in it probably I can expose it, or I can perhaps reconstruct neededevt
data as I already havetraceCoords
,screenCoords
, mouse button info etc. inscene2d.js
.Hopefully I didn't misunderstand your question. What I don't quite understand yet is how it's possible to generate the event data that you ask for (i.e. identical structure and content with the SVG charts) without doing something similar to what you did for Mapbox, because doing so seems to require the execution of most if not all logic in
fx.hover
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.. PS. if you meant to ask if I have all the details in
scene2d.js
to raise aplotly_hover
event in identical structure with the SVG counterparts, then the answer is, I'd need to cut and paste so much of what's infx.hover
already that I'd say no :-) i.e. doable and basic inputs are available but would lead to lots of duplication betweenscene2d.js
andgraph_interact.js
.