Skip to content

Fix hover label positioning for bar trace with set width#1527

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
alexcjohnson merged 6 commits into from
Apr 5, 2017
Merged

Fix hover label positioning for bar trace with set width#1527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7a4154f
make distance fn accept pt index as arg
etpinard Mar 17, 2017
c84e2a9
generalize bar delta computation
etpinard Mar 17, 2017
591bd9e
add two bar hover cases with set 'width'
etpinard Mar 17, 2017
b34d9ef
Merge branch 'master' into bar-width-hover
alexcjohnson Apr 4, 2017
30bc8c7
broaden bar hover acceptance, and test
alexcjohnson Apr 5, 2017
06097d7
shift new bar hover test for ci
alexcjohnson Apr 5, 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
72 changes: 44 additions & 28 deletions src/traces/bar/hover.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,50 +15,66 @@ var Color = require('../../components/color');


module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
var cd = pointData.cd,
trace = cd[0].trace,
t = cd[0].t,
xa = pointData.xa,
ya = pointData.ya,
barDelta = (hovermode === 'closest') ?
t.barwidth / 2 :
t.bargroupwidth / 2,
barPos;

if(hovermode !== 'closest') barPos = function(di) { return di.p; };
else if(trace.orientation === 'h') barPos = function(di) { return di.y; };
else barPos = function(di) { return di.x; };

var dx, dy;
var cd = pointData.cd;
var trace = cd[0].trace;
var t = cd[0].t;
var xa = pointData.xa;
var ya = pointData.ya;

var posVal, thisBarMinPos, thisBarMaxPos, minPos, maxPos, dx, dy;

var positionFn = function(di) {
return Fx.inbox(minPos(di) - posVal, maxPos(di) - posVal);
};

if(trace.orientation === 'h') {
posVal = yval;
thisBarMinPos = function(di) { return di.y - di.w / 2; };
thisBarMaxPos = function(di) { return di.y + di.w / 2; };
dx = function(di) {
// add a gradient so hovering near the end of a
// bar makes it a little closer match
return Fx.inbox(di.b - xval, di.x - xval) + (di.x - xval) / (di.x - di.b);
};
dy = function(di) {
var centerPos = barPos(di) - yval;
return Fx.inbox(centerPos - barDelta, centerPos + barDelta);
};
dy = positionFn;
}
else {
posVal = xval;
thisBarMinPos = function(di) { return di.x - di.w / 2; };
thisBarMaxPos = function(di) { return di.x + di.w / 2; };
dy = function(di) {
return Fx.inbox(di.b - yval, di.y - yval) + (di.y - yval) / (di.y - di.b);
};
dx = function(di) {
var centerPos = barPos(di) - xval;
return Fx.inbox(centerPos - barDelta, centerPos + barDelta);
};
dx = positionFn;
}

minPos = (hovermode === 'closest') ?
thisBarMinPos :
function(di) {
/*
* In compare mode, accept a bar if you're on it *or* its group.
* Nearly always it's the group that matters, but in case the bar
* was explicitly set wider than its group we'd better accept the
* whole bar.
*/

@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

fixes the "compare" part of #1317 - at least if the bar is wider than our default. I guess if it's narrower and there are other things on the plot, we could be accepting this point when we shouldn't be... maybe I'll look at that case tomorrow and see if I can trigger it.

@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Alright, the only case I can see where this pops up is an explicitly narrow (width<1) single bar with other things on the plot as well, like:

vardata=[{type: 'bar',x: [1],y: [2],width: 0.1},{x:[1.1],y:[1]}];varlayout={xaxis: {range: [-2,2]}};Plotly.newPlot(gd,data,layout);

then the hoverable region for the bar is so wide it actually extends past the scatter point. But the scatter point still wins when you'd want it to. So if that's all it is, I don't feel like it's worth addressing this very obscure edge case, it'd be just as likely to cause other problems.

@etpinardetpinardApr 5, 2017

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I agree with ⬆️ . Thanks for documenting it!

return Math.min(thisBarMinPos(di), di.p - t.bargroupwidth / 2);
};

maxPos = (hovermode === 'closest') ?
thisBarMaxPos :
function(di) {
return Math.max(thisBarMaxPos(di), di.p + t.bargroupwidth / 2);
};

var distfn = Fx.getDistanceFunction(hovermode, dx, dy);
Fx.getClosest(cd, distfn, pointData);

// skip the rest (for this trace) if we didn't find a close point
if(pointData.index === false) return;

// the closest data point
var di = cd[pointData.index],
var index = pointData.index,
di = cd[index],
mc = di.mcc || trace.marker.color,
mlc = di.mlcc || trace.marker.line.color,
mlw = di.mlw || trace.marker.line.width;
Expand All@@ -70,16 +86,16 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
pointData.x0 = pointData.x1 = xa.c2p(di.x, true);
pointData.xLabelVal = size;

pointData.y0 = ya.c2p(barPos(di) - barDelta, true);
pointData.y1 = ya.c2p(barPos(di) + barDelta, true);
pointData.y0 = ya.c2p(minPos(di), true);
pointData.y1 = ya.c2p(maxPos(di), true);
pointData.yLabelVal = di.p;
}
else {
pointData.y0 = pointData.y1 = ya.c2p(di.y, true);
pointData.yLabelVal = size;

pointData.x0 = xa.c2p(barPos(di) - barDelta, true);
pointData.x1 = xa.c2p(barPos(di) + barDelta, true);
pointData.x0 = xa.c2p(minPos(di), true);
pointData.x1 = xa.c2p(maxPos(di), true);
pointData.xLabelVal = di.p;
}

