Skip to content

Fixes hover tooltip position with non-origin div position #1664

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 2 commits into from
May 10, 2017
Merged

Fixes hover tooltip position with non-origin div position #1664

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3d9fcf2
fix hover tooltip position with non-original div position
monfera May 10, 2017
0c53c82
remove obsolete scroll check
monfera May 10, 2017
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
18 changes: 10 additions & 8 deletions src/traces/sankey/plot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,14 @@ module.exports = function plot(gd, calcData) {

var linkHoverFollow = function(element, d) {

var rootBBox = gd.getBoundingClientRect();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use fullLayout.width and fullLayout.height or even fullLayout._size instead? Those getBoundingClientRect calls are expensive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, I'll check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

... maybe I could use help for a low-latency fix :-) the code needs to know the positions, not just the width/height - having said this, the speed impact is likely negligible as it's only triggered on one element at a time. Yet if there's a need to quickly switch, I could use suggestions for getting the right position without a bbox call.

Copy link
Collaborator

Choose a reason for hiding this comment

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

this matches the pattern we've had to use elsewhere. It's possible that both getBoundingClientRect calls could be avoided by figuring out the position directly from the link/node element (or by caching it in the element's d3 data or something) but if you use getBoundingClientRect for one you'd better use it for both. That could be an optimization later, this is good for now.

var boundingBox = element.getBoundingClientRect();
var hoverCenterX = boundingBox.left + boundingBox.width / 2;
var hoverCenterY = boundingBox.top + boundingBox.height / 2;

var tooltip = Fx.loneHover({
x: hoverCenterX + window.scrollX,
y: hoverCenterY + window.scrollY,
x: hoverCenterX - rootBBox.left,
y: hoverCenterY - rootBBox.top,
name: d3.format(d.valueFormat)(d.link.value) + d.valueSuffix,
text: [
d.link.label,
Expand Down Expand Up @@ -173,15 +174,16 @@ module.exports = function plot(gd, calcData) {
var nodeHoverFollow = function(element, d) {

var nodeRect = d3.select(element).select('.nodeRect');
var rootBBox = gd.getBoundingClientRect();
var boundingBox = nodeRect.node().getBoundingClientRect();
var hoverCenterX0 = boundingBox.left - 2;
var hoverCenterX1 = boundingBox.right + 2;
var hoverCenterY = boundingBox.top + boundingBox.height / 4;
var hoverCenterX0 = boundingBox.left - 2 - rootBBox.left;
var hoverCenterX1 = boundingBox.right + 2 - rootBBox.left;
var hoverCenterY = boundingBox.top + boundingBox.height / 4 - rootBBox.top;

var tooltip = Fx.loneHover({
x0: hoverCenterX0 + window.scrollX,
x1: hoverCenterX1 + window.scrollX,
y: hoverCenterY + window.scrollY,
x0: hoverCenterX0,
x1: hoverCenterX1,
y: hoverCenterY,
name: d3.format(d.valueFormat)(d.node.value) + d.valueSuffix,
text: [
d.node.label,
Expand Down