[Bug] Manual width breaks hover boxes

[lewisjb]

Example: http://codepen.io/etpinard/pen/VPPvpM

Two cases:
1 (Top example). Manual width > automatic: Hover boxes don't show up
2 (Bottom example). Manual width < automatic: Hover boxes not aligned properly

Related: #1308

[alexcjohnson]

Nice find. In the top example the hover box does show up... if you hover over exactly the middle of the bar. The issue is that we're not taking the manual width into account in determining the region you can hover over to see the label. We'll have to be careful with how this plays with grouped bars (which can mean that the hoverable region is quite different from the bar itself) and note that the problem is only in "compare data" mode - in "closest data" mode it works as expected.

[hy9be]

Run into the same problem. Is anyone working on this issue already? Thanks!

[etpinard]

Is anyone working on this issue already? Thanks

@hy9be no one that I'm aware of. Want to give a shot?

[hy9be]

@etpinard Why not. I will see what I could figure out. Any suggestions for directions or where should I start? Looks tricky by a quick glance.

[etpinard]

The problem is in https://github.com/plotly/plotly.js/blob/master/src/traces/bar/hover.js where some fields computed in https://github.com/plotly/plotly.js/blob/master/src/traces/bar/set_positions.js are not used properly in the case of custom width.

[lewisjb]

I had a look into it, it's due to hover using bargroupwidth, which seems to always be 0.8.
But, due to it being a fairly new addition, I wasn't entirely sure about what it's purpose is. I created a test case under the assumption that bargroupwidth should be the same as barwidth when using a custom width; but when I made the changes in set_positions.js, a few other tests failed as they expect bargroupwidth to be 0.8. I haven't looked further into it yet

[hy9be]

I took a look at the "closest" hovermode only. The problem is t.barwidth at https://github.com/plotly/plotly.js/blob/master/src/traces/bar/hover.js#L24, is an array of the widths of all bars when manual width is set. Thus barDelta results in a NaN. My solution is that, find the width of the closest bar to the xval and yval hovering point.

My change worked and passed the tests, but the code does not look very efficient to me thus I do not want to submit a PR yet:

module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
//...
    function closestIndex(list, x) {
        var min,
            chosen = 0;
        for (var i in list) {
            min = Math.abs(list[chosen] - x);
            if (Math.abs(list[i] - x) < min) {
                chosen = i;
            }
        }
        return chosen;
    }

    var dx, dy;
    if(trace.orientation === 'h') {
        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);
        };

        if (isNaN(barDelta)) {
            barDelta = t.barwidth[closestIndex(cd.map(function(elem) {return elem.y}), yval)]/2;
        }
    }
    else {
        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);
        };
        if (isNaN(barDelta)) {
            barDelta = t.barwidth[closestIndex(cd.map(function(elem) {return elem.x}), xval)]/2;
        }
    }
//...

Any suggestion? @etpinard

[etpinard]

@hy9be thanks for the code snippet. I'll try to revisit this at some point this week. 👍

[etpinard]

@hy9be I pushed a few commits to bar-width-hover inspired by your technique.

I think this is the desired behavior for the hovermode: 'closest' though I haven't had the time to check it with other bar attribute combinations (e.g. with other barmode, base, orientation, bargap, bargroupgap ... etc.). Would you mind helping me test out that bar-width-hover branch?

As for the other possible hovermode values, I believe the current situation is less broken as even maybe desirable. For example:

Here the y labels appear at the edge of the bar group, even the bars don't reach it. We could obviously make the y labels appear at Math.min(/* edge of bar group */, /* bar extend in that group */) instead. What do you think?

[hy9be]

@etpinard Sorry for the late response. I tested the hovermode: 'closest' cases on your branch with single bar, grouped bars, and stacked bars cases, and combining with different sets of bargap, bargroupgap values. All cases are good.

[etpinard]

@hy9be no worries. Thanks very much for looking at it!

This patch will be part of the next minor release 1.26.0

[hy9be]

@etpinard Thanks! And I agree with you that the behavior on the other hovermode may be desirable. We can see if there are more people report this behavior as a problem.

Thanks for making the fix!

[ankuradhey]

Behavior is very much desirable for other hovermode. Is anyone working on this issue?