Output bounding box to hover event data #5512

rreusser · 2021-02-18T15:59:14Z

Addressing #1323

This PR adds bbox: {x0, x1, y0, y1} data to the points of hover event data. I've so far opted to keep the same format as the original hover data since I think it's very easy understand and doesn't mix coordinates with information about the righthand side of the plot, which I think is best left to the user. (Or at the very least would be useful additional/duplicate information while leaving x0/x1/y0/y1 available.)

A short todo list:

fix pie
ensure it works with CSS transforms
add test(s)

A list of related tasks:

figure out the clearest way to convey the meaning of the coordinates
create a dash example for server-side hover
create a dash example for client-side spinner with server-side hover
add specification of hover info to plotly.js docs
add examples to dash-docs (partially complete, will submit WIP PR)

Regarding the last point, the coordinates are basically gd.offsetTop + gd.clientTop + offsetWithinPlot. The benefit of this is that positioning hovers within the plot should be very simple. You simply place a hover element as a sibling of the graph div, assign it the coordinates, and it should work without needing to worry about the margins, borders, or the offset parent (nearest element with position: 'relative' | 'absolute'). The downside is that this brings in position information from outside the graph div itself, but I think a reasonable solution is to state that if you want coordinates strictly limited to the plot, you should place HTML hover content and the graph div as direct children of a relative|absolute-positioned element.

Regarding CSS transforms, I think that I just need to apply layout._invTransform to the coordinates within the plot, but I haven't yet confirmed this. In matrix pseudocode, that would make the coordinates [offsetLeft, offsetTop] + [clientLeft, clientTop] + invTransform * [plotOffsetLeft, plotOffsetTop].

The current list of trace types supported is:

Obviously some of these aren't so important, but I think if there are some not worth supporting (e.g. pointcloud?), it'd at least be nicely to clearly state expectations around what does happen. I think it should not be too bad to fix the GL plots, which probably mostly goes through loneHover.

Some of them (e.g. ohlc) also require particular knowledge of what the hover points mean since you get a flat list of maybe five points (bounding box, o, h, l, c?), but I think the main thing for now is that the data is there.

rreusser · 2021-03-05T15:03:30Z

One thing I've noticed about this PR is that Dash rejects any nested properties so that adding bbox: {x0, x1, y0, y1} does not get passed to pointData in dash event data. I've kept them as flat properties at the moment, e.g. offsetX0, so that they end up in pointData in dash.

alexcjohnson · 2021-03-05T20:11:10Z

Dash rejects any nested properties

FYI that's here - if a flat object is too cumbersome to work with we can certainly carve out an exception in dcc.Graph to handle the preferred nested structure. I think the purpose of that rejection is so we don't try to include the whole x/y axis objects in the event data that gets serialized and sent to the server, but small nested objects would be fine.

nicolaskruchten · 2021-04-20T14:16:40Z

Hey guys, what's the status of this PR?

nicolaskruchten · 2021-04-20T15:03:26Z

This is to feed plotly/dash-core-components#940 right?

nicolaskruchten · 2021-07-18T17:48:07Z

@archmoj @alexcjohnson what's left here? Can we merge this for 2.3.0?

alexcjohnson · 2021-07-18T20:41:43Z

src/components/fx/helpers.js

+        if ('x0' in pt) out.x0 = pt.x0;
+        if ('x1' in pt) out.x1 = pt.x1;
+        if ('y0' in pt) out.y0 = pt.y0;
+        if ('y1' in pt) out.y1 = pt.y1;


🌴 there's 7 or 8 of these... could be something like copyCoords(pt, out) - could even include the xa/ya-> xaxis/yaxis part, AFAICT only scattercarpet is missing that one, but I bet it wouldn't even hurt to include that.

Is that information already accessible through plotly_hover? If so, can't we just modify dcc.Graph instead of making this PR?

The original mouse event is accessible in plotly_hover, and that contains the pointer location in various coordinate systems. But plotly.js hover events are more sophisticated, and it would be far better if we can match the plotly.js behavior with our new interface.

These coordinates mark the data points rather than the mouse.

