Skip to content
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

Output bounding box to hover event data #5512

Merged
merged 19 commits into from
Aug 26, 2021
Merged

Output bounding box to hover event data #5512

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
4573f34
Output bounding box to hover event data
rreusser Feb 18, 2021
e817828
Merge remote-tracking branch 'origin/master' into external-hover-2
archmoj Aug 25, 2021
86a4fbe
draft log for PR 5512
archmoj Aug 25, 2021
4b04337
fix syntax
archmoj Aug 25, 2021
034779d
rename top and left variables
archmoj Aug 25, 2021
9a57ec3
adjust jasmine tests
archmoj Aug 25, 2021
6078118
drop extra sort
archmoj Aug 25, 2021
03b22ec
revise bbox positions
archmoj Aug 25, 2021
17dbadd
ensure x0 <= x1 and y0 <= y1
archmoj Aug 26, 2021
bd6688e
interactive test
archmoj Aug 26, 2021
ccd2c88
handle gl3d, pie, sunburst, treemap, etc.
archmoj Aug 26, 2021
c41b31d
drop hover bbox display
archmoj Aug 26, 2021
888a6ba
add jasmine tests for cartesian
archmoj Aug 26, 2021
8100f8a
provide bbox in gl3d event data
archmoj Aug 26, 2021
262505d
bbox for pie and funnelarea
archmoj Aug 26, 2021
53e1b61
provide bbox for sunburst treemap and icicle
archmoj Aug 26, 2021
aa00c18
improve bbox for sunburst, treemap and icicle slices
archmoj Aug 26, 2021
9716d7e
improve bbox for pie slices
archmoj Aug 26, 2021
6ba88db
improve bbox for funnelarea slices
archmoj Aug 26, 2021
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
5 changes: 5 additions & 0 deletions src/components/fx/helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,11 @@ exports.makeEventData = function(pt, trace, cd) {
if('yVal' in pt) out.y = pt.yVal;
else if('y' in pt) out.y = pt.y;

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;
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.


if(pt.xa) out.xaxis = pt.xa;
if(pt.ya) out.yaxis = pt.ya;
if(pt.zLabelVal !== undefined) out.z = pt.zLabelVal;
Expand Down
14 changes: 14 additions & 0 deletions src/components/fx/hover.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,12 @@ function _hover(gd, evt, subplot, noHoverEvent) {
var oldhoverdata = gd._hoverdata;
var newhoverdata = [];

// Top/left hover offsets relative to graph div. As long as hover content is
// a sibling of the graph div, it will be positioned correctly relative to
// the offset parent, whatever that may be.
var hot = gd.offsetTop + gd.clientTop;
var hol = gd.offsetLeft + gd.clientLeft;

// pull out just the data that's useful to
// other people and send it to the event
for(itemnum = 0; itemnum < hoverData.length; itemnum++) {
Expand All @@ -694,6 +700,14 @@ function _hover(gd, evt, subplot, noHoverEvent) {
pt.hovertemplate = ht || pt.trace.hovertemplate || false;
}

var bbox = {};
eventData.bbox = bbox;

if ('x0' in pt) bbox.x0 = hol + pt.x0 + pt.xa._offset;
if ('x1' in pt) bbox.x1 = hol + pt.x1 + pt.xa._offset;
if ('y0' in pt) bbox.y0 = hot + pt.y0 + pt.ya._offset;
if ('y1' in pt) bbox.y1 = hot + pt.y1 + pt.ya._offset;

pt.eventData = [eventData];
newhoverdata.push(eventData);
}
Expand Down
5 changes: 5 additions & 0 deletions src/traces/bar/event_data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ module.exports = function eventData(out, pt, trace) {
if(pt.xa) out.xaxis = pt.xa;
if(pt.ya) out.yaxis = pt.ya;

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;

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

if(trace.orientation === 'h') {
out.label = out.y;
out.value = out.x;
Expand Down
5 changes: 5 additions & 0 deletions src/traces/box/event_data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,10 @@ module.exports = function eventData(out, pt) {
if(pt.xa) out.xaxis = pt.xa;
if(pt.ya) out.yaxis = pt.ya;

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;

return out;
};
5 changes: 5 additions & 0 deletions src/traces/funnel/event_data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,10 @@ module.exports = function eventData(out, pt /* , trace, cd, pointNumber */) {
if(pt.xa) out.xaxis = pt.xa;
if(pt.ya) out.yaxis = pt.ya;

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;

return out;
};
5 changes: 5 additions & 0 deletions src/traces/histogram/event_data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ module.exports = function eventData(out, pt, trace, cd, pointNumber) {
if(pt.xa) out.xaxis = pt.xa;
if(pt.ya) out.yaxis = pt.ya;

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;

// specific to histogram - CDFs do not have pts (yet?)
if(!(trace.cumulative || {}).enabled) {
var pts = Array.isArray(pointNumber) ?
Expand Down
6 changes: 6 additions & 0 deletions src/traces/image/event_data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
module.exports = function eventData(out, pt) {
if('xVal' in pt) out.x = pt.xVal;
if('yVal' in pt) out.y = pt.yVal;

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;

if(pt.xa) out.xaxis = pt.xa;
if(pt.ya) out.yaxis = pt.ya;
out.color = pt.color;
Expand Down
11 changes: 10 additions & 1 deletion src/traces/pie/event_data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,16 @@ module.exports = function eventData(pt, trace) {
text: pt.text,

// pt.v (and pt.i below) for backward compatibility
v: pt.v
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)
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Addressed in 03b22ec.

bbox: {
x0: pt.x0,
x1: pt.x1,
y0: pt.y0,
y1: pt.y1,
},
};

// Only include pointNumber if it's unambiguous
Expand Down
20 changes: 14 additions & 6 deletions src/traces/pie/plot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,12 +379,20 @@ function attachFxHandlers(sliceTop, gd, cd) {

if(hoverinfo === 'all') hoverinfo = 'label+text+value+percent+name';

// If hoverinfo === 'none', we still want the *coordinates* of hover to be
// output, just not the hover to actually display
var rInscribed = pt.rInscribed || 0;
var hoverCenterX = cx + pt.pxmid[0] * (1 - rInscribed);
var hoverCenterY = cy + pt.pxmid[1] * (1 - rInscribed);
pt.x0 = hoverCenterX - rInscribed * cd0.r;
pt.x1 = hoverCenterX + rInscribed * cd0.r;
pt.y0 = hoverCenterY;
pt.y1 = hoverCenterY;

// in case we dragged over the pie from another subplot,
// or if hover is turned off
if(trace2.hovertemplate || (hoverinfo !== 'none' && hoverinfo !== 'skip' && hoverinfo)) {
var rInscribed = pt.rInscribed || 0;
var hoverCenterX = cx + pt.pxmid[0] * (1 - rInscribed);
var hoverCenterY = cy + pt.pxmid[1] * (1 - rInscribed);

var separators = fullLayout2.separators;
var text = [];

Expand All @@ -406,9 +414,9 @@ function attachFxHandlers(sliceTop, gd, cd) {

Fx.loneHover({
trace: trace,
x0: hoverCenterX - rInscribed * cd0.r,
x1: hoverCenterX + rInscribed * cd0.r,
y: hoverCenterY,
x0: pt.x0,
x1: pt.x1,
y: pt.y0,
text: text.join('<br>'),
name: (trace2.hovertemplate || hoverinfo.indexOf('name') !== -1) ? trace2.name : undefined,
idealAlign: pt.pxmid[0] < 0 ? 'left' : 'right',
Expand Down
5 changes: 5 additions & 0 deletions src/traces/scattercarpet/event_data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,10 @@ module.exports = function eventData(out, pt, trace, cd, pointNumber) {
out.b = cdi.b;
out.y = cdi.y;

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;

return out;
};
5 changes: 5 additions & 0 deletions src/traces/scatterternary/event_data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ module.exports = function eventData(out, pt, trace, cd, pointNumber) {
if(pt.xa) out.xaxis = pt.xa;
if(pt.ya) out.yaxis = pt.ya;

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;

if(cd[pointNumber]) {
var cdi = cd[pointNumber];

Expand Down
5 changes: 5 additions & 0 deletions src/traces/waterfall/event_data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,10 @@ module.exports = function eventData(out, pt /* , trace, cd, pointNumber */) {
if(pt.xa) out.xaxis = pt.xa;
if(pt.ya) out.yaxis = pt.ya;

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;

return out;
};