Expand Down
6 changes: 2 additions & 4 deletions src/traces/bar/plot.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -48,9 +48,7 @@ module.exports = function plot(gd, plotinfo, cdbar) {
var t = d[0].t,
trace = d[0].trace,
poffset = t.poffset,
poffsetIsArray = Array.isArray(poffset),
barwidth = t.barwidth,
barwidthIsArray = Array.isArray(barwidth);
poffsetIsArray = Array.isArray(poffset);

d3.select(this).selectAll('g.point')
.data(Lib.identity)
Expand All@@ -61,7 +59,7 @@ module.exports = function plot(gd, plotinfo, cdbar) {
// log values go off-screen by plotwidth
// so you see them continue if you drag the plot
var p0 = di.p + ((poffsetIsArray) ? poffset[i] : poffset),
p1 = p0 + ((barwidthIsArray) ? barwidth[i] : barwidth),
p1 = p0 + di.w,

@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

a little follow-on benefit of adding di.w

s0 = di.b,
s1 = s0 + di.s;

Expand Down
10 changes: 6 additions & 4 deletions src/traces/bar/set_positions.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -236,7 +236,7 @@ function setOffsetAndWidth(gd, pa, sieve) {
applyAttributes(sieve);

// store the bar center in each calcdata item
setBarCenter(gd, pa, sieve);
setBarCenterAndWidth(gd, pa, sieve);

// update position axes
updatePositionAxis(gd, pa, sieve);
Expand DownExpand Up@@ -286,7 +286,7 @@ function setOffsetAndWidthInGroupMode(gd, pa, sieve) {
applyAttributes(sieve);

// store the bar center in each calcdata item
setBarCenter(gd, pa, sieve);
setBarCenterAndWidth(gd, pa, sieve);

// update position axes
updatePositionAxis(gd, pa, sieve, overlap);
Expand DownExpand Up@@ -377,7 +377,7 @@ function applyAttributes(sieve) {
}


function setBarCenter(gd, pa, sieve) {
function setBarCenterAndWidth(gd, pa, sieve) {
var calcTraces = sieve.traces,
pLetter = getAxisLetter(pa);

Expand All@@ -392,9 +392,11 @@ function setBarCenter(gd, pa, sieve) {
for(var j = 0; j < calcTrace.length; j++) {
var calcBar = calcTrace[j];

// store the actual bar width and position, for use by hover
var width = calcBar.w = (barwidthIsArray) ? barwidth[j] : barwidth;

@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I suspect we could clean up the hover routine quite a bit more if we stopped using .x and .y in calcdata... then we'd only have to switch at the beginning of the routine "are xval and yval position and size or vice versa" but I wasn't sure what the consequences of that would be beyond hover, just adding one more value was safe and easy.

calcBar[pLetter] = calcBar.p +
((poffsetIsArray) ? poffset[j] : poffset) +
((barwidthIsArray) ? barwidth[j] : barwidth) / 2;
width / 2;


}
Expand Down
97 changes: 95 additions & 2 deletions test/jasmine/tests/bar_test.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,7 @@ var Axes = PlotlyInternal.Axes;
var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');
var customMatchers = require('../assets/custom_matchers');
var failTest = require('../assets/fail_test');

describe('Bar.supplyDefaults', function() {
'use strict';
Expand DownExpand Up@@ -1207,9 +1208,12 @@ describe('bar hover', function() {
};
}

function _hover(gd, xval, yval, closest) {
function _hover(gd, xval, yval, hovermode) {
var pointData = getPointData(gd);
var pt = Bar.hoverPoints(pointData, xval, yval, closest)[0];
var pts = Bar.hoverPoints(pointData, xval, yval, hovermode);
if(!pts) return false;

var pt = pts[0];

return {
style: [pt.index, pt.color, pt.xLabelVal, pt.yLabelVal],
Expand DownExpand Up@@ -1290,6 +1294,95 @@ describe('bar hover', function() {
});
});

describe('with special width/offset combinations', function() {

beforeEach(function() {
gd = createGraphDiv();
});

it('should return correct hover data (single bar, trace width)', function(done) {
Plotly.plot(gd, [{
type: 'bar',
x: [1],
y: [2],
width: 10,
marker: { color: 'red' }
}], {
xaxis: { range: [-200, 200] }
})
.then(function() {
// all these x, y, hovermode should give the same (the only!) hover label
[
[0, 0, 'closest'],
[-3.9, 1, 'closest'],
[5.9, 1.9, 'closest'],
[-3.9, -10, 'x'],
[5.9, 19, 'x']
].forEach(function(hoverSpec) {
var out = _hover(gd, hoverSpec[0], hoverSpec[1], hoverSpec[2]);

expect(out.style).toEqual([0, 'red', 1, 2], hoverSpec);
assertPos(out.pos, [264, 278, 14, 14], hoverSpec);
});

// then a few that are off the edge so yield nothing
[
[1, -0.1, 'closest'],
[1, 2.1, 'closest'],
[-4.1, 1, 'closest'],
[6.1, 1, 'closest'],
[-4.1, 1, 'x'],
[6.1, 1, 'x']
].forEach(function(hoverSpec) {
var out = _hover(gd, hoverSpec[0], hoverSpec[1], hoverSpec[2]);

expect(out).toBe(false, hoverSpec);
});
})
.catch(failTest)
.then(done);
});

it('should return correct hover data (two bars, array width)', function(done) {
Plotly.plot(gd, [{
type: 'bar',
x: [1, 200],
y: [2, 1],
width: [10, 20],
marker: { color: 'red' }
}, {
type: 'bar',
x: [1, 200],
y: [1, 2],
width: [20, 10],
marker: { color: 'green' }
}], {
xaxis: { range: [-200, 300] },
width: 500,
height: 500
})
.then(function() {
var out = _hover(gd, -36, 1.5, 'closest');

expect(out.style).toEqual([0, 'red', 1, 2]);
assertPos(out.pos, [99, 106, 13, 13]);

out = _hover(gd, 164, 0.8, 'closest');

expect(out.style).toEqual([1, 'red', 200, 1]);
assertPos(out.pos, [222, 235, 168, 168]);

out = _hover(gd, 125, 0.8, 'x');

expect(out.style).toEqual([1, 'red', 200, 1]);
assertPos(out.pos, [203, 304, 168, 168]);
})
.catch(failTest)
.then(done);
});

});

});

function mockBarPlot(dataWithoutTraceType, layout) {
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Fix hover label positioning for bar trace with set `width` by etpinard · Pull Request #1527 · plotly/plotly.js · GitHub
Skip to content
Fix hover label positioning for bar trace with set width#1527

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

alexcjohnson
merged 6 commits into

from

Apr 5, 2017
 Merged


Fix hover label positioning for bar trace with set width#1527
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
7a4154f

make distance fn accept pt index as arg


etpinard Mar 17, 2017
c84e2a9

generalize bar delta computation


etpinard Mar 17, 2017
591bd9e

add two bar hover cases with set 'width'


etpinard Mar 17, 2017
b34d9ef

Merge branch 'master' into bar-width-hover


alexcjohnson Apr 4, 2017
30bc8c7

broaden bar hover acceptance, and test


alexcjohnson Apr 5, 2017
06097d7

shift new bar hover test for ci


alexcjohnson Apr 5, 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


72 changes: 44 additions & 28 deletions
src/traces/bar/hover.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -15,50 +15,66 @@ var Color = require('../../components/color');


module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
 var cd = pointData.cd,
 trace = cd[0].trace,
 t = cd[0].t,
 xa = pointData.xa,
 ya = pointData.ya,
 barDelta = (hovermode === 'closest') ?
 t.barwidth / 2 :
 t.bargroupwidth / 2,
 barPos;

 if(hovermode !== 'closest') barPos = function(di) { return di.p; };
 else if(trace.orientation === 'h') barPos = function(di) { return di.y; };
 else barPos = function(di) { return di.x; };

 var dx, dy;
 var cd = pointData.cd;
 var trace = cd[0].trace;
 var t = cd[0].t;
 var xa = pointData.xa;
 var ya = pointData.ya;

 var posVal, thisBarMinPos, thisBarMaxPos, minPos, maxPos, dx, dy;

 var positionFn = function(di) {
 return Fx.inbox(minPos(di) - posVal, maxPos(di) - posVal);
 };

 if(trace.orientation === 'h') {
 posVal = yval;
 thisBarMinPos = function(di) { return di.y - di.w / 2; };
 thisBarMaxPos = function(di) { return di.y + di.w / 2; };
 dx = function(di) {
 // add a gradient so hovering near the end of a
 // bar makes it a little closer match
 return Fx.inbox(di.b - xval, di.x - xval) + (di.x - xval) / (di.x - di.b);
 };
 dy = function(di) {
 var centerPos = barPos(di) - yval;
 return Fx.inbox(centerPos - barDelta, centerPos + barDelta);
 };
 dy = positionFn;
 }
 else {
 posVal = xval;
 thisBarMinPos = function(di) { return di.x - di.w / 2; };
 thisBarMaxPos = function(di) { return di.x + di.w / 2; };
 dy = function(di) {
 return Fx.inbox(di.b - yval, di.y - yval) + (di.y - yval) / (di.y - di.b);
 };
 dx = function(di) {
 var centerPos = barPos(di) - xval;
 return Fx.inbox(centerPos - barDelta, centerPos + barDelta);
 };
 dx = positionFn;
 }

 minPos = (hovermode === 'closest') ?
 thisBarMinPos :
 function(di) {
 /*
 * In compare mode, accept a bar if you're on it *or* its group.
 * Nearly always it's the group that matters, but in case the bar
 * was explicitly set wider than its group we'd better accept the
 * whole bar.
 */
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

fixes the "compare" part of #1317 - at least if the bar is wider than our default. I guess if it's narrower and there are other things on the plot, we could be accepting this point when we shouldn't be... maybe I'll look at that case tomorrow and see if I can trigger it.
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Alright, the only case I can see where this pops up is an explicitly narrow (width<1) single bar with other things on the plot as well, like:
vardata=[{type: 'bar',x: [1],y: [2],width: 0.1},{x:[1.1],y:[1]}];varlayout={xaxis: {range: [-2,2]}};Plotly.newPlot(gd,data,layout);
then the hoverable region for the bar is so wide it actually extends past the scatter point. But the scatter point still wins when you'd want it to. So if that's all it is, I don't feel like it's worth addressing this very obscure edge case, it'd be just as likely to cause other problems.
@etpinardetpinardApr 5, 2017

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I agree with ⬆️ . Thanks for documenting it!
 return Math.min(thisBarMinPos(di), di.p - t.bargroupwidth / 2);
 };

 maxPos = (hovermode === 'closest') ?
 thisBarMaxPos :
 function(di) {
 return Math.max(thisBarMaxPos(di), di.p + t.bargroupwidth / 2);
 };

 var distfn = Fx.getDistanceFunction(hovermode, dx, dy);
 Fx.getClosest(cd, distfn, pointData);

 // skip the rest (for this trace) if we didn't find a close point
 if(pointData.index === false) return;

 // the closest data point
 var di = cd[pointData.index],
 var index = pointData.index,
 di = cd[index],
 mc = di.mcc || trace.marker.color,
 mlc = di.mlcc || trace.marker.line.color,
 mlw = di.mlw || trace.marker.line.width;
Expand All@@ -70,16 +86,16 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
 pointData.x0 = pointData.x1 = xa.c2p(di.x, true);
 pointData.xLabelVal = size;

 pointData.y0 = ya.c2p(barPos(di) - barDelta, true);
 pointData.y1 = ya.c2p(barPos(di) + barDelta, true);
 pointData.y0 = ya.c2p(minPos(di), true);
 pointData.y1 = ya.c2p(maxPos(di), true);
 pointData.yLabelVal = di.p;
 }
 else {
 pointData.y0 = pointData.y1 = ya.c2p(di.y, true);
 pointData.yLabelVal = size;

 pointData.x0 = xa.c2p(barPos(di) - barDelta, true);
 pointData.x1 = xa.c2p(barPos(di) + barDelta, true);
 pointData.x0 = xa.c2p(minPos(di), true);
 pointData.x1 = xa.c2p(maxPos(di), true);
 pointData.xLabelVal = di.p;
 }

Expand Down

6 changes: 2 additions & 4 deletions
src/traces/bar/plot.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -48,9 +48,7 @@ module.exports = function plot(gd, plotinfo, cdbar) {
 var t = d[0].t,
 trace = d[0].trace,
 poffset = t.poffset,
 poffsetIsArray = Array.isArray(poffset),
 barwidth = t.barwidth,
 barwidthIsArray = Array.isArray(barwidth);
 poffsetIsArray = Array.isArray(poffset);

 d3.select(this).selectAll('g.point')
 .data(Lib.identity)
Expand All@@ -61,7 +59,7 @@ module.exports = function plot(gd, plotinfo, cdbar) {
 // log values go off-screen by plotwidth
 // so you see them continue if you drag the plot
 var p0 = di.p + ((poffsetIsArray) ? poffset[i] : poffset),
 p1 = p0 + ((barwidthIsArray) ? barwidth[i] : barwidth),
 p1 = p0 + di.w,
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

a little follow-on benefit of adding di.w
 s0 = di.b,
 s1 = s0 + di.s;

Expand Down

10 changes: 6 additions & 4 deletions
src/traces/bar/set_positions.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -236,7 +236,7 @@ function setOffsetAndWidth(gd, pa, sieve) {
 applyAttributes(sieve);

 // store the bar center in each calcdata item
setBarCenter(gd, pa, sieve);
setBarCenterAndWidth(gd, pa, sieve);

 // update position axes
 updatePositionAxis(gd, pa, sieve);
Expand DownExpand Up@@ -286,7 +286,7 @@ function setOffsetAndWidthInGroupMode(gd, pa, sieve) {
 applyAttributes(sieve);

 // store the bar center in each calcdata item
setBarCenter(gd, pa, sieve);
setBarCenterAndWidth(gd, pa, sieve);

 // update position axes
 updatePositionAxis(gd, pa, sieve, overlap);
Expand DownExpand Up@@ -377,7 +377,7 @@ function applyAttributes(sieve) {
}


function setBarCenter(gd, pa, sieve) {
function setBarCenterAndWidth(gd, pa, sieve) {
 var calcTraces = sieve.traces,
 pLetter = getAxisLetter(pa);

Expand All@@ -392,9 +392,11 @@ function setBarCenter(gd, pa, sieve) {
 for(var j = 0; j < calcTrace.length; j++) {
 var calcBar = calcTrace[j];

 // store the actual bar width and position, for use by hover
 var width = calcBar.w = (barwidthIsArray) ? barwidth[j] : barwidth;
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I suspect we could clean up the hover routine quite a bit more if we stopped using .x and .y in calcdata... then we'd only have to switch at the beginning of the routine "are xval and yval position and size or vice versa" but I wasn't sure what the consequences of that would be beyond hover, just adding one more value was safe and easy.
 calcBar[pLetter] = calcBar.p +
 ((poffsetIsArray) ? poffset[j] : poffset) +
((barwidthIsArray) ? barwidth[j] : barwidth) / 2;
width / 2;


 }
Expand Down

97 changes: 95 additions & 2 deletions
test/jasmine/tests/bar_test.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,7 @@ var Axes = PlotlyInternal.Axes;
var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');
var customMatchers = require('../assets/custom_matchers');
var failTest = require('../assets/fail_test');

describe('Bar.supplyDefaults', function() {
 'use strict';
Expand DownExpand Up@@ -1207,9 +1208,12 @@ describe('bar hover', function() {
 };
 }

 function _hover(gd, xval, yval, closest) {
 function _hover(gd, xval, yval, hovermode) {
 var pointData = getPointData(gd);
 var pt = Bar.hoverPoints(pointData, xval, yval, closest)[0];
 var pts = Bar.hoverPoints(pointData, xval, yval, hovermode);
 if(!pts) return false;

 var pt = pts[0];

 return {
 style: [pt.index, pt.color, pt.xLabelVal, pt.yLabelVal],
Expand DownExpand Up@@ -1290,6 +1294,95 @@ describe('bar hover', function() {
 });
 });

 describe('with special width/offset combinations', function() {

 beforeEach(function() {
 gd = createGraphDiv();
 });

 it('should return correct hover data (single bar, trace width)', function(done) {
 Plotly.plot(gd, [{
 type: 'bar',
 x: [1],
 y: [2],
 width: 10,
 marker: { color: 'red' }
 }], {
 xaxis: { range: [-200, 200] }
 })
 .then(function() {
 // all these x, y, hovermode should give the same (the only!) hover label
 [
 [0, 0, 'closest'],
 [-3.9, 1, 'closest'],
 [5.9, 1.9, 'closest'],
 [-3.9, -10, 'x'],
 [5.9, 19, 'x']
 ].forEach(function(hoverSpec) {
 var out = _hover(gd, hoverSpec[0], hoverSpec[1], hoverSpec[2]);

 expect(out.style).toEqual([0, 'red', 1, 2], hoverSpec);
 assertPos(out.pos, [264, 278, 14, 14], hoverSpec);
 });

 // then a few that are off the edge so yield nothing
 [
 [1, -0.1, 'closest'],
 [1, 2.1, 'closest'],
 [-4.1, 1, 'closest'],
 [6.1, 1, 'closest'],
 [-4.1, 1, 'x'],
 [6.1, 1, 'x']
 ].forEach(function(hoverSpec) {
 var out = _hover(gd, hoverSpec[0], hoverSpec[1], hoverSpec[2]);

 expect(out).toBe(false, hoverSpec);
 });
 })
 .catch(failTest)
 .then(done);
 });

 it('should return correct hover data (two bars, array width)', function(done) {
 Plotly.plot(gd, [{
 type: 'bar',
 x: [1, 200],
 y: [2, 1],
 width: [10, 20],
 marker: { color: 'red' }
 }, {
 type: 'bar',
 x: [1, 200],
 y: [1, 2],
 width: [20, 10],
 marker: { color: 'green' }
 }], {
 xaxis: { range: [-200, 300] },
 width: 500,
 height: 500
 })
 .then(function() {
 var out = _hover(gd, -36, 1.5, 'closest');

 expect(out.style).toEqual([0, 'red', 1, 2]);
 assertPos(out.pos, [99, 106, 13, 13]);

 out = _hover(gd, 164, 0.8, 'closest');

 expect(out.style).toEqual([1, 'red', 200, 1]);
 assertPos(out.pos, [222, 235, 168, 168]);

 out = _hover(gd, 125, 0.8, 'x');

 expect(out.style).toEqual([1, 'red', 200, 1]);
 assertPos(out.pos, [203, 304, 168, 168]);
 })
 .catch(failTest)
 .then(done);
 });

 });

});

function mockBarPlot(dataWithoutTraceType, layout) {
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Fix hover label positioning for bar trace with set `width` by etpinard · Pull Request #1527 · plotly/plotly.js · GitHub
Skip to content
Fix hover label positioning for bar trace with set width#1527

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

alexcjohnson
merged 6 commits into

from

Apr 5, 2017
 Merged


Fix hover label positioning for bar trace with set width#1527
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
7a4154f

make distance fn accept pt index as arg


etpinard Mar 17, 2017
c84e2a9

generalize bar delta computation


etpinard Mar 17, 2017
591bd9e

add two bar hover cases with set 'width'


etpinard Mar 17, 2017
b34d9ef

Merge branch 'master' into bar-width-hover


alexcjohnson Apr 4, 2017
30bc8c7

broaden bar hover acceptance, and test


alexcjohnson Apr 5, 2017
06097d7

shift new bar hover test for ci


alexcjohnson Apr 5, 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


72 changes: 44 additions & 28 deletions
src/traces/bar/hover.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -15,50 +15,66 @@ var Color = require('../../components/color');


module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
 var cd = pointData.cd,
 trace = cd[0].trace,
 t = cd[0].t,
 xa = pointData.xa,
 ya = pointData.ya,
 barDelta = (hovermode === 'closest') ?
 t.barwidth / 2 :
 t.bargroupwidth / 2,
 barPos;

 if(hovermode !== 'closest') barPos = function(di) { return di.p; };
 else if(trace.orientation === 'h') barPos = function(di) { return di.y; };
 else barPos = function(di) { return di.x; };

 var dx, dy;
 var cd = pointData.cd;
 var trace = cd[0].trace;
 var t = cd[0].t;
 var xa = pointData.xa;
 var ya = pointData.ya;

 var posVal, thisBarMinPos, thisBarMaxPos, minPos, maxPos, dx, dy;

 var positionFn = function(di) {
 return Fx.inbox(minPos(di) - posVal, maxPos(di) - posVal);
 };

 if(trace.orientation === 'h') {
 posVal = yval;
 thisBarMinPos = function(di) { return di.y - di.w / 2; };
 thisBarMaxPos = function(di) { return di.y + di.w / 2; };
 dx = function(di) {
 // add a gradient so hovering near the end of a
 // bar makes it a little closer match
 return Fx.inbox(di.b - xval, di.x - xval) + (di.x - xval) / (di.x - di.b);
 };
 dy = function(di) {
 var centerPos = barPos(di) - yval;
 return Fx.inbox(centerPos - barDelta, centerPos + barDelta);
 };
 dy = positionFn;
 }
 else {
 posVal = xval;
 thisBarMinPos = function(di) { return di.x - di.w / 2; };
 thisBarMaxPos = function(di) { return di.x + di.w / 2; };
 dy = function(di) {
 return Fx.inbox(di.b - yval, di.y - yval) + (di.y - yval) / (di.y - di.b);
 };
 dx = function(di) {
 var centerPos = barPos(di) - xval;
 return Fx.inbox(centerPos - barDelta, centerPos + barDelta);
 };
 dx = positionFn;
 }

 minPos = (hovermode === 'closest') ?
 thisBarMinPos :
 function(di) {
 /*
 * In compare mode, accept a bar if you're on it *or* its group.
 * Nearly always it's the group that matters, but in case the bar
 * was explicitly set wider than its group we'd better accept the
 * whole bar.
 */
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

fixes the "compare" part of #1317 - at least if the bar is wider than our default. I guess if it's narrower and there are other things on the plot, we could be accepting this point when we shouldn't be... maybe I'll look at that case tomorrow and see if I can trigger it.
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Alright, the only case I can see where this pops up is an explicitly narrow (width<1) single bar with other things on the plot as well, like:
vardata=[{type: 'bar',x: [1],y: [2],width: 0.1},{x:[1.1],y:[1]}];varlayout={xaxis: {range: [-2,2]}};Plotly.newPlot(gd,data,layout);
then the hoverable region for the bar is so wide it actually extends past the scatter point. But the scatter point still wins when you'd want it to. So if that's all it is, I don't feel like it's worth addressing this very obscure edge case, it'd be just as likely to cause other problems.
@etpinardetpinardApr 5, 2017

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I agree with ⬆️ . Thanks for documenting it!
 return Math.min(thisBarMinPos(di), di.p - t.bargroupwidth / 2);
 };

 maxPos = (hovermode === 'closest') ?
 thisBarMaxPos :
 function(di) {
 return Math.max(thisBarMaxPos(di), di.p + t.bargroupwidth / 2);
 };

 var distfn = Fx.getDistanceFunction(hovermode, dx, dy);
 Fx.getClosest(cd, distfn, pointData);

 // skip the rest (for this trace) if we didn't find a close point
 if(pointData.index === false) return;

 // the closest data point
 var di = cd[pointData.index],
 var index = pointData.index,
 di = cd[index],
 mc = di.mcc || trace.marker.color,
 mlc = di.mlcc || trace.marker.line.color,
 mlw = di.mlw || trace.marker.line.width;
Expand All@@ -70,16 +86,16 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
 pointData.x0 = pointData.x1 = xa.c2p(di.x, true);
 pointData.xLabelVal = size;

 pointData.y0 = ya.c2p(barPos(di) - barDelta, true);
 pointData.y1 = ya.c2p(barPos(di) + barDelta, true);
 pointData.y0 = ya.c2p(minPos(di), true);
 pointData.y1 = ya.c2p(maxPos(di), true);
 pointData.yLabelVal = di.p;
 }
 else {
 pointData.y0 = pointData.y1 = ya.c2p(di.y, true);
 pointData.yLabelVal = size;

 pointData.x0 = xa.c2p(barPos(di) - barDelta, true);
 pointData.x1 = xa.c2p(barPos(di) + barDelta, true);
 pointData.x0 = xa.c2p(minPos(di), true);
 pointData.x1 = xa.c2p(maxPos(di), true);
 pointData.xLabelVal = di.p;
 }

Expand Down

6 changes: 2 additions & 4 deletions
src/traces/bar/plot.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -48,9 +48,7 @@ module.exports = function plot(gd, plotinfo, cdbar) {
 var t = d[0].t,
 trace = d[0].trace,
 poffset = t.poffset,
 poffsetIsArray = Array.isArray(poffset),
 barwidth = t.barwidth,
 barwidthIsArray = Array.isArray(barwidth);
 poffsetIsArray = Array.isArray(poffset);

 d3.select(this).selectAll('g.point')
 .data(Lib.identity)
Expand All@@ -61,7 +59,7 @@ module.exports = function plot(gd, plotinfo, cdbar) {
 // log values go off-screen by plotwidth
 // so you see them continue if you drag the plot
 var p0 = di.p + ((poffsetIsArray) ? poffset[i] : poffset),
 p1 = p0 + ((barwidthIsArray) ? barwidth[i] : barwidth),
 p1 = p0 + di.w,
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

a little follow-on benefit of adding di.w
 s0 = di.b,
 s1 = s0 + di.s;

Expand Down

10 changes: 6 additions & 4 deletions
src/traces/bar/set_positions.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -236,7 +236,7 @@ function setOffsetAndWidth(gd, pa, sieve) {
 applyAttributes(sieve);

 // store the bar center in each calcdata item
setBarCenter(gd, pa, sieve);
setBarCenterAndWidth(gd, pa, sieve);

 // update position axes
 updatePositionAxis(gd, pa, sieve);
Expand DownExpand Up@@ -286,7 +286,7 @@ function setOffsetAndWidthInGroupMode(gd, pa, sieve) {
 applyAttributes(sieve);

 // store the bar center in each calcdata item
setBarCenter(gd, pa, sieve);
setBarCenterAndWidth(gd, pa, sieve);

 // update position axes
 updatePositionAxis(gd, pa, sieve, overlap);
Expand DownExpand Up@@ -377,7 +377,7 @@ function applyAttributes(sieve) {
}


function setBarCenter(gd, pa, sieve) {
function setBarCenterAndWidth(gd, pa, sieve) {
 var calcTraces = sieve.traces,
 pLetter = getAxisLetter(pa);

Expand All@@ -392,9 +392,11 @@ function setBarCenter(gd, pa, sieve) {
 for(var j = 0; j < calcTrace.length; j++) {
 var calcBar = calcTrace[j];

 // store the actual bar width and position, for use by hover
 var width = calcBar.w = (barwidthIsArray) ? barwidth[j] : barwidth;
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I suspect we could clean up the hover routine quite a bit more if we stopped using .x and .y in calcdata... then we'd only have to switch at the beginning of the routine "are xval and yval position and size or vice versa" but I wasn't sure what the consequences of that would be beyond hover, just adding one more value was safe and easy.
 calcBar[pLetter] = calcBar.p +
 ((poffsetIsArray) ? poffset[j] : poffset) +
((barwidthIsArray) ? barwidth[j] : barwidth) / 2;
width / 2;


 }
Expand Down

97 changes: 95 additions & 2 deletions
test/jasmine/tests/bar_test.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,7 @@ var Axes = PlotlyInternal.Axes;
var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');
var customMatchers = require('../assets/custom_matchers');
var failTest = require('../assets/fail_test');

describe('Bar.supplyDefaults', function() {
 'use strict';
Expand DownExpand Up@@ -1207,9 +1208,12 @@ describe('bar hover', function() {
 };
 }

 function _hover(gd, xval, yval, closest) {
 function _hover(gd, xval, yval, hovermode) {
 var pointData = getPointData(gd);
 var pt = Bar.hoverPoints(pointData, xval, yval, closest)[0];
 var pts = Bar.hoverPoints(pointData, xval, yval, hovermode);
 if(!pts) return false;

 var pt = pts[0];

 return {
 style: [pt.index, pt.color, pt.xLabelVal, pt.yLabelVal],
Expand DownExpand Up@@ -1290,6 +1294,95 @@ describe('bar hover', function() {
 });
 });

 describe('with special width/offset combinations', function() {

 beforeEach(function() {
 gd = createGraphDiv();
 });

 it('should return correct hover data (single bar, trace width)', function(done) {
 Plotly.plot(gd, [{
 type: 'bar',
 x: [1],
 y: [2],
 width: 10,
 marker: { color: 'red' }
 }], {
 xaxis: { range: [-200, 200] }
 })
 .then(function() {
 // all these x, y, hovermode should give the same (the only!) hover label
 [
 [0, 0, 'closest'],
 [-3.9, 1, 'closest'],
 [5.9, 1.9, 'closest'],
 [-3.9, -10, 'x'],
 [5.9, 19, 'x']
 ].forEach(function(hoverSpec) {
 var out = _hover(gd, hoverSpec[0], hoverSpec[1], hoverSpec[2]);

 expect(out.style).toEqual([0, 'red', 1, 2], hoverSpec);
 assertPos(out.pos, [264, 278, 14, 14], hoverSpec);
 });

 // then a few that are off the edge so yield nothing
 [
 [1, -0.1, 'closest'],
 [1, 2.1, 'closest'],
 [-4.1, 1, 'closest'],
 [6.1, 1, 'closest'],
 [-4.1, 1, 'x'],
 [6.1, 1, 'x']
 ].forEach(function(hoverSpec) {
 var out = _hover(gd, hoverSpec[0], hoverSpec[1], hoverSpec[2]);

 expect(out).toBe(false, hoverSpec);
 });
 })
 .catch(failTest)
 .then(done);
 });

 it('should return correct hover data (two bars, array width)', function(done) {
 Plotly.plot(gd, [{
 type: 'bar',
 x: [1, 200],
 y: [2, 1],
 width: [10, 20],
 marker: { color: 'red' }
 }, {
 type: 'bar',
 x: [1, 200],
 y: [1, 2],
 width: [20, 10],
 marker: { color: 'green' }
 }], {
 xaxis: { range: [-200, 300] },
 width: 500,
 height: 500
 })
 .then(function() {
 var out = _hover(gd, -36, 1.5, 'closest');

 expect(out.style).toEqual([0, 'red', 1, 2]);
 assertPos(out.pos, [99, 106, 13, 13]);

 out = _hover(gd, 164, 0.8, 'closest');

 expect(out.style).toEqual([1, 'red', 200, 1]);
 assertPos(out.pos, [222, 235, 168, 168]);

 out = _hover(gd, 125, 0.8, 'x');

 expect(out.style).toEqual([1, 'red', 200, 1]);
 assertPos(out.pos, [203, 304, 168, 168]);
 })
 .catch(failTest)
 .then(done);
 });

 });

});

function mockBarPlot(dataWithoutTraceType, layout) {
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Fix hover label positioning for bar trace with set `width` by etpinard · Pull Request #1527 · plotly/plotly.js · GitHub
Skip to content
Fix hover label positioning for bar trace with set width#1527

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

alexcjohnson
merged 6 commits into

from

Apr 5, 2017
 Merged


Fix hover label positioning for bar trace with set width#1527
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
7a4154f

make distance fn accept pt index as arg


etpinard Mar 17, 2017
c84e2a9

generalize bar delta computation


etpinard Mar 17, 2017
591bd9e

add two bar hover cases with set 'width'


etpinard Mar 17, 2017
b34d9ef

Merge branch 'master' into bar-width-hover


alexcjohnson Apr 4, 2017
30bc8c7

broaden bar hover acceptance, and test


alexcjohnson Apr 5, 2017
06097d7

shift new bar hover test for ci


alexcjohnson Apr 5, 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


72 changes: 44 additions & 28 deletions
src/traces/bar/hover.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -15,50 +15,66 @@ var Color = require('../../components/color');


module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
 var cd = pointData.cd,
 trace = cd[0].trace,
 t = cd[0].t,
 xa = pointData.xa,
 ya = pointData.ya,
 barDelta = (hovermode === 'closest') ?
 t.barwidth / 2 :
 t.bargroupwidth / 2,
 barPos;

 if(hovermode !== 'closest') barPos = function(di) { return di.p; };
 else if(trace.orientation === 'h') barPos = function(di) { return di.y; };
 else barPos = function(di) { return di.x; };

 var dx, dy;
 var cd = pointData.cd;
 var trace = cd[0].trace;
 var t = cd[0].t;
 var xa = pointData.xa;
 var ya = pointData.ya;

 var posVal, thisBarMinPos, thisBarMaxPos, minPos, maxPos, dx, dy;

 var positionFn = function(di) {
 return Fx.inbox(minPos(di) - posVal, maxPos(di) - posVal);
 };

 if(trace.orientation === 'h') {
 posVal = yval;
 thisBarMinPos = function(di) { return di.y - di.w / 2; };
 thisBarMaxPos = function(di) { return di.y + di.w / 2; };
 dx = function(di) {
 // add a gradient so hovering near the end of a
 // bar makes it a little closer match
 return Fx.inbox(di.b - xval, di.x - xval) + (di.x - xval) / (di.x - di.b);
 };
 dy = function(di) {
 var centerPos = barPos(di) - yval;
 return Fx.inbox(centerPos - barDelta, centerPos + barDelta);
 };
 dy = positionFn;
 }
 else {
 posVal = xval;
 thisBarMinPos = function(di) { return di.x - di.w / 2; };
 thisBarMaxPos = function(di) { return di.x + di.w / 2; };
 dy = function(di) {
 return Fx.inbox(di.b - yval, di.y - yval) + (di.y - yval) / (di.y - di.b);
 };
 dx = function(di) {
 var centerPos = barPos(di) - xval;
 return Fx.inbox(centerPos - barDelta, centerPos + barDelta);
 };
 dx = positionFn;
 }

 minPos = (hovermode === 'closest') ?
 thisBarMinPos :
 function(di) {
 /*
 * In compare mode, accept a bar if you're on it *or* its group.
 * Nearly always it's the group that matters, but in case the bar
 * was explicitly set wider than its group we'd better accept the
 * whole bar.
 */
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

fixes the "compare" part of #1317 - at least if the bar is wider than our default. I guess if it's narrower and there are other things on the plot, we could be accepting this point when we shouldn't be... maybe I'll look at that case tomorrow and see if I can trigger it.
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Alright, the only case I can see where this pops up is an explicitly narrow (width<1) single bar with other things on the plot as well, like:
vardata=[{type: 'bar',x: [1],y: [2],width: 0.1},{x:[1.1],y:[1]}];varlayout={xaxis: {range: [-2,2]}};Plotly.newPlot(gd,data,layout);
then the hoverable region for the bar is so wide it actually extends past the scatter point. But the scatter point still wins when you'd want it to. So if that's all it is, I don't feel like it's worth addressing this very obscure edge case, it'd be just as likely to cause other problems.
@etpinardetpinardApr 5, 2017

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I agree with ⬆️ . Thanks for documenting it!
 return Math.min(thisBarMinPos(di), di.p - t.bargroupwidth / 2);
 };

 maxPos = (hovermode === 'closest') ?
 thisBarMaxPos :
 function(di) {
 return Math.max(thisBarMaxPos(di), di.p + t.bargroupwidth / 2);
 };

 var distfn = Fx.getDistanceFunction(hovermode, dx, dy);
 Fx.getClosest(cd, distfn, pointData);

 // skip the rest (for this trace) if we didn't find a close point
 if(pointData.index === false) return;

 // the closest data point
 var di = cd[pointData.index],
 var index = pointData.index,
 di = cd[index],
 mc = di.mcc || trace.marker.color,
 mlc = di.mlcc || trace.marker.line.color,
 mlw = di.mlw || trace.marker.line.width;
Expand All@@ -70,16 +86,16 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
 pointData.x0 = pointData.x1 = xa.c2p(di.x, true);
 pointData.xLabelVal = size;

 pointData.y0 = ya.c2p(barPos(di) - barDelta, true);
 pointData.y1 = ya.c2p(barPos(di) + barDelta, true);
 pointData.y0 = ya.c2p(minPos(di), true);
 pointData.y1 = ya.c2p(maxPos(di), true);
 pointData.yLabelVal = di.p;
 }
 else {
 pointData.y0 = pointData.y1 = ya.c2p(di.y, true);
 pointData.yLabelVal = size;

 pointData.x0 = xa.c2p(barPos(di) - barDelta, true);
 pointData.x1 = xa.c2p(barPos(di) + barDelta, true);
 pointData.x0 = xa.c2p(minPos(di), true);
 pointData.x1 = xa.c2p(maxPos(di), true);
 pointData.xLabelVal = di.p;
 }

Expand Down

6 changes: 2 additions & 4 deletions
src/traces/bar/plot.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -48,9 +48,7 @@ module.exports = function plot(gd, plotinfo, cdbar) {
 var t = d[0].t,
 trace = d[0].trace,
 poffset = t.poffset,
 poffsetIsArray = Array.isArray(poffset),
 barwidth = t.barwidth,
 barwidthIsArray = Array.isArray(barwidth);
 poffsetIsArray = Array.isArray(poffset);

 d3.select(this).selectAll('g.point')
 .data(Lib.identity)
Expand All@@ -61,7 +59,7 @@ module.exports = function plot(gd, plotinfo, cdbar) {
 // log values go off-screen by plotwidth
 // so you see them continue if you drag the plot
 var p0 = di.p + ((poffsetIsArray) ? poffset[i] : poffset),
 p1 = p0 + ((barwidthIsArray) ? barwidth[i] : barwidth),
 p1 = p0 + di.w,
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

a little follow-on benefit of adding di.w
 s0 = di.b,
 s1 = s0 + di.s;

Expand Down

10 changes: 6 additions & 4 deletions
src/traces/bar/set_positions.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -236,7 +236,7 @@ function setOffsetAndWidth(gd, pa, sieve) {
 applyAttributes(sieve);

 // store the bar center in each calcdata item
setBarCenter(gd, pa, sieve);
setBarCenterAndWidth(gd, pa, sieve);

 // update position axes
 updatePositionAxis(gd, pa, sieve);
Expand DownExpand Up@@ -286,7 +286,7 @@ function setOffsetAndWidthInGroupMode(gd, pa, sieve) {
 applyAttributes(sieve);

 // store the bar center in each calcdata item
setBarCenter(gd, pa, sieve);
setBarCenterAndWidth(gd, pa, sieve);

 // update position axes
 updatePositionAxis(gd, pa, sieve, overlap);
Expand DownExpand Up@@ -377,7 +377,7 @@ function applyAttributes(sieve) {
}


function setBarCenter(gd, pa, sieve) {
function setBarCenterAndWidth(gd, pa, sieve) {
 var calcTraces = sieve.traces,
 pLetter = getAxisLetter(pa);

Expand All@@ -392,9 +392,11 @@ function setBarCenter(gd, pa, sieve) {
 for(var j = 0; j < calcTrace.length; j++) {
 var calcBar = calcTrace[j];

 // store the actual bar width and position, for use by hover
 var width = calcBar.w = (barwidthIsArray) ? barwidth[j] : barwidth;
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I suspect we could clean up the hover routine quite a bit more if we stopped using .x and .y in calcdata... then we'd only have to switch at the beginning of the routine "are xval and yval position and size or vice versa" but I wasn't sure what the consequences of that would be beyond hover, just adding one more value was safe and easy.
 calcBar[pLetter] = calcBar.p +
 ((poffsetIsArray) ? poffset[j] : poffset) +
((barwidthIsArray) ? barwidth[j] : barwidth) / 2;
width / 2;


 }
Expand Down

97 changes: 95 additions & 2 deletions
test/jasmine/tests/bar_test.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,7 @@ var Axes = PlotlyInternal.Axes;
var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');
var customMatchers = require('../assets/custom_matchers');
var failTest = require('../assets/fail_test');

describe('Bar.supplyDefaults', function() {
 'use strict';
Expand DownExpand Up@@ -1207,9 +1208,12 @@ describe('bar hover', function() {
 };
 }

 function _hover(gd, xval, yval, closest) {
 function _hover(gd, xval, yval, hovermode) {
 var pointData = getPointData(gd);
 var pt = Bar.hoverPoints(pointData, xval, yval, closest)[0];
 var pts = Bar.hoverPoints(pointData, xval, yval, hovermode);
 if(!pts) return false;

 var pt = pts[0];

 return {
 style: [pt.index, pt.color, pt.xLabelVal, pt.yLabelVal],
Expand DownExpand Up@@ -1290,6 +1294,95 @@ describe('bar hover', function() {
 });
 });

 describe('with special width/offset combinations', function() {

 beforeEach(function() {
 gd = createGraphDiv();
 });

 it('should return correct hover data (single bar, trace width)', function(done) {
 Plotly.plot(gd, [{
 type: 'bar',
 x: [1],
 y: [2],
 width: 10,
 marker: { color: 'red' }
 }], {
 xaxis: { range: [-200, 200] }
 })
 .then(function() {
 // all these x, y, hovermode should give the same (the only!) hover label
 [
 [0, 0, 'closest'],
 [-3.9, 1, 'closest'],
 [5.9, 1.9, 'closest'],
 [-3.9, -10, 'x'],
 [5.9, 19, 'x']
 ].forEach(function(hoverSpec) {
 var out = _hover(gd, hoverSpec[0], hoverSpec[1], hoverSpec[2]);

 expect(out.style).toEqual([0, 'red', 1, 2], hoverSpec);
 assertPos(out.pos, [264, 278, 14, 14], hoverSpec);
 });

 // then a few that are off the edge so yield nothing
 [
 [1, -0.1, 'closest'],
 [1, 2.1, 'closest'],
 [-4.1, 1, 'closest'],
 [6.1, 1, 'closest'],
 [-4.1, 1, 'x'],
 [6.1, 1, 'x']
 ].forEach(function(hoverSpec) {
 var out = _hover(gd, hoverSpec[0], hoverSpec[1], hoverSpec[2]);

 expect(out).toBe(false, hoverSpec);
 });
 })
 .catch(failTest)
 .then(done);
 });

 it('should return correct hover data (two bars, array width)', function(done) {
 Plotly.plot(gd, [{
 type: 'bar',
 x: [1, 200],
 y: [2, 1],
 width: [10, 20],
 marker: { color: 'red' }
 }, {
 type: 'bar',
 x: [1, 200],
 y: [1, 2],
 width: [20, 10],
 marker: { color: 'green' }
 }], {
 xaxis: { range: [-200, 300] },
 width: 500,
 height: 500
 })
 .then(function() {
 var out = _hover(gd, -36, 1.5, 'closest');

 expect(out.style).toEqual([0, 'red', 1, 2]);
 assertPos(out.pos, [99, 106, 13, 13]);

 out = _hover(gd, 164, 0.8, 'closest');

 expect(out.style).toEqual([1, 'red', 200, 1]);
 assertPos(out.pos, [222, 235, 168, 168]);

 out = _hover(gd, 125, 0.8, 'x');

 expect(out.style).toEqual([1, 'red', 200, 1]);
 assertPos(out.pos, [203, 304, 168, 168]);
 })
 .catch(failTest)
 .then(done);
 });

 });

});

function mockBarPlot(dataWithoutTraceType, layout) {
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
Fix hover label positioning for bar trace with set `width` by etpinard · Pull Request #1527 · plotly/plotly.js · GitHub
Skip to content
Fix hover label positioning for bar trace with set width#1527

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

alexcjohnson
merged 6 commits into

from

Apr 5, 2017
 Merged


Fix hover label positioning for bar trace with set width#1527
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
7a4154f

make distance fn accept pt index as arg


etpinard Mar 17, 2017
c84e2a9

generalize bar delta computation


etpinard Mar 17, 2017
591bd9e

add two bar hover cases with set 'width'


etpinard Mar 17, 2017
b34d9ef

Merge branch 'master' into bar-width-hover


alexcjohnson Apr 4, 2017
30bc8c7

broaden bar hover acceptance, and test


alexcjohnson Apr 5, 2017
06097d7

shift new bar hover test for ci


alexcjohnson Apr 5, 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


72 changes: 44 additions & 28 deletions
src/traces/bar/hover.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -15,50 +15,66 @@ var Color = require('../../components/color');


module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
 var cd = pointData.cd,
 trace = cd[0].trace,
 t = cd[0].t,
 xa = pointData.xa,
 ya = pointData.ya,
 barDelta = (hovermode === 'closest') ?
 t.barwidth / 2 :
 t.bargroupwidth / 2,
 barPos;

 if(hovermode !== 'closest') barPos = function(di) { return di.p; };
 else if(trace.orientation === 'h') barPos = function(di) { return di.y; };
 else barPos = function(di) { return di.x; };

 var dx, dy;
 var cd = pointData.cd;
 var trace = cd[0].trace;
 var t = cd[0].t;
 var xa = pointData.xa;
 var ya = pointData.ya;

 var posVal, thisBarMinPos, thisBarMaxPos, minPos, maxPos, dx, dy;

 var positionFn = function(di) {
 return Fx.inbox(minPos(di) - posVal, maxPos(di) - posVal);
 };

 if(trace.orientation === 'h') {
 posVal = yval;
 thisBarMinPos = function(di) { return di.y - di.w / 2; };
 thisBarMaxPos = function(di) { return di.y + di.w / 2; };
 dx = function(di) {
 // add a gradient so hovering near the end of a
 // bar makes it a little closer match
 return Fx.inbox(di.b - xval, di.x - xval) + (di.x - xval) / (di.x - di.b);
 };
 dy = function(di) {
 var centerPos = barPos(di) - yval;
 return Fx.inbox(centerPos - barDelta, centerPos + barDelta);
 };
 dy = positionFn;
 }
 else {
 posVal = xval;
 thisBarMinPos = function(di) { return di.x - di.w / 2; };
 thisBarMaxPos = function(di) { return di.x + di.w / 2; };
 dy = function(di) {
 return Fx.inbox(di.b - yval, di.y - yval) + (di.y - yval) / (di.y - di.b);
 };
 dx = function(di) {
 var centerPos = barPos(di) - xval;
 return Fx.inbox(centerPos - barDelta, centerPos + barDelta);
 };
 dx = positionFn;
 }

 minPos = (hovermode === 'closest') ?
 thisBarMinPos :
 function(di) {
 /*
 * In compare mode, accept a bar if you're on it *or* its group.
 * Nearly always it's the group that matters, but in case the bar
 * was explicitly set wider than its group we'd better accept the
 * whole bar.
 */
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

fixes the "compare" part of #1317 - at least if the bar is wider than our default. I guess if it's narrower and there are other things on the plot, we could be accepting this point when we shouldn't be... maybe I'll look at that case tomorrow and see if I can trigger it.
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Alright, the only case I can see where this pops up is an explicitly narrow (width<1) single bar with other things on the plot as well, like:
vardata=[{type: 'bar',x: [1],y: [2],width: 0.1},{x:[1.1],y:[1]}];varlayout={xaxis: {range: [-2,2]}};Plotly.newPlot(gd,data,layout);
then the hoverable region for the bar is so wide it actually extends past the scatter point. But the scatter point still wins when you'd want it to. So if that's all it is, I don't feel like it's worth addressing this very obscure edge case, it'd be just as likely to cause other problems.
@etpinardetpinardApr 5, 2017

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I agree with ⬆️ . Thanks for documenting it!
 return Math.min(thisBarMinPos(di), di.p - t.bargroupwidth / 2);
 };

 maxPos = (hovermode === 'closest') ?
 thisBarMaxPos :
 function(di) {
 return Math.max(thisBarMaxPos(di), di.p + t.bargroupwidth / 2);
 };

 var distfn = Fx.getDistanceFunction(hovermode, dx, dy);
 Fx.getClosest(cd, distfn, pointData);

 // skip the rest (for this trace) if we didn't find a close point
 if(pointData.index === false) return;

 // the closest data point
 var di = cd[pointData.index],
 var index = pointData.index,
 di = cd[index],
 mc = di.mcc || trace.marker.color,
 mlc = di.mlcc || trace.marker.line.color,
 mlw = di.mlw || trace.marker.line.width;
Expand All@@ -70,16 +86,16 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
 pointData.x0 = pointData.x1 = xa.c2p(di.x, true);
 pointData.xLabelVal = size;

 pointData.y0 = ya.c2p(barPos(di) - barDelta, true);
 pointData.y1 = ya.c2p(barPos(di) + barDelta, true);
 pointData.y0 = ya.c2p(minPos(di), true);
 pointData.y1 = ya.c2p(maxPos(di), true);
 pointData.yLabelVal = di.p;
 }
 else {
 pointData.y0 = pointData.y1 = ya.c2p(di.y, true);
 pointData.yLabelVal = size;

 pointData.x0 = xa.c2p(barPos(di) - barDelta, true);
 pointData.x1 = xa.c2p(barPos(di) + barDelta, true);
 pointData.x0 = xa.c2p(minPos(di), true);
 pointData.x1 = xa.c2p(maxPos(di), true);
 pointData.xLabelVal = di.p;
 }

Expand Down

6 changes: 2 additions & 4 deletions
src/traces/bar/plot.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -48,9 +48,7 @@ module.exports = function plot(gd, plotinfo, cdbar) {
 var t = d[0].t,
 trace = d[0].trace,
 poffset = t.poffset,
 poffsetIsArray = Array.isArray(poffset),
 barwidth = t.barwidth,
 barwidthIsArray = Array.isArray(barwidth);
 poffsetIsArray = Array.isArray(poffset);

 d3.select(this).selectAll('g.point')
 .data(Lib.identity)
Expand All@@ -61,7 +59,7 @@ module.exports = function plot(gd, plotinfo, cdbar) {
 // log values go off-screen by plotwidth
 // so you see them continue if you drag the plot
 var p0 = di.p + ((poffsetIsArray) ? poffset[i] : poffset),
 p1 = p0 + ((barwidthIsArray) ? barwidth[i] : barwidth),
 p1 = p0 + di.w,
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

a little follow-on benefit of adding di.w
 s0 = di.b,
 s1 = s0 + di.s;

Expand Down

10 changes: 6 additions & 4 deletions
src/traces/bar/set_positions.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -236,7 +236,7 @@ function setOffsetAndWidth(gd, pa, sieve) {
 applyAttributes(sieve);

 // store the bar center in each calcdata item
setBarCenter(gd, pa, sieve);
setBarCenterAndWidth(gd, pa, sieve);

 // update position axes
 updatePositionAxis(gd, pa, sieve);
Expand DownExpand Up@@ -286,7 +286,7 @@ function setOffsetAndWidthInGroupMode(gd, pa, sieve) {
 applyAttributes(sieve);

 // store the bar center in each calcdata item
setBarCenter(gd, pa, sieve);
setBarCenterAndWidth(gd, pa, sieve);

 // update position axes
 updatePositionAxis(gd, pa, sieve, overlap);
Expand DownExpand Up@@ -377,7 +377,7 @@ function applyAttributes(sieve) {
}


function setBarCenter(gd, pa, sieve) {
function setBarCenterAndWidth(gd, pa, sieve) {
 var calcTraces = sieve.traces,
 pLetter = getAxisLetter(pa);

Expand All@@ -392,9 +392,11 @@ function setBarCenter(gd, pa, sieve) {
 for(var j = 0; j < calcTrace.length; j++) {
 var calcBar = calcTrace[j];

 // store the actual bar width and position, for use by hover
 var width = calcBar.w = (barwidthIsArray) ? barwidth[j] : barwidth;
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I suspect we could clean up the hover routine quite a bit more if we stopped using .x and .y in calcdata... then we'd only have to switch at the beginning of the routine "are xval and yval position and size or vice versa" but I wasn't sure what the consequences of that would be beyond hover, just adding one more value was safe and easy.
 calcBar[pLetter] = calcBar.p +
 ((poffsetIsArray) ? poffset[j] : poffset) +
((barwidthIsArray) ? barwidth[j] : barwidth) / 2;
width / 2;


 }
Expand Down

97 changes: 95 additions & 2 deletions
test/jasmine/tests/bar_test.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,7 @@ var Axes = PlotlyInternal.Axes;
var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');
var customMatchers = require('../assets/custom_matchers');
var failTest = require('../assets/fail_test');

describe('Bar.supplyDefaults', function() {
 'use strict';
Expand DownExpand Up@@ -1207,9 +1208,12 @@ describe('bar hover', function() {
 };
 }

 function _hover(gd, xval, yval, closest) {
 function _hover(gd, xval, yval, hovermode) {
 var pointData = getPointData(gd);
 var pt = Bar.hoverPoints(pointData, xval, yval, closest)[0];
 var pts = Bar.hoverPoints(pointData, xval, yval, hovermode);
 if(!pts) return false;

 var pt = pts[0];

 return {
 style: [pt.index, pt.color, pt.xLabelVal, pt.yLabelVal],
Expand DownExpand Up@@ -1290,6 +1294,95 @@ describe('bar hover', function() {
 });
 });

 describe('with special width/offset combinations', function() {

 beforeEach(function() {
 gd = createGraphDiv();
 });

 it('should return correct hover data (single bar, trace width)', function(done) {
 Plotly.plot(gd, [{
 type: 'bar',
 x: [1],
 y: [2],
 width: 10,
 marker: { color: 'red' }
 }], {
 xaxis: { range: [-200, 200] }
 })
 .then(function() {
 // all these x, y, hovermode should give the same (the only!) hover label
 [
 [0, 0, 'closest'],
 [-3.9, 1, 'closest'],
 [5.9, 1.9, 'closest'],
 [-3.9, -10, 'x'],
 [5.9, 19, 'x']
 ].forEach(function(hoverSpec) {
 var out = _hover(gd, hoverSpec[0], hoverSpec[1], hoverSpec[2]);

 expect(out.style).toEqual([0, 'red', 1, 2], hoverSpec);
 assertPos(out.pos, [264, 278, 14, 14], hoverSpec);
 });

 // then a few that are off the edge so yield nothing
 [
 [1, -0.1, 'closest'],
 [1, 2.1, 'closest'],
 [-4.1, 1, 'closest'],
 [6.1, 1, 'closest'],
 [-4.1, 1, 'x'],
 [6.1, 1, 'x']
 ].forEach(function(hoverSpec) {
 var out = _hover(gd, hoverSpec[0], hoverSpec[1], hoverSpec[2]);

 expect(out).toBe(false, hoverSpec);
 });
 })
 .catch(failTest)
 .then(done);
 });

 it('should return correct hover data (two bars, array width)', function(done) {
 Plotly.plot(gd, [{
 type: 'bar',
 x: [1, 200],
 y: [2, 1],
 width: [10, 20],
 marker: { color: 'red' }
 }, {
 type: 'bar',
 x: [1, 200],
 y: [1, 2],
 width: [20, 10],
 marker: { color: 'green' }
 }], {
 xaxis: { range: [-200, 300] },
 width: 500,
 height: 500
 })
 .then(function() {
 var out = _hover(gd, -36, 1.5, 'closest');

 expect(out.style).toEqual([0, 'red', 1, 2]);
 assertPos(out.pos, [99, 106, 13, 13]);

 out = _hover(gd, 164, 0.8, 'closest');

 expect(out.style).toEqual([1, 'red', 200, 1]);
 assertPos(out.pos, [222, 235, 168, 168]);

 out = _hover(gd, 125, 0.8, 'x');

 expect(out.style).toEqual([1, 'red', 200, 1]);
 assertPos(out.pos, [203, 304, 168, 168]);
 })
 .catch(failTest)
 .then(done);
 });

 });

});

function mockBarPlot(dataWithoutTraceType, layout) {
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Fix hover label positioning for bar trace with set `width` by etpinard · Pull Request #1527 · plotly/plotly.js · GitHub
Skip to content
Fix hover label positioning for bar trace with set width#1527

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

alexcjohnson
merged 6 commits into

from

Apr 5, 2017
 Merged


Fix hover label positioning for bar trace with set width#1527
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
7a4154f

make distance fn accept pt index as arg


etpinard Mar 17, 2017
c84e2a9

generalize bar delta computation


etpinard Mar 17, 2017
591bd9e

add two bar hover cases with set 'width'


etpinard Mar 17, 2017
b34d9ef

Merge branch 'master' into bar-width-hover


alexcjohnson Apr 4, 2017
30bc8c7

broaden bar hover acceptance, and test


alexcjohnson Apr 5, 2017
06097d7

shift new bar hover test for ci


alexcjohnson Apr 5, 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


72 changes: 44 additions & 28 deletions
src/traces/bar/hover.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -15,50 +15,66 @@ var Color = require('../../components/color');


module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
 var cd = pointData.cd,
 trace = cd[0].trace,
 t = cd[0].t,
 xa = pointData.xa,
 ya = pointData.ya,
 barDelta = (hovermode === 'closest') ?
 t.barwidth / 2 :
 t.bargroupwidth / 2,
 barPos;

 if(hovermode !== 'closest') barPos = function(di) { return di.p; };
 else if(trace.orientation === 'h') barPos = function(di) { return di.y; };
 else barPos = function(di) { return di.x; };

 var dx, dy;
 var cd = pointData.cd;
 var trace = cd[0].trace;
 var t = cd[0].t;
 var xa = pointData.xa;
 var ya = pointData.ya;

 var posVal, thisBarMinPos, thisBarMaxPos, minPos, maxPos, dx, dy;

 var positionFn = function(di) {
 return Fx.inbox(minPos(di) - posVal, maxPos(di) - posVal);
 };

 if(trace.orientation === 'h') {
 posVal = yval;
 thisBarMinPos = function(di) { return di.y - di.w / 2; };
 thisBarMaxPos = function(di) { return di.y + di.w / 2; };
 dx = function(di) {
 // add a gradient so hovering near the end of a
 // bar makes it a little closer match
 return Fx.inbox(di.b - xval, di.x - xval) + (di.x - xval) / (di.x - di.b);
 };
 dy = function(di) {
 var centerPos = barPos(di) - yval;
 return Fx.inbox(centerPos - barDelta, centerPos + barDelta);
 };
 dy = positionFn;
 }
 else {
 posVal = xval;
 thisBarMinPos = function(di) { return di.x - di.w / 2; };
 thisBarMaxPos = function(di) { return di.x + di.w / 2; };
 dy = function(di) {
 return Fx.inbox(di.b - yval, di.y - yval) + (di.y - yval) / (di.y - di.b);
 };
 dx = function(di) {
 var centerPos = barPos(di) - xval;
 return Fx.inbox(centerPos - barDelta, centerPos + barDelta);
 };
 dx = positionFn;
 }

 minPos = (hovermode === 'closest') ?
 thisBarMinPos :
 function(di) {
 /*
 * In compare mode, accept a bar if you're on it *or* its group.
 * Nearly always it's the group that matters, but in case the bar
 * was explicitly set wider than its group we'd better accept the
 * whole bar.
 */
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

fixes the "compare" part of #1317 - at least if the bar is wider than our default. I guess if it's narrower and there are other things on the plot, we could be accepting this point when we shouldn't be... maybe I'll look at that case tomorrow and see if I can trigger it.
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Alright, the only case I can see where this pops up is an explicitly narrow (width<1) single bar with other things on the plot as well, like:
vardata=[{type: 'bar',x: [1],y: [2],width: 0.1},{x:[1.1],y:[1]}];varlayout={xaxis: {range: [-2,2]}};Plotly.newPlot(gd,data,layout);
then the hoverable region for the bar is so wide it actually extends past the scatter point. But the scatter point still wins when you'd want it to. So if that's all it is, I don't feel like it's worth addressing this very obscure edge case, it'd be just as likely to cause other problems.
@etpinardetpinardApr 5, 2017

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I agree with ⬆️ . Thanks for documenting it!
 return Math.min(thisBarMinPos(di), di.p - t.bargroupwidth / 2);
 };

 maxPos = (hovermode === 'closest') ?
 thisBarMaxPos :
 function(di) {
 return Math.max(thisBarMaxPos(di), di.p + t.bargroupwidth / 2);
 };

 var distfn = Fx.getDistanceFunction(hovermode, dx, dy);
 Fx.getClosest(cd, distfn, pointData);

 // skip the rest (for this trace) if we didn't find a close point
 if(pointData.index === false) return;

 // the closest data point
 var di = cd[pointData.index],
 var index = pointData.index,
 di = cd[index],
 mc = di.mcc || trace.marker.color,
 mlc = di.mlcc || trace.marker.line.color,
 mlw = di.mlw || trace.marker.line.width;
Expand All@@ -70,16 +86,16 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
 pointData.x0 = pointData.x1 = xa.c2p(di.x, true);
 pointData.xLabelVal = size;

 pointData.y0 = ya.c2p(barPos(di) - barDelta, true);
 pointData.y1 = ya.c2p(barPos(di) + barDelta, true);
 pointData.y0 = ya.c2p(minPos(di), true);
 pointData.y1 = ya.c2p(maxPos(di), true);
 pointData.yLabelVal = di.p;
 }
 else {
 pointData.y0 = pointData.y1 = ya.c2p(di.y, true);
 pointData.yLabelVal = size;

 pointData.x0 = xa.c2p(barPos(di) - barDelta, true);
 pointData.x1 = xa.c2p(barPos(di) + barDelta, true);
 pointData.x0 = xa.c2p(minPos(di), true);
 pointData.x1 = xa.c2p(maxPos(di), true);
 pointData.xLabelVal = di.p;
 }

Expand Down

6 changes: 2 additions & 4 deletions
src/traces/bar/plot.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -48,9 +48,7 @@ module.exports = function plot(gd, plotinfo, cdbar) {
 var t = d[0].t,
 trace = d[0].trace,
 poffset = t.poffset,
 poffsetIsArray = Array.isArray(poffset),
 barwidth = t.barwidth,
 barwidthIsArray = Array.isArray(barwidth);
 poffsetIsArray = Array.isArray(poffset);

 d3.select(this).selectAll('g.point')
 .data(Lib.identity)
Expand All@@ -61,7 +59,7 @@ module.exports = function plot(gd, plotinfo, cdbar) {
 // log values go off-screen by plotwidth
 // so you see them continue if you drag the plot
 var p0 = di.p + ((poffsetIsArray) ? poffset[i] : poffset),
 p1 = p0 + ((barwidthIsArray) ? barwidth[i] : barwidth),
 p1 = p0 + di.w,
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

a little follow-on benefit of adding di.w
 s0 = di.b,
 s1 = s0 + di.s;

Expand Down

10 changes: 6 additions & 4 deletions
src/traces/bar/set_positions.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -236,7 +236,7 @@ function setOffsetAndWidth(gd, pa, sieve) {
 applyAttributes(sieve);

 // store the bar center in each calcdata item
setBarCenter(gd, pa, sieve);
setBarCenterAndWidth(gd, pa, sieve);

 // update position axes
 updatePositionAxis(gd, pa, sieve);
Expand DownExpand Up@@ -286,7 +286,7 @@ function setOffsetAndWidthInGroupMode(gd, pa, sieve) {
 applyAttributes(sieve);

 // store the bar center in each calcdata item
setBarCenter(gd, pa, sieve);
setBarCenterAndWidth(gd, pa, sieve);

 // update position axes
 updatePositionAxis(gd, pa, sieve, overlap);
Expand DownExpand Up@@ -377,7 +377,7 @@ function applyAttributes(sieve) {
}


function setBarCenter(gd, pa, sieve) {
function setBarCenterAndWidth(gd, pa, sieve) {
 var calcTraces = sieve.traces,
 pLetter = getAxisLetter(pa);

Expand All@@ -392,9 +392,11 @@ function setBarCenter(gd, pa, sieve) {
 for(var j = 0; j < calcTrace.length; j++) {
 var calcBar = calcTrace[j];

 // store the actual bar width and position, for use by hover
 var width = calcBar.w = (barwidthIsArray) ? barwidth[j] : barwidth;
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I suspect we could clean up the hover routine quite a bit more if we stopped using .x and .y in calcdata... then we'd only have to switch at the beginning of the routine "are xval and yval position and size or vice versa" but I wasn't sure what the consequences of that would be beyond hover, just adding one more value was safe and easy.
 calcBar[pLetter] = calcBar.p +
 ((poffsetIsArray) ? poffset[j] : poffset) +
((barwidthIsArray) ? barwidth[j] : barwidth) / 2;
width / 2;


 }
Expand Down

97 changes: 95 additions & 2 deletions
test/jasmine/tests/bar_test.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,7 @@ var Axes = PlotlyInternal.Axes;
var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');
var customMatchers = require('../assets/custom_matchers');
var failTest = require('../assets/fail_test');

describe('Bar.supplyDefaults', function() {
 'use strict';
Expand DownExpand Up@@ -1207,9 +1208,12 @@ describe('bar hover', function() {
 };
 }

 function _hover(gd, xval, yval, closest) {
 function _hover(gd, xval, yval, hovermode) {
 var pointData = getPointData(gd);
 var pt = Bar.hoverPoints(pointData, xval, yval, closest)[0];
 var pts = Bar.hoverPoints(pointData, xval, yval, hovermode);
 if(!pts) return false;

 var pt = pts[0];

 return {
 style: [pt.index, pt.color, pt.xLabelVal, pt.yLabelVal],
Expand DownExpand Up@@ -1290,6 +1294,95 @@ describe('bar hover', function() {
 });
 });

 describe('with special width/offset combinations', function() {

 beforeEach(function() {
 gd = createGraphDiv();
 });

 it('should return correct hover data (single bar, trace width)', function(done) {
 Plotly.plot(gd, [{
 type: 'bar',
 x: [1],
 y: [2],
 width: 10,
 marker: { color: 'red' }
 }], {
 xaxis: { range: [-200, 200] }
 })
 .then(function() {
 // all these x, y, hovermode should give the same (the only!) hover label
 [
 [0, 0, 'closest'],
 [-3.9, 1, 'closest'],
 [5.9, 1.9, 'closest'],
 [-3.9, -10, 'x'],
 [5.9, 19, 'x']
 ].forEach(function(hoverSpec) {
 var out = _hover(gd, hoverSpec[0], hoverSpec[1], hoverSpec[2]);

 expect(out.style).toEqual([0, 'red', 1, 2], hoverSpec);
 assertPos(out.pos, [264, 278, 14, 14], hoverSpec);
 });

 // then a few that are off the edge so yield nothing
 [
 [1, -0.1, 'closest'],
 [1, 2.1, 'closest'],
 [-4.1, 1, 'closest'],
 [6.1, 1, 'closest'],
 [-4.1, 1, 'x'],
 [6.1, 1, 'x']
 ].forEach(function(hoverSpec) {
 var out = _hover(gd, hoverSpec[0], hoverSpec[1], hoverSpec[2]);

 expect(out).toBe(false, hoverSpec);
 });
 })
 .catch(failTest)
 .then(done);
 });

 it('should return correct hover data (two bars, array width)', function(done) {
 Plotly.plot(gd, [{
 type: 'bar',
 x: [1, 200],
 y: [2, 1],
 width: [10, 20],
 marker: { color: 'red' }
 }, {
 type: 'bar',
 x: [1, 200],
 y: [1, 2],
 width: [20, 10],
 marker: { color: 'green' }
 }], {
 xaxis: { range: [-200, 300] },
 width: 500,
 height: 500
 })
 .then(function() {
 var out = _hover(gd, -36, 1.5, 'closest');

 expect(out.style).toEqual([0, 'red', 1, 2]);
 assertPos(out.pos, [99, 106, 13, 13]);

 out = _hover(gd, 164, 0.8, 'closest');

 expect(out.style).toEqual([1, 'red', 200, 1]);
 assertPos(out.pos, [222, 235, 168, 168]);

 out = _hover(gd, 125, 0.8, 'x');

 expect(out.style).toEqual([1, 'red', 200, 1]);
 assertPos(out.pos, [203, 304, 168, 168]);
 })
 .catch(failTest)
 .then(done);
 });

 });

});

function mockBarPlot(dataWithoutTraceType, layout) {
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Fix hover label positioning for bar trace with set `width` by etpinard · Pull Request #1527 · plotly/plotly.js · GitHub
Skip to content
Fix hover label positioning for bar trace with set width#1527

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

alexcjohnson
merged 6 commits into

from

Apr 5, 2017
 Merged


Fix hover label positioning for bar trace with set width#1527
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
7a4154f

make distance fn accept pt index as arg


etpinard Mar 17, 2017
c84e2a9

generalize bar delta computation


etpinard Mar 17, 2017
591bd9e

add two bar hover cases with set 'width'


etpinard Mar 17, 2017
b34d9ef

Merge branch 'master' into bar-width-hover


alexcjohnson Apr 4, 2017
30bc8c7

broaden bar hover acceptance, and test


alexcjohnson Apr 5, 2017
06097d7

shift new bar hover test for ci


alexcjohnson Apr 5, 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


72 changes: 44 additions & 28 deletions
src/traces/bar/hover.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -15,50 +15,66 @@ var Color = require('../../components/color');


module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
 var cd = pointData.cd,
 trace = cd[0].trace,
 t = cd[0].t,
 xa = pointData.xa,
 ya = pointData.ya,
 barDelta = (hovermode === 'closest') ?
 t.barwidth / 2 :
 t.bargroupwidth / 2,
 barPos;

 if(hovermode !== 'closest') barPos = function(di) { return di.p; };
 else if(trace.orientation === 'h') barPos = function(di) { return di.y; };
 else barPos = function(di) { return di.x; };

 var dx, dy;
 var cd = pointData.cd;
 var trace = cd[0].trace;
 var t = cd[0].t;
 var xa = pointData.xa;
 var ya = pointData.ya;

 var posVal, thisBarMinPos, thisBarMaxPos, minPos, maxPos, dx, dy;

 var positionFn = function(di) {
 return Fx.inbox(minPos(di) - posVal, maxPos(di) - posVal);
 };

 if(trace.orientation === 'h') {
 posVal = yval;
 thisBarMinPos = function(di) { return di.y - di.w / 2; };
 thisBarMaxPos = function(di) { return di.y + di.w / 2; };
 dx = function(di) {
 // add a gradient so hovering near the end of a
 // bar makes it a little closer match
 return Fx.inbox(di.b - xval, di.x - xval) + (di.x - xval) / (di.x - di.b);
 };
 dy = function(di) {
 var centerPos = barPos(di) - yval;
 return Fx.inbox(centerPos - barDelta, centerPos + barDelta);
 };
 dy = positionFn;
 }
 else {
 posVal = xval;
 thisBarMinPos = function(di) { return di.x - di.w / 2; };
 thisBarMaxPos = function(di) { return di.x + di.w / 2; };
 dy = function(di) {
 return Fx.inbox(di.b - yval, di.y - yval) + (di.y - yval) / (di.y - di.b);
 };
 dx = function(di) {
 var centerPos = barPos(di) - xval;
 return Fx.inbox(centerPos - barDelta, centerPos + barDelta);
 };
 dx = positionFn;
 }

 minPos = (hovermode === 'closest') ?
 thisBarMinPos :
 function(di) {
 /*
 * In compare mode, accept a bar if you're on it *or* its group.
 * Nearly always it's the group that matters, but in case the bar
 * was explicitly set wider than its group we'd better accept the
 * whole bar.
 */
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

fixes the "compare" part of #1317 - at least if the bar is wider than our default. I guess if it's narrower and there are other things on the plot, we could be accepting this point when we shouldn't be... maybe I'll look at that case tomorrow and see if I can trigger it.
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Alright, the only case I can see where this pops up is an explicitly narrow (width<1) single bar with other things on the plot as well, like:
vardata=[{type: 'bar',x: [1],y: [2],width: 0.1},{x:[1.1],y:[1]}];varlayout={xaxis: {range: [-2,2]}};Plotly.newPlot(gd,data,layout);
then the hoverable region for the bar is so wide it actually extends past the scatter point. But the scatter point still wins when you'd want it to. So if that's all it is, I don't feel like it's worth addressing this very obscure edge case, it'd be just as likely to cause other problems.
@etpinardetpinardApr 5, 2017

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I agree with ⬆️ . Thanks for documenting it!
 return Math.min(thisBarMinPos(di), di.p - t.bargroupwidth / 2);
 };

 maxPos = (hovermode === 'closest') ?
 thisBarMaxPos :
 function(di) {
 return Math.max(thisBarMaxPos(di), di.p + t.bargroupwidth / 2);
 };

 var distfn = Fx.getDistanceFunction(hovermode, dx, dy);
 Fx.getClosest(cd, distfn, pointData);

 // skip the rest (for this trace) if we didn't find a close point
 if(pointData.index === false) return;

 // the closest data point
 var di = cd[pointData.index],
 var index = pointData.index,
 di = cd[index],
 mc = di.mcc || trace.marker.color,
 mlc = di.mlcc || trace.marker.line.color,
 mlw = di.mlw || trace.marker.line.width;
Expand All@@ -70,16 +86,16 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
 pointData.x0 = pointData.x1 = xa.c2p(di.x, true);
 pointData.xLabelVal = size;

 pointData.y0 = ya.c2p(barPos(di) - barDelta, true);
 pointData.y1 = ya.c2p(barPos(di) + barDelta, true);
 pointData.y0 = ya.c2p(minPos(di), true);
 pointData.y1 = ya.c2p(maxPos(di), true);
 pointData.yLabelVal = di.p;
 }
 else {
 pointData.y0 = pointData.y1 = ya.c2p(di.y, true);
 pointData.yLabelVal = size;

 pointData.x0 = xa.c2p(barPos(di) - barDelta, true);
 pointData.x1 = xa.c2p(barPos(di) + barDelta, true);
 pointData.x0 = xa.c2p(minPos(di), true);
 pointData.x1 = xa.c2p(maxPos(di), true);
 pointData.xLabelVal = di.p;
 }

Expand Down

6 changes: 2 additions & 4 deletions
src/traces/bar/plot.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -48,9 +48,7 @@ module.exports = function plot(gd, plotinfo, cdbar) {
 var t = d[0].t,
 trace = d[0].trace,
 poffset = t.poffset,
 poffsetIsArray = Array.isArray(poffset),
 barwidth = t.barwidth,
 barwidthIsArray = Array.isArray(barwidth);
 poffsetIsArray = Array.isArray(poffset);

 d3.select(this).selectAll('g.point')
 .data(Lib.identity)
Expand All@@ -61,7 +59,7 @@ module.exports = function plot(gd, plotinfo, cdbar) {
 // log values go off-screen by plotwidth
 // so you see them continue if you drag the plot
 var p0 = di.p + ((poffsetIsArray) ? poffset[i] : poffset),
 p1 = p0 + ((barwidthIsArray) ? barwidth[i] : barwidth),
 p1 = p0 + di.w,
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

a little follow-on benefit of adding di.w
 s0 = di.b,
 s1 = s0 + di.s;

Expand Down

10 changes: 6 additions & 4 deletions
src/traces/bar/set_positions.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -236,7 +236,7 @@ function setOffsetAndWidth(gd, pa, sieve) {
 applyAttributes(sieve);

 // store the bar center in each calcdata item
setBarCenter(gd, pa, sieve);
setBarCenterAndWidth(gd, pa, sieve);

 // update position axes
 updatePositionAxis(gd, pa, sieve);
Expand DownExpand Up@@ -286,7 +286,7 @@ function setOffsetAndWidthInGroupMode(gd, pa, sieve) {
 applyAttributes(sieve);

 // store the bar center in each calcdata item
setBarCenter(gd, pa, sieve);
setBarCenterAndWidth(gd, pa, sieve);

 // update position axes
 updatePositionAxis(gd, pa, sieve, overlap);
Expand DownExpand Up@@ -377,7 +377,7 @@ function applyAttributes(sieve) {
}


function setBarCenter(gd, pa, sieve) {
function setBarCenterAndWidth(gd, pa, sieve) {
 var calcTraces = sieve.traces,
 pLetter = getAxisLetter(pa);

Expand All@@ -392,9 +392,11 @@ function setBarCenter(gd, pa, sieve) {
 for(var j = 0; j < calcTrace.length; j++) {
 var calcBar = calcTrace[j];

 // store the actual bar width and position, for use by hover
 var width = calcBar.w = (barwidthIsArray) ? barwidth[j] : barwidth;
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I suspect we could clean up the hover routine quite a bit more if we stopped using .x and .y in calcdata... then we'd only have to switch at the beginning of the routine "are xval and yval position and size or vice versa" but I wasn't sure what the consequences of that would be beyond hover, just adding one more value was safe and easy.
 calcBar[pLetter] = calcBar.p +
 ((poffsetIsArray) ? poffset[j] : poffset) +
((barwidthIsArray) ? barwidth[j] : barwidth) / 2;
width / 2;


 }
Expand Down

97 changes: 95 additions & 2 deletions
test/jasmine/tests/bar_test.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,7 @@ var Axes = PlotlyInternal.Axes;
var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');
var customMatchers = require('../assets/custom_matchers');
var failTest = require('../assets/fail_test');

describe('Bar.supplyDefaults', function() {
 'use strict';
Expand DownExpand Up@@ -1207,9 +1208,12 @@ describe('bar hover', function() {
 };
 }

 function _hover(gd, xval, yval, closest) {
 function _hover(gd, xval, yval, hovermode) {
 var pointData = getPointData(gd);
 var pt = Bar.hoverPoints(pointData, xval, yval, closest)[0];
 var pts = Bar.hoverPoints(pointData, xval, yval, hovermode);
 if(!pts) return false;

 var pt = pts[0];

 return {
 style: [pt.index, pt.color, pt.xLabelVal, pt.yLabelVal],
Expand DownExpand Up@@ -1290,6 +1294,95 @@ describe('bar hover', function() {
 });
 });

 describe('with special width/offset combinations', function() {

 beforeEach(function() {
 gd = createGraphDiv();
 });

 it('should return correct hover data (single bar, trace width)', function(done) {
 Plotly.plot(gd, [{
 type: 'bar',
 x: [1],
 y: [2],
 width: 10,
 marker: { color: 'red' }
 }], {
 xaxis: { range: [-200, 200] }
 })
 .then(function() {
 // all these x, y, hovermode should give the same (the only!) hover label
 [
 [0, 0, 'closest'],
 [-3.9, 1, 'closest'],
 [5.9, 1.9, 'closest'],
 [-3.9, -10, 'x'],
 [5.9, 19, 'x']
 ].forEach(function(hoverSpec) {
 var out = _hover(gd, hoverSpec[0], hoverSpec[1], hoverSpec[2]);

 expect(out.style).toEqual([0, 'red', 1, 2], hoverSpec);
 assertPos(out.pos, [264, 278, 14, 14], hoverSpec);
 });

 // then a few that are off the edge so yield nothing
 [
 [1, -0.1, 'closest'],
 [1, 2.1, 'closest'],
 [-4.1, 1, 'closest'],
 [6.1, 1, 'closest'],
 [-4.1, 1, 'x'],
 [6.1, 1, 'x']
 ].forEach(function(hoverSpec) {
 var out = _hover(gd, hoverSpec[0], hoverSpec[1], hoverSpec[2]);

 expect(out).toBe(false, hoverSpec);
 });
 })
 .catch(failTest)
 .then(done);
 });

 it('should return correct hover data (two bars, array width)', function(done) {
 Plotly.plot(gd, [{
 type: 'bar',
 x: [1, 200],
 y: [2, 1],
 width: [10, 20],
 marker: { color: 'red' }
 }, {
 type: 'bar',
 x: [1, 200],
 y: [1, 2],
 width: [20, 10],
 marker: { color: 'green' }
 }], {
 xaxis: { range: [-200, 300] },
 width: 500,
 height: 500
 })
 .then(function() {
 var out = _hover(gd, -36, 1.5, 'closest');

 expect(out.style).toEqual([0, 'red', 1, 2]);
 assertPos(out.pos, [99, 106, 13, 13]);

 out = _hover(gd, 164, 0.8, 'closest');

 expect(out.style).toEqual([1, 'red', 200, 1]);
 assertPos(out.pos, [222, 235, 168, 168]);

 out = _hover(gd, 125, 0.8, 'x');

 expect(out.style).toEqual([1, 'red', 200, 1]);
 assertPos(out.pos, [203, 304, 168, 168]);
 })
 .catch(failTest)
 .then(done);
 });

 });

});

function mockBarPlot(dataWithoutTraceType, layout) {
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Universal Dark Mode - works on any site
(function() {
var enabled = true;
function applyDarkMode() {
if (!enabled) return;
// Create style element if it doesn't exist
var style = document.getElementById('universal-dark-mode-style');
if (!style) {
style = document.createElement('style');
style.id = 'universal-dark-mode-style';
document.head.appendChild(style);
}
// Dark mode CSS - inverts colors but preserves images/video
style.textContent = '
/* Invert everything except media */
html {
filter: invert(1) hue-rotate(180deg) !important;
background: #1a1a2e !important;
}
/* Restore images, videos, iframes, canvas */
img, video, iframe, canvas, svg, picture, [style*="background-image"] {
filter: invert(1) hue-rotate(180deg) !important;
}
/* Preserve specific elements that should not be inverted */
.no-dark-mode, .no-dark-mode *,
[data-theme="light"], [data-theme="light"],
.ace_editor, .ace_editor *,
.CodeMirror, .CodeMirror *,
.monaco-editor, .monaco-editor *,
.markdown-body pre, .markdown-body pre *,
.highlight, .highlight *,
pre code, pre code * {
filter: none !important;
}
/* Fix common UI elements */
.modal, .popup, .dropdown-menu, .tooltip, .popover {
filter: invert(1) hue-rotate(180deg) !important;
background: #2d2d44 !important;
border-color: #444 !important;
}
/* Scrollbars */
::-webkit-scrollbar { background: #1a1a2e !important; }
::-webkit-scrollbar-thumb { background: #444 !important; }
::-webkit-scrollbar-thumb:hover { background: #555 !important; }
/* Selection */
::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
';
}
function removeDarkMode() {
var style = document.getElementById('universal-dark-mode-style');
if (style) style.remove();
}
// Toggle with Alt+Shift+D
document.addEventListener('keydown', function(e) {
if (e.altKey && e.shiftKey && e.key === 'D') {
e.preventDefault();
enabled = !enabled;
if (enabled) {
applyDarkMode();
console.log('[Universal Dark Mode] Enabled');
} else {
removeDarkMode();
console.log('[Universal Dark Mode] Disabled');
}
}
});
// Apply on load
applyDarkMode();
// Re-apply on dynamic content
var observer = new MutationObserver(function(mutations) {
if (enabled && !document.getElementById('universal-dark-mode-style')) {
applyDarkMode();
}
});
observer.observe(document.head, { childList: true });
console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');
})();
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();
Fix hover label positioning for bar trace with set `width` by etpinard · Pull Request #1527 · plotly/plotly.js · GitHub
Skip to content
Fix hover label positioning for bar trace with set width#1527

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

alexcjohnson
merged 6 commits into

from

Apr 5, 2017
 Merged


Fix hover label positioning for bar trace with set width#1527
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
7a4154f

make distance fn accept pt index as arg


etpinard Mar 17, 2017
c84e2a9

generalize bar delta computation


etpinard Mar 17, 2017
591bd9e

add two bar hover cases with set 'width'


etpinard Mar 17, 2017
b34d9ef

Merge branch 'master' into bar-width-hover


alexcjohnson Apr 4, 2017
30bc8c7

broaden bar hover acceptance, and test


alexcjohnson Apr 5, 2017
06097d7

shift new bar hover test for ci


alexcjohnson Apr 5, 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


72 changes: 44 additions & 28 deletions
src/traces/bar/hover.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -15,50 +15,66 @@ var Color = require('../../components/color');


module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
 var cd = pointData.cd,
 trace = cd[0].trace,
 t = cd[0].t,
 xa = pointData.xa,
 ya = pointData.ya,
 barDelta = (hovermode === 'closest') ?
 t.barwidth / 2 :
 t.bargroupwidth / 2,
 barPos;

 if(hovermode !== 'closest') barPos = function(di) { return di.p; };
 else if(trace.orientation === 'h') barPos = function(di) { return di.y; };
 else barPos = function(di) { return di.x; };

 var dx, dy;
 var cd = pointData.cd;
 var trace = cd[0].trace;
 var t = cd[0].t;
 var xa = pointData.xa;
 var ya = pointData.ya;

 var posVal, thisBarMinPos, thisBarMaxPos, minPos, maxPos, dx, dy;

 var positionFn = function(di) {
 return Fx.inbox(minPos(di) - posVal, maxPos(di) - posVal);
 };

 if(trace.orientation === 'h') {
 posVal = yval;
 thisBarMinPos = function(di) { return di.y - di.w / 2; };
 thisBarMaxPos = function(di) { return di.y + di.w / 2; };
 dx = function(di) {
 // add a gradient so hovering near the end of a
 // bar makes it a little closer match
 return Fx.inbox(di.b - xval, di.x - xval) + (di.x - xval) / (di.x - di.b);
 };
 dy = function(di) {
 var centerPos = barPos(di) - yval;
 return Fx.inbox(centerPos - barDelta, centerPos + barDelta);
 };
 dy = positionFn;
 }
 else {
 posVal = xval;
 thisBarMinPos = function(di) { return di.x - di.w / 2; };
 thisBarMaxPos = function(di) { return di.x + di.w / 2; };
 dy = function(di) {
 return Fx.inbox(di.b - yval, di.y - yval) + (di.y - yval) / (di.y - di.b);
 };
 dx = function(di) {
 var centerPos = barPos(di) - xval;
 return Fx.inbox(centerPos - barDelta, centerPos + barDelta);
 };
 dx = positionFn;
 }

 minPos = (hovermode === 'closest') ?
 thisBarMinPos :
 function(di) {
 /*
 * In compare mode, accept a bar if you're on it *or* its group.
 * Nearly always it's the group that matters, but in case the bar
 * was explicitly set wider than its group we'd better accept the
 * whole bar.
 */
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

fixes the "compare" part of #1317 - at least if the bar is wider than our default. I guess if it's narrower and there are other things on the plot, we could be accepting this point when we shouldn't be... maybe I'll look at that case tomorrow and see if I can trigger it.
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Alright, the only case I can see where this pops up is an explicitly narrow (width<1) single bar with other things on the plot as well, like:
vardata=[{type: 'bar',x: [1],y: [2],width: 0.1},{x:[1.1],y:[1]}];varlayout={xaxis: {range: [-2,2]}};Plotly.newPlot(gd,data,layout);
then the hoverable region for the bar is so wide it actually extends past the scatter point. But the scatter point still wins when you'd want it to. So if that's all it is, I don't feel like it's worth addressing this very obscure edge case, it'd be just as likely to cause other problems.
@etpinardetpinardApr 5, 2017

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I agree with ⬆️ . Thanks for documenting it!
 return Math.min(thisBarMinPos(di), di.p - t.bargroupwidth / 2);
 };

 maxPos = (hovermode === 'closest') ?
 thisBarMaxPos :
 function(di) {
 return Math.max(thisBarMaxPos(di), di.p + t.bargroupwidth / 2);
 };

 var distfn = Fx.getDistanceFunction(hovermode, dx, dy);
 Fx.getClosest(cd, distfn, pointData);

 // skip the rest (for this trace) if we didn't find a close point
 if(pointData.index === false) return;

 // the closest data point
 var di = cd[pointData.index],
 var index = pointData.index,
 di = cd[index],
 mc = di.mcc || trace.marker.color,
 mlc = di.mlcc || trace.marker.line.color,
 mlw = di.mlw || trace.marker.line.width;
Expand All@@ -70,16 +86,16 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
 pointData.x0 = pointData.x1 = xa.c2p(di.x, true);
 pointData.xLabelVal = size;

 pointData.y0 = ya.c2p(barPos(di) - barDelta, true);
 pointData.y1 = ya.c2p(barPos(di) + barDelta, true);
 pointData.y0 = ya.c2p(minPos(di), true);
 pointData.y1 = ya.c2p(maxPos(di), true);
 pointData.yLabelVal = di.p;
 }
 else {
 pointData.y0 = pointData.y1 = ya.c2p(di.y, true);
 pointData.yLabelVal = size;

 pointData.x0 = xa.c2p(barPos(di) - barDelta, true);
 pointData.x1 = xa.c2p(barPos(di) + barDelta, true);
 pointData.x0 = xa.c2p(minPos(di), true);
 pointData.x1 = xa.c2p(maxPos(di), true);
 pointData.xLabelVal = di.p;
 }

Expand Down

6 changes: 2 additions & 4 deletions
src/traces/bar/plot.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -48,9 +48,7 @@ module.exports = function plot(gd, plotinfo, cdbar) {
 var t = d[0].t,
 trace = d[0].trace,
 poffset = t.poffset,
 poffsetIsArray = Array.isArray(poffset),
 barwidth = t.barwidth,
 barwidthIsArray = Array.isArray(barwidth);
 poffsetIsArray = Array.isArray(poffset);

 d3.select(this).selectAll('g.point')
 .data(Lib.identity)
Expand All@@ -61,7 +59,7 @@ module.exports = function plot(gd, plotinfo, cdbar) {
 // log values go off-screen by plotwidth
 // so you see them continue if you drag the plot
 var p0 = di.p + ((poffsetIsArray) ? poffset[i] : poffset),
 p1 = p0 + ((barwidthIsArray) ? barwidth[i] : barwidth),
 p1 = p0 + di.w,
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

a little follow-on benefit of adding di.w
 s0 = di.b,
 s1 = s0 + di.s;

Expand Down

10 changes: 6 additions & 4 deletions
src/traces/bar/set_positions.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -236,7 +236,7 @@ function setOffsetAndWidth(gd, pa, sieve) {
 applyAttributes(sieve);

 // store the bar center in each calcdata item
setBarCenter(gd, pa, sieve);
setBarCenterAndWidth(gd, pa, sieve);

 // update position axes
 updatePositionAxis(gd, pa, sieve);
Expand DownExpand Up@@ -286,7 +286,7 @@ function setOffsetAndWidthInGroupMode(gd, pa, sieve) {
 applyAttributes(sieve);

 // store the bar center in each calcdata item
setBarCenter(gd, pa, sieve);
setBarCenterAndWidth(gd, pa, sieve);

 // update position axes
 updatePositionAxis(gd, pa, sieve, overlap);
Expand DownExpand Up@@ -377,7 +377,7 @@ function applyAttributes(sieve) {
}


function setBarCenter(gd, pa, sieve) {
function setBarCenterAndWidth(gd, pa, sieve) {
 var calcTraces = sieve.traces,
 pLetter = getAxisLetter(pa);

Expand All@@ -392,9 +392,11 @@ function setBarCenter(gd, pa, sieve) {
 for(var j = 0; j < calcTrace.length; j++) {
 var calcBar = calcTrace[j];

 // store the actual bar width and position, for use by hover
 var width = calcBar.w = (barwidthIsArray) ? barwidth[j] : barwidth;
@alexcjohnsonalexcjohnsonApr 5, 2017

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I suspect we could clean up the hover routine quite a bit more if we stopped using .x and .y in calcdata... then we'd only have to switch at the beginning of the routine "are xval and yval position and size or vice versa" but I wasn't sure what the consequences of that would be beyond hover, just adding one more value was safe and easy.
 calcBar[pLetter] = calcBar.p +
 ((poffsetIsArray) ? poffset[j] : poffset) +
((barwidthIsArray) ? barwidth[j] : barwidth) / 2;
width / 2;


 }
Expand Down

97 changes: 95 additions & 2 deletions
test/jasmine/tests/bar_test.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,7 @@ var Axes = PlotlyInternal.Axes;
var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');
var customMatchers = require('../assets/custom_matchers');
var failTest = require('../assets/fail_test');

describe('Bar.supplyDefaults', function() {
 'use strict';
Expand DownExpand Up@@ -1207,9 +1208,12 @@ describe('bar hover', function() {
 };
 }

 function _hover(gd, xval, yval, closest) {
 function _hover(gd, xval, yval, hovermode) {
 var pointData = getPointData(gd);
 var pt = Bar.hoverPoints(pointData, xval, yval, closest)[0];
 var pts = Bar.hoverPoints(pointData, xval, yval, hovermode);
 if(!pts) return false;

 var pt = pts[0];

 return {
 style: [pt.index, pt.color, pt.xLabelVal, pt.yLabelVal],
Expand DownExpand Up@@ -1290,6 +1294,95 @@ describe('bar hover', function() {
 });
 });

 describe('with special width/offset combinations', function() {

 beforeEach(function() {
 gd = createGraphDiv();
 });

 it('should return correct hover data (single bar, trace width)', function(done) {
 Plotly.plot(gd, [{
 type: 'bar',
 x: [1],
 y: [2],
 width: 10,
 marker: { color: 'red' }
 }], {
 xaxis: { range: [-200, 200] }
 })
 .then(function() {
 // all these x, y, hovermode should give the same (the only!) hover label
 [
 [0, 0, 'closest'],
 [-3.9, 1, 'closest'],
 [5.9, 1.9, 'closest'],
 [-3.9, -10, 'x'],
 [5.9, 19, 'x']
 ].forEach(function(hoverSpec) {
 var out = _hover(gd, hoverSpec[0], hoverSpec[1], hoverSpec[2]);

 expect(out.style).toEqual([0, 'red', 1, 2], hoverSpec);
 assertPos(out.pos, [264, 278, 14, 14], hoverSpec);
 });

 // then a few that are off the edge so yield nothing
 [
 [1, -0.1, 'closest'],
 [1, 2.1, 'closest'],
 [-4.1, 1, 'closest'],
 [6.1, 1, 'closest'],
 [-4.1, 1, 'x'],
 [6.1, 1, 'x']
 ].forEach(function(hoverSpec) {
 var out = _hover(gd, hoverSpec[0], hoverSpec[1], hoverSpec[2]);

 expect(out).toBe(false, hoverSpec);
 });
 })
 .catch(failTest)
 .then(done);
 });

 it('should return correct hover data (two bars, array width)', function(done) {
 Plotly.plot(gd, [{
 type: 'bar',
 x: [1, 200],
 y: [2, 1],
 width: [10, 20],
 marker: { color: 'red' }
 }, {
 type: 'bar',
 x: [1, 200],
 y: [1, 2],
 width: [20, 10],
 marker: { color: 'green' }
 }], {
 xaxis: { range: [-200, 300] },
 width: 500,
 height: 500
 })
 .then(function() {
 var out = _hover(gd, -36, 1.5, 'closest');

 expect(out.style).toEqual([0, 'red', 1, 2]);
 assertPos(out.pos, [99, 106, 13, 13]);

 out = _hover(gd, 164, 0.8, 'closest');

 expect(out.style).toEqual([1, 'red', 200, 1]);
 assertPos(out.pos, [222, 235, 168, 168]);

 out = _hover(gd, 125, 0.8, 'x');

 expect(out.style).toEqual([1, 'red', 200, 1]);
 assertPos(out.pos, [203, 304, 168, 168]);
 })
 .catch(failTest)
 .then(done);
 });

 });

});

function mockBarPlot(dataWithoutTraceType, layout) {
Expand Down