Merge pull request #2414 from plotly/bar-hover-width

push hover labels in to the bar group extent in compare mode
plotly · Feb 27, 2018 · 315272b · 315272b
2 parents 36b8483 + 47cfe32
commit 315272b
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 3 deletions.
diff --git a/src/traces/bar/hover.js b/src/traces/bar/hover.js
@@ -128,8 +128,9 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
     pointData[sizeLetter + '0'] = pointData[sizeLetter + '1'] = sa.c2p(di[sizeLetter], true);
     pointData[sizeLetter + 'LabelVal'] = size;
 
-    pointData[posLetter + '0'] = pa.c2p(minPos(di), true);
-    pointData[posLetter + '1'] = pa.c2p(maxPos(di), true);
+    var extent = t.extents[t.extents.round(di.p)];
+    pointData[posLetter + '0'] = pa.c2p(isClosest ? minPos(di) : extent[0], true);
+    pointData[posLetter + '1'] = pa.c2p(isClosest ? maxPos(di) : extent[1], true);
     pointData[posLetter + 'LabelVal'] = di.p;
 
     // spikelines always want "closest" distance regardless of hovermode

diff --git a/src/traces/bar/set_positions.js b/src/traces/bar/set_positions.js
@@ -105,6 +105,8 @@ function setGroupPositions(gd, pa, sa, calcTraces) {
             setGroupPositionsInOverlayMode(gd, pa, sa, excluded);
         }
     }
+
+    collectExtents(calcTraces, pa);
 }
 
 
@@ -596,3 +598,54 @@ function normalizeBars(gd, sa, sieve) {
 function getAxisLetter(ax) {
     return ax._id.charAt(0);
 }
+
+// find the full position span of bars at each position
+// for use by hover, to ensure labels move in if bars are
+// narrower than the space they're in.
+// run once per trace group (subplot & direction) and
+// the same mapping is attached to all calcdata traces
+function collectExtents(calcTraces, pa) {
+    var posLetter = pa._id.charAt(0);
+    var extents = {};
+    var pMin = Infinity;
+    var pMax = -Infinity;
+
+    var i, j, cd;
+    for(i = 0; i < calcTraces.length; i++) {
+        cd = calcTraces[i];
+        for(j = 0; j < cd.length; j++) {
+            var p = cd[j].p;
+            if(isNumeric(p)) {
+                pMin = Math.min(pMin, p);
+                pMax = Math.max(pMax, p);
+            }
+        }
+    }
+
+    // this is just for positioning of hover labels, and nobody will care if
+    // the label is 1px too far out; so round positions to 1/10K in case
+    // position values don't exactly match from trace to trace
+    var roundFactor = 10000 / (pMax - pMin);
+    var round = extents.round = function(p) {
+        return String(Math.round(roundFactor * (p - pMin)));
+    };
+
+    for(i = 0; i < calcTraces.length; i++) {
+        cd = calcTraces[i];
+        cd[0].t.extents = extents;
+        for(j = 0; j < cd.length; j++) {
+            var di = cd[j];
+            var p0 = di[posLetter] - di.w / 2;
+            if(isNumeric(p0)) {
+                var p1 = di[posLetter] + di.w / 2;
+                var pVal = round(di.p);
+                if(extents[pVal]) {
+                    extents[pVal] = [Math.min(p0, extents[pVal][0]), Math.max(p1, extents[pVal][1])];
+                }
+                else {
+                    extents[pVal] = [p0, p1];
+                }
+            }
+        }
+    }
+}
diff --git a/test/jasmine/tests/bar_test.js b/test/jasmine/tests/bar_test.js
@@ -1508,7 +1508,37 @@ describe('bar hover', function() {
                 out = _hover(gd, 125, 0.8, 'x');
 
                 expect(out.style).toEqual([1, 'red', 200, 1]);
-                assertPos(out.pos, [203, 304, 168, 168]);
+                assertPos(out.pos, [222, 280, 168, 168]);
+            })
+            .catch(fail)
+            .then(done);
+        });
+
+        it('positions labels correctly w.r.t. narrow bars', function(done) {
+            Plotly.newPlot(gd, [{
+                x: [0, 10, 20],
+                y: [1, 3, 2],
+                type: 'bar',
+                width: 1
+            }], {
+                width: 500,
+                height: 500,
+                margin: {l: 100, r: 100, t: 100, b: 100}
+            })
+            .then(function() {
+                // you can still hover over the gap (14) but the label will
+                // get pushed in to the bar
+                var out = _hover(gd, 14, 2, 'x');
+                assertPos(out.pos, [145, 155, 15, 15]);
+
+                // in closest mode you must be over the bar though
+                out = _hover(gd, 14, 2, 'closest');
+                expect(out).toBe(false);
+
+                // now for a single bar trace, closest and compare modes give the same
+                // positioning of hover labels
+                out = _hover(gd, 10, 2, 'closest');
+                assertPos(out.pos, [145, 155, 15, 15]);
             })
             .catch(fail)
             .then(done);