fix and test finance hover labels - including test for #1807

plotly · alexcjohnson · Apr 17, 2018 · Apr 5, 2018 · Apr 6, 2018 · Apr 10, 2018
commit 3509b99223cf17d3770b5e56f8fbed0deae863cb
diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js
@@ -819,13 +819,6 @@ function createHoverText(hoverData, opts, gd) {
             }
         }
 
-        // used by other modules (initially just ternary) that
-        // manage their own hoverinfo independent of cleanPoint
-        // the rest of this will still apply, so such modules
-        // can still put things in (x|y|z)Label, text, and name
-        // and hoverinfo will still determine their visibility
-        if(d.extraText !== undefined) text += d.extraText;
-
         if(d.zLabel !== undefined) {
             if(d.xLabel !== undefined) text += 'x: ' + d.xLabel + '<br>';
             if(d.yLabel !== undefined) text += 'y: ' + d.yLabel + '<br>';
@@ -844,6 +837,13 @@ function createHoverText(hoverData, opts, gd) {
             text += (text ? '<br>' : '') + d.text;
         }
 
+        // used by other modules (initially just ternary) that
+        // manage their own hoverinfo independent of cleanPoint
+        // the rest of this will still apply, so such modules
+        // can still put things in (x|y|z)Label, text, and name
+        // and hoverinfo will still determine their visibility
+        if(d.extraText !== undefined) text += (text ? '<br>' : '') + d.extraText;
+
         // if 'text' is empty at this point,
         // put 'name' in main label and don't show secondary label
         if(text === '') {

diff --git a/src/traces/ohlc/hover.js b/src/traces/ohlc/hover.js
@@ -83,24 +83,29 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
         return t.labels[attr] + Axes.hoverLabelText(ya, trace[attr][i]);
     }
 
-    var textParts = [
+    var hoverinfo = trace.hoverinfo;
+    var hoverParts = hoverinfo.split('+');
+    var isAll = hoverinfo === 'all';
+    var hasY = isAll || hoverParts.indexOf('y') !== -1;
+    var hasText = isAll || hoverParts.indexOf('text') !== -1;
+
+    var textParts = hasY ? [
         getLabelLine('open'),
         getLabelLine('high'),
         getLabelLine('low'),
-        getLabelLine('close')
-    ];
-    fillHoverText(di, trace, textParts);
-    pointData.text = textParts.join('<br>');
+        getLabelLine('close') + '  ' + DIRSYMBOL[dir]
+    ] : [];
+    if(hasText) fillHoverText(di, trace, textParts);
+
+    // don't make .yLabelVal or .text, since we're managing hoverinfo
+    // put it all in .extraText
+    pointData.extraText = textParts.join('<br>');
 
     // this puts the label at the midpoint of the box, ie
     // halfway between open and close, not between high and low.
     // TODO: the spike also links to this point, whereas previously
     // it linked to close. Is one better?
     pointData.y0 = pointData.y1 = ya.c2p(di.yc, true);
 
-    // indicate increasing/decreasing in the "name" field
-    // TODO: only shows up if name is displayed, ie multiple traces.
-    pointData.name += '<br>' + DIRSYMBOL[dir];
-
     return [pointData];
 };