We give the full extent of the location being hovered on. For example, for a vertical bar this is the left and right edges of the top of the bar. For a scatter trace, this is the bounding box of the marker.

There can be multiple points in the hover set, each with their own bounding box.

alexcjohnson · 2021-07-18T20:43:09Z

src/traces/pie/event_data.js

+        v: pt.v,
+
+        // TODO: These coordinates aren't quite correct and don't take into account some offset
+        // I still haven't quite located (similar to xa._offset)


If this doesn't work correctly yet, can we just drop it and leave pie as a TODO?

Addressed in 03b22ec.

archmoj · 2021-08-25T17:13:01Z

test/jasmine/tests/ternary_test.js

+var SORTED_EVENT_KEYS = [
+    'data', 'fullData', 'curveNumber', 'pointNumber', 'pointIndex',
+    'xaxis', 'yaxis', 'a', 'b', 'c',
+    'bbox', 'x0', 'x1', 'y0', 'y1'


Do we need x0, x1, y0 and y1 keys here?
They are available in bbox.

archmoj · 2021-08-25T17:19:21Z

src/traces/bar/event_data.js

+    if('x1' in pt) out.x1 = pt.x1;
+    if('y0' in pt) out.y0 = pt.y0;
+    if('y1' in pt) out.y1 = pt.y1;
+


Do we need x0 , x1, y0 and y1 in event data? Having bbox in not enough?

I think we concluded that more was better here, to make it easier for folks in Dash-land.

Is this correct? For vertical bars y0 is equal to y1 here and in the bbox.

Shouldn't p0, p1, s0 and s1 used instead of (x|y) (0|1)?

For vertical bars y0 is equal to y1

Yes, y0===y1 for vertical bars. We want the label to be at the end of the bar, not in the middle.

Shouldn't p0, p1, s0 and s1 used instead

The final event data should contain x/y. p/s is not useful to the user for positioning other elements, and that's the purpose here.

archmoj · 2021-08-25T21:20:54Z

The x0, y0, x1, y1 positions outside bbox appear to be unused.
So after the revision made in 03b22ec commit x0, y0, x1, y1 re only exposed in the bbox and ONLY for the points that have x and y axes.

alexcjohnson · 2021-08-25T22:09:53Z

The x0, y0, x1, y1 positions outside bbox appear to be unused.

Yes, that's OK - currently dcc.Graph drops nested objects from the event so it doesn't try to serialize xaxis & yaxis objects, but in the tooltip PR (which I'm going to have to remake in the monorepo anyway) we're whitelisting this object.

So after the revision made in 03b22ec commit x0, y0, x1, y1 re only exposed in the bbox and ONLY for the points that have x and y axes.

That's fine for right now, but please make an issue to come back to this later and add support for other traces. I'm particularly expecting people will want this to work in pies and 3D.

alexcjohnson

💃

nicolaskruchten · 2021-08-25T22:33:20Z

The original issue for this was for 3D... this PR doesn't work for 3D scatter?

archmoj · 2021-08-26T15:13:18Z

Right now marker sizes are take into account for scatter points; but not for scattergl points.

archmoj · 2021-08-26T15:17:47Z

For image trace the y0 is always equal to y1!

alexcjohnson · 2021-08-26T16:00:37Z

y0===y1 isn't a big deal - mostly people will be using the average y value and depending on x0 & x1 to specify the horizontal extent. I wouldn't spend much time investigating all of these now, but later, if folks on the Dash side want to be able to position hover labels above or below the point instead of left/right, we can revisit this as a minor bug.

archmoj · 2021-08-26T18:12:58Z

gl3d traces as well as pie, funnelarea, sunburst, treemap and icicle now add bbox to event data.

Output bounding box to hover event data

4573f34

nicolaskruchten mentioned this pull request Apr 20, 2021

ToolTip component plotly/dash-core-components#940

Closed

alexcjohnson reviewed Jul 18, 2021

View reviewed changes

archmoj marked this pull request as ready for review July 19, 2021 01:22

archmoj added status: has TODOs feature something new labels Jul 19, 2021

xhluca mentioned this pull request Aug 9, 2021

Re-add new tooltip component plotly/dash-core-components#982

Closed

6 tasks

archmoj mentioned this pull request Aug 23, 2021

Output bounding box to hover event data (new attempt) #5872

Closed

archmoj added this to the v2.4.0 milestone Aug 24, 2021

archmoj added 5 commits August 25, 2021 10:44

Merge remote-tracking branch 'origin/master' into external-hover-2

e817828

draft log for PR 5512

86a4fbe

fix syntax

4b04337

rename top and left variables

034779d

adjust jasmine tests

9a57ec3

archmoj added status: reviewable and removed status: has TODOs labels Aug 25, 2021

drop extra sort

6078118

archmoj reviewed Aug 25, 2021

View reviewed changes

archmoj requested a review from alexcjohnson August 25, 2021 17:29

revise bbox positions

03b22ec

alexcjohnson approved these changes Aug 25, 2021

View reviewed changes

archmoj added 2 commits August 26, 2021 11:48

ensure x0 <= x1 and y0 <= y1

17dbadd

interactive test

bd6688e

archmoj added 2 commits August 26, 2021 13:22

handle gl3d, pie, sunburst, treemap, etc.

ccd2c88

drop hover bbox display

c41b31d

archmoj added 7 commits August 26, 2021 16:28

add jasmine tests for cartesian

888a6ba

provide bbox in gl3d event data

8100f8a

bbox for pie and funnelarea

262505d

provide bbox for sunburst treemap and icicle

53e1b61

improve bbox for sunburst, treemap and icicle slices

aa00c18

improve bbox for pie slices

9716d7e

improve bbox for funnelarea slices

6ba88db

archmoj merged commit ed63090 into master Aug 26, 2021

archmoj deleted the external-hover-2 branch August 26, 2021 23:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Output bounding box to hover event data #5512

Output bounding box to hover event data #5512

rreusser commented Feb 18, 2021 •

edited by archmoj

Loading

rreusser commented Mar 5, 2021 •

edited

Loading

alexcjohnson commented Mar 5, 2021

nicolaskruchten commented Apr 20, 2021

nicolaskruchten commented Apr 20, 2021

nicolaskruchten commented Jul 18, 2021

alexcjohnson Jul 18, 2021

xhluca Aug 2, 2021

alexcjohnson Aug 3, 2021

alexcjohnson Jul 18, 2021

archmoj Aug 25, 2021

archmoj Aug 25, 2021

archmoj Aug 25, 2021

nicolaskruchten Aug 25, 2021

archmoj Aug 25, 2021

archmoj Aug 25, 2021

alexcjohnson Aug 25, 2021

archmoj commented Aug 25, 2021

alexcjohnson commented Aug 25, 2021

alexcjohnson left a comment

nicolaskruchten commented Aug 25, 2021

archmoj commented Aug 26, 2021

archmoj commented Aug 26, 2021

alexcjohnson commented Aug 26, 2021

archmoj commented Aug 26, 2021 •

edited

Loading

Output bounding box to hover event data #5512

Output bounding box to hover event data #5512

Conversation

rreusser commented Feb 18, 2021 • edited by archmoj Loading

rreusser commented Mar 5, 2021 • edited Loading

alexcjohnson commented Mar 5, 2021

nicolaskruchten commented Apr 20, 2021

nicolaskruchten commented Apr 20, 2021

nicolaskruchten commented Jul 18, 2021

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

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

archmoj commented Aug 25, 2021

alexcjohnson commented Aug 25, 2021

alexcjohnson left a comment

Choose a reason for hiding this comment

nicolaskruchten commented Aug 25, 2021

archmoj commented Aug 26, 2021

archmoj commented Aug 26, 2021

alexcjohnson commented Aug 26, 2021

archmoj commented Aug 26, 2021 • edited Loading

rreusser commented Feb 18, 2021 •

edited by archmoj

Loading

rreusser commented Mar 5, 2021 •

edited

Loading

archmoj commented Aug 26, 2021 •

edited

Loading