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

monfera · 2016-09-29T13:51:00Z

Covers plotly_hover, plotly_unhover, plotly_click.

#743
Also: FI-71
Partial coverage: #130

etpinard

@monfera this is looking great. Thanks for doing into this!

Are you planning on adding plotly_click event in this PR too?

etpinard · 2016-09-29T14:48:02Z

src/plots/gl2d/scene2d.js

+ color: nextSelection.color,
+ name: nextSelection.name,
+ hoverinfo: nextSelection.hoverinfo,
+ screenCoord: nextSelection.screenCoord.slice()


Oh nice. That looks pretty useful.

But I would prefer aiming for parity with cartesian event data as listed here for now.

Referencing #988

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

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

yes exactly.

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

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?

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.

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

etpinard · 2016-09-29T14:56:12Z

test/jasmine/tests/gl2d_click_test.js

+// var click = require('../assets/click');
+var hover = require('../assets/hover');
+
+describe('Test click interactions:', function() {


Nice tests. But unfortunately, the CircleCI runner still can't boot up gl contexts.

Referencing #241

Previously, we ignored the gl test suites in the karma CI config, but more recently, I've been using this hasWebGLSupport util directly in the test suites. See example in the mapbox test - which outputs this

making it a little clearer that we're skipping a few tests on CI (I think).

Thanks @etpinard, I'll update with the hasWebGLSupport.

Yes, planning to add click to this PR.

monfera · 2016-10-03T13:40:25Z

src/plots/gl2d/scene2d.js

@@ -332,6 +337,8 @@ proto.destroy = function() {
 this.container.removeChild(this.svgContainer);
 this.container.removeChild(this.mouseContainer);

+ this.fullData = null;
+ this._inputs = null;


@etpinard I added this.fullData and this._inputs to retain info prerequisite for the event data contents you suggested (below), please let me know if there's a better way. Working on curveNumber and pointNumber now, I suppose these are just the indices.

this.graphDiv.emit('plotly_click', { points: [{ x: , y: , curveNumber: , pointNumber: , data: nextSelection.trace._input, fullData: nextSelection.trace, xaxis: this.axis, yaxis: this.yaxis }]

etpinard

@monfera looking great.

Let's just make sure pointIndex is emitted correctly for trace !== 'scattergl' and this PR will be ✅

etpinard · 2016-10-03T22:31:07Z

src/plots/gl2d/scene2d.js

+
+ var curveIndex = this._inputs[nextSelection.trace.uid];
+
+ this.graphDiv.emit(eventType, {


very nicely done.

etpinard · 2016-10-03T22:34:12Z

src/traces/scattergl/convert.js

@@ -128,6 +128,7 @@ proto.handlePick = function(pickResult) {
 this.color[index] :
 this.color,
 name: this.name,
+ pointIndex: index,


Great.

Now, can you add pointIndex to pointcloud, heatmapgl and contourgl? Note that for heatmapgl and contourgl, pointIndex will be a 2-item array.

etpinard · 2016-10-05T17:12:49Z

src/traces/contourgl/convert.js

 ],
 textLabel: this.textLabels[index],
 name: this.name,
+ pointIndex: [xIndex, yIndex, zIndex],


I would prefer

pointIndex: [xIndex, yIndex]

to match cartesian heatmap and contour

monfera added 2 commits September 28, 2016 20:40

test

53c7c11

hover, unhover

c4baeb2

etpinard added feature something new status: in progress labels Sep 29, 2016

etpinard added this to the v1.18.0 milestone Sep 29, 2016

etpinard suggested changes Sep 29, 2016

View reviewed changes

monfera added 2 commits September 30, 2016 14:44

adds click emission; adds test cases; uses hasWebGLSupport

359d50a

PR feedback: enriching event callback data

1e56d7f

monfera commented Oct 3, 2016

View reviewed changes

monfera added 2 commits October 3, 2016 18:11

PR feedback: enriching event callback data - indices

2e8db74

DRY up event emission

9f77c11

etpinard added status: reviewable and removed status: in progress labels Oct 3, 2016

etpinard suggested changes Oct 3, 2016

View reviewed changes

monfera added 2 commits October 4, 2016 21:39

PR feedback - cover the other gl2d plot types

2e54a99

PR feedback - cover the other gl2d plot types - other plot type tests

57cec8c

monfera force-pushed the FI-71 branch from e78c2c0 to 57cec8c Compare October 4, 2016 20:20

PR feedback - cover the other gl2d plot types - contourgl plot type test

a9b8b3e

etpinard reviewed Oct 5, 2016

View reviewed changes

PR feedback - return [x, y] only

c1abd3e

etpinard approved these changes Oct 5, 2016

View reviewed changes

etpinard merged commit 293f201 into plotly:master Oct 5, 2016

This was referenced Oct 5, 2016

Click event on "scattergl" doesn't work #743

Closed

gl2d known limitations #130

Closed

This was referenced Jan 6, 2017

event_data doesn't work with scatterplot (type = "scattergl") plotly/plotly.R#624

Closed

event_data doesn't work with scatterplot (type = "scattergl") plotly/plotly.R#845

Closed

Highlighting with click/hover in gl2d seems glitchy plotly/plotly.R#846

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

monfera commented Sep 29, 2016 •

edited

Loading

etpinard left a comment

etpinard Sep 29, 2016 •

edited

Loading

monfera Sep 29, 2016

monfera Sep 30, 2016

etpinard Sep 30, 2016

monfera Sep 30, 2016

etpinard Sep 30, 2016 •

edited

Loading

monfera Sep 30, 2016

monfera Sep 30, 2016

etpinard Sep 29, 2016

monfera Sep 29, 2016

monfera Sep 29, 2016

monfera Oct 3, 2016 •

edited

Loading

etpinard Oct 3, 2016

etpinard left a comment

etpinard Oct 3, 2016

etpinard Oct 3, 2016 •

edited

Loading

etpinard Oct 5, 2016


		var curveIndex = this._inputs[nextSelection.trace.uid];

		this.graphDiv.emit(eventType, {

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

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

Conversation

monfera commented Sep 29, 2016 • edited Loading

etpinard left a comment

Choose a reason for hiding this comment

etpinard Sep 29, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard Sep 30, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monfera Oct 3, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard Oct 3, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monfera commented Sep 29, 2016 •

edited

Loading

etpinard Sep 29, 2016 •

edited

Loading

etpinard Sep 30, 2016 •

edited

Loading

monfera Oct 3, 2016 •

edited

Loading

etpinard Oct 3, 2016 •

edited

Loading