Skip to content

Hover / unhover events on the 2d WebGL plots #994

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

Merged
merged 10 commits into from
Oct 5, 2016
+571 −13
Merged

Hover / unhover events on the 2d WebGL plots #994

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
53c7c11
test
monfera Sep 28, 2016
c4baeb2
hover, unhover
monfera Sep 29, 2016
359d50a
adds click emission; adds test cases; uses hasWebGLSupport
monfera Sep 30, 2016
1e56d7f
PR feedback: enriching event callback data
monfera Oct 3, 2016
2e8db74
PR feedback: enriching event callback data - indices
monfera Oct 3, 2016
9f77c11
DRY up event emission
monfera Oct 3, 2016
2e54a99
PR feedback - cover the other gl2d plot types
monfera Oct 4, 2016
57cec8c
PR feedback - cover the other gl2d plot types - other plot type tests
monfera Oct 4, 2016
a9b8b3e
PR feedback - cover the other gl2d plot types - contourgl plot type test
monfera Oct 5, 2016
c1abd3e
PR feedback - return [x, y] only
monfera Oct 5, 2016
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
Prev Previous commit
Next Next commit
hover, unhover
  • Loading branch information
@monfera
monfera committed Sep 29, 2016
commit c4baeb2b3f80fd597e00b18cfacc445e1dba22a8
16 changes: 16 additions & 0 deletions src/plots/gl2d/scene2d.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,21 @@ proto.draw = function() {
glplot.pixelRatio
];

// this needs to happen before the next block that deletes traceCoord data
// also it's important to copy, otherwise data is lost by the time event data is read
this.graphDiv.emit('plotly_hover', {
points: [{
trace: nextSelection.trace,
dataCoord: nextSelection.dataCoord.slice(),
traceCoord: nextSelection.traceCoord.slice(),
textLabel: nextSelection.textLabel,
color: nextSelection.color,
name: nextSelection.name,
hoverinfo: nextSelection.hoverinfo,
screenCoord: nextSelection.screenCoord.slice()
Copy link
Contributor

@etpinard etpinard Sep 29, 2016 •
edited
Loading

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

Copy link
Contributor Author

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

Copy link
Contributor Author

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.
image

Copy link
Contributor

Choose a reason for hiding this comment

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

yes exactly.

Copy link
Contributor Author

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() {...} :-)

Copy link
Contributor

@etpinard etpinard Sep 30, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Do you think it's possible to make it work just by invoking it from within scene2d

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?

Copy link
Contributor Author

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 within scene2d.js:

-gd is available

  • subplot just equals to a constant xy for scene2d.js I believe
  • evt isn't available because the DOM mouse event capture is done deeper in the gl-vis stack, but with some work in it probably I can expose it, or I can perhaps reconstruct needed evt data as I already have traceCoords, screenCoords, mouse button info etc. in scene2d.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.

Copy link
Contributor Author

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 a plotly_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 in fx.hover already that I'd say no :-) i.e. doable and basic inputs are available but would lead to lots of duplication between scene2d.js and graph_interact.js.

}]
});

var hoverinfo = selection.hoverinfo;
if(hoverinfo !== 'all') {
var parts = hoverinfo.split('+');
Expand Down Expand Up @@ -549,6 +564,7 @@ proto.draw = function() {
else if(!result && this.lastPickResult) {
this.spikes.update({});
this.lastPickResult = null;
this.graphDiv.emit('plotly_unhover');
Fx.loneUnhover(this.svgContainer);
}
}
Expand Down
5 changes: 5 additions & 0 deletions test/jasmine/assets/hover.js
View file
Open in desktop
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);
};
Loading