Merge pull request #2055 from plotly/arrayok-hoverinfo-fixups

ArrayOk hoverinfo fixups

Author: etpinard

src/components/drawing/index.js

@@ -443,12 +443,10 @@ drawing.tryColorscale = function(marker, prefix) {
 var TEXTOFFSETSIGN = {start: 1, end: -1, middle: 0, bottom: 1, top: -1};
 drawing.textPointStyle = function(s, trace, gd) {
     s.each(function(d) {
-        var p = d3.select(this),
-            text = d.tx || trace.text;
+        var p = d3.select(this);
+        var text = Lib.extractOption(d, trace, 'tx', 'text');
 
-        if(!text || Array.isArray(text)) {
-            // isArray test handles the case of (intentionally) missing
-            // or empty text within a text array
+        if(!text) {
             p.remove();
             return;
         }
@@ -889,6 +887,9 @@ drawing.setTextPointsScale = function(selection, xScale, yScale) {
         var transforms;
         var el = d3.select(this);
         var text = el.select('text');
+
+        if(!text.node()) return;
+
         var x = parseFloat(text.attr('x') || 0);
         var y = parseFloat(text.attr('y') || 0);
 

src/components/fx/hover.js

@@ -1028,24 +1028,22 @@ function alignHoverText(hoverLabels, rotateLabels) {
 }
 
 function cleanPoint(d, hovermode) {
+    var index = d.index;
     var trace = d.trace || {};
     var cd0 = d.cd[0];
-    var cd = d.cd[d.index] || {};
+    var cd = d.cd[index] || {};
+
+    var getVal = Array.isArray(index) ?
+        function(calcKey, traceKey) {
+            return Lib.castOption(cd0, index, calcKey) ||
+                Lib.extractOption({}, trace, '', traceKey);
+        } :
+        function(calcKey, traceKey) {
+            return Lib.extractOption(cd, trace, calcKey, traceKey);
+        };
 
     function fill(key, calcKey, traceKey) {
-        var val;
-
-        if(cd[calcKey]) {
-            val = cd[calcKey];
-        } else if(cd0[calcKey]) {
-            var arr = cd0[calcKey];
-            if(Array.isArray(arr) && Array.isArray(arr[d.index[0]])) {
-                val = arr[d.index[0]][d.index[1]];
-            }
-        } else {
-            val = Lib.nestedProperty(trace, traceKey).get();
-        }
-
+        var val = getVal(calcKey, traceKey);
         if(val) d[key] = val;
     }
 

src/lib/index.js

@@ -437,6 +437,26 @@ lib.castOption = function(trace, ptNumber, astr, fn) {
     }
 };
 
+/** Extract option from calcdata item, correctly falling back to
+ *  trace value if not found.
+ *
+ *  @param {object} calcPt : calcdata[i][j] item
+ *  @param {object} trace : (full) trace object
+ *  @param {string} calcKey : calcdata key
+ *  @param {string} traceKey : aka trace attribute string
+ *  @return {any}
+ */
+lib.extractOption = function(calcPt, trace, calcKey, traceKey) {
+    if(calcKey in calcPt) return calcPt[calcKey];
+
+    // fallback to trace value,
+    //   must check if value isn't itself an array
+    //   which means the trace attribute has a corresponding
+    //   calcdata key, but its value is falsy
+    var traceVal = lib.nestedProperty(trace, traceKey).get();
+    if(!Array.isArray(traceVal)) return traceVal;
+};
+
 /** Returns target as set by 'target' transform attribute
  *
  * @param {object} trace : full trace object

src/traces/bar/hover.js

@@ -12,7 +12,7 @@
 var Fx = require('../../components/fx');
 var ErrorBars = require('../../components/errorbars');
 var Color = require('../../components/color');
-
+var fillHoverText = require('../scatter/fill_hover_text');
 
 module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
     var cd = pointData.cd;
@@ -99,11 +99,7 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
         pointData.xLabelVal = di.p;
     }
 
-    if(di.htx) pointData.text = di.htx;
-    else if(trace.hovertext) pointData.text = trace.hovertext;
-    else if(di.tx) pointData.text = di.tx;
-    else if(trace.text) pointData.text = trace.text;
-
+    fillHoverText(di, trace, pointData);
     ErrorBars.hoverInfo(di, trace, pointData);
 
     return [pointData];

src/traces/choropleth/attributes.js

@@ -34,11 +34,9 @@ module.exports = extendFlat({
         editType: 'calc',
         description: 'Sets the color values.'
     },
-    text: {
-        valType: 'data_array',
-        editType: 'calc',
+    text: extendFlat({}, ScatterGeoAttrs.text, {
         description: 'Sets the text elements associated with each location.'
-    },
+    }),
     marker: {
         line: {
             color: ScatterGeoMarkerLineAttrs.color,

src/traces/choropleth/hover.js

@@ -11,6 +11,7 @@
 
 var Axes = require('../../plots/cartesian/axes');
 var attributes = require('./attributes');
+var fillHoverText = require('../scatter/fill_hover_text');
 
 module.exports = function hoverPoints(pointData, xval, yval) {
     var cd = pointData.cd;
@@ -53,17 +54,17 @@ module.exports = function hoverPoints(pointData, xval, yval) {
 };
 
 function makeHoverInfo(pointData, trace, pt, axis) {
-    var hoverinfo = trace.hoverinfo;
+    var hoverinfo = pt.hi || trace.hoverinfo;
 
     var parts = (hoverinfo === 'all') ?
         attributes.hoverinfo.flags :
         hoverinfo.split('+');
 
-    var hasName = (parts.indexOf('name') !== -1),
-        hasLocation = (parts.indexOf('location') !== -1),
-        hasZ = (parts.indexOf('z') !== -1),
-        hasText = (parts.indexOf('text') !== -1),
-        hasIdAsNameLabel = !hasName && hasLocation;
+    var hasName = (parts.indexOf('name') !== -1);
+    var hasLocation = (parts.indexOf('location') !== -1);
+    var hasZ = (parts.indexOf('z') !== -1);
+    var hasText = (parts.indexOf('text') !== -1);
+    var hasIdAsNameLabel = !hasName && hasLocation;
 
     var text = [];
 
@@ -79,7 +80,9 @@ function makeHoverInfo(pointData, trace, pt, axis) {
     }
 
     if(hasZ) text.push(formatter(pt.z));
-    if(hasText) text.push(pt.tx);
+    if(hasText) {
+        fillHoverText(pt, trace, text);
+    }
 
     pointData.extraText = text.join('<br>');
 }

src/traces/scatter/fill_hover_text.js

@@ -0,0 +1,41 @@
+/**
+* Copyright 2012-2017, Plotly, Inc.
+* All rights reserved.
+*
+* This source code is licensed under the MIT license found in the
+* LICENSE file in the root directory of this source tree.
+*/
+
+'use strict';
+
+var Lib = require('../../lib');
+
+/** Fill hover 'pointData' container with 'correct' hover text value
+ *
+ * - If trace hoverinfo contains a 'text' flag and hovertext is not set,
+ *   the text elements will be seen in the hover labels.
+ *
+ * - If trace hoverinfo contains a 'text' flag and hovertext is set,
+ *   hovertext takes precedence over text
+ *   i.e. the hoverinfo elements will be seen in the hover labels
+ *
+ *  @param {object} calcPt
+ *  @param {object} trace
+ *  @param {object || array} contOut (mutated here)
+ */
+module.exports = function fillHoverText(calcPt, trace, contOut) {
+    var fill = Array.isArray(contOut) ?
+        function(v) { contOut.push(v); } :
+        function(v) { contOut.text = v; };
+
+    var htx = Lib.extractOption(calcPt, trace, 'htx', 'hovertext');
+    if(isValid(htx)) return fill(htx);
+
+    var tx = Lib.extractOption(calcPt, trace, 'tx', 'text');
+    if(isValid(tx)) return fill(tx);
+};
+
+// accept all truthy values and 0 (which gets cast to '0' in the hover labels)
+function isValid(v) {
+    return v || v === 0;
+}

src/traces/scatter/hover.js

@@ -6,14 +6,14 @@
 * LICENSE file in the root directory of this source tree.
 */
 
-
 'use strict';
 
 var Lib = require('../../lib');
 var Fx = require('../../components/fx');
 var ErrorBars = require('../../components/errorbars');
 var getTraceColor = require('./get_trace_color');
 var Color = require('../../components/color');
+var fillHoverText = require('./fill_hover_text');
 
 var MAXDIST = Fx.constants.MAXDIST;
 
@@ -73,11 +73,7 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
                 yLabelVal: di.y
             });
 
-            if(di.htx) pointData.text = di.htx;
-            else if(trace.hovertext) pointData.text = trace.hovertext;
-            else if(di.tx) pointData.text = di.tx;
-            else if(trace.text) pointData.text = trace.text;
-
+            fillHoverText(di, trace, pointData);
             ErrorBars.hoverInfo(di, trace, pointData);
 
             return [pointData];

src/traces/scattercarpet/hover.js

@@ -46,18 +46,27 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
     newPointData.yLabelVal = undefined;
     // TODO: nice formatting, and label by axis title, for a, b, and c?
 
-    var trace = newPointData.trace,
-        carpet = trace._carpet,
-        hoverinfo = trace.hoverinfo.split('+'),
-        text = [];
+    var trace = newPointData.trace;
+    var carpet = trace._carpet;
+    var hoverinfo = cdi.hi || trace.hoverinfo;
+    var parts = hoverinfo.split('+');
+    var text = [];
 
     function textPart(ax, val) {
-        text.push(((ax.labelprefix && ax.labelprefix.length > 0) ? ax.labelprefix : (ax._hovertitle + ': ')) + val.toFixed(3) + ax.labelsuffix);
+        var prefix;
+
+        if(ax.labelprefix && ax.labelprefix.length > 0) {
+            prefix = ax.labelprefix.replace(/ = $/, '');
+        } else {
+            prefix = ax._hovertitle;
+        }
+
+        text.push(prefix + ': ' + val.toFixed(3) + ax.labelsuffix);
     }
 
-    if(hoverinfo.indexOf('all') !== -1) hoverinfo = ['a', 'b'];
-    if(hoverinfo.indexOf('a') !== -1) textPart(carpet.aaxis, cdi.a);
-    if(hoverinfo.indexOf('b') !== -1) textPart(carpet.baxis, cdi.b);
+    if(parts.indexOf('all') !== -1) parts = ['a', 'b'];
+    if(parts.indexOf('a') !== -1) textPart(carpet.aaxis, cdi.a);
+    if(parts.indexOf('b') !== -1) textPart(carpet.baxis, cdi.b);
 
     var ij = carpet.ab2ij([cdi.a, cdi.b]);
     var i0 = Math.floor(ij[0]);

src/traces/scattergeo/hover.js

@@ -14,9 +14,9 @@ var Axes = require('../../plots/cartesian/axes');
 var BADNUM = require('../../constants/numerical').BADNUM;
 
 var getTraceColor = require('../scatter/get_trace_color');
+var fillHoverText = require('../scatter/fill_hover_text');
 var attributes = require('./attributes');
 
-
 module.exports = function hoverPoints(pointData, xval, yval) {
     var cd = pointData.cd;
     var trace = cd[0].trace;
@@ -71,39 +71,34 @@ module.exports = function hoverPoints(pointData, xval, yval) {
 };
 
 function getExtraText(trace, pt, axis) {
-    var hoverinfo = trace.hoverinfo;
+    var hoverinfo = pt.hi || trace.hoverinfo;
 
-    var parts = (hoverinfo === 'all') ?
+    var parts = hoverinfo === 'all' ?
         attributes.hoverinfo.flags :
         hoverinfo.split('+');
 
-    var hasLocation = parts.indexOf('location') !== -1 && Array.isArray(trace.locations),
-        hasLon = (parts.indexOf('lon') !== -1),
-        hasLat = (parts.indexOf('lat') !== -1),
-        hasText = (parts.indexOf('text') !== -1);
-
+    var hasLocation = parts.indexOf('location') !== -1 && Array.isArray(trace.locations);
+    var hasLon = (parts.indexOf('lon') !== -1);
+    var hasLat = (parts.indexOf('lat') !== -1);
+    var hasText = (parts.indexOf('text') !== -1);
     var text = [];
 
     function format(val) {
         return Axes.tickText(axis, axis.c2l(val), 'hover').text + '\u00B0';
     }
 
-    if(hasLocation) text.push(pt.loc);
-    else if(hasLon && hasLat) {
+    if(hasLocation) {
+        text.push(pt.loc);
+    } else if(hasLon && hasLat) {
         text.push('(' + format(pt.lonlat[0]) + ', ' + format(pt.lonlat[1]) + ')');
+    } else if(hasLon) {
+        text.push('lon: ' + format(pt.lonlat[0]));
+    } else if(hasLat) {
+        text.push('lat: ' + format(pt.lonlat[1]));
     }
-    else if(hasLon) text.push('lon: ' + format(pt.lonlat[0]));
-    else if(hasLat) text.push('lat: ' + format(pt.lonlat[1]));
 
     if(hasText) {
-        var tx;
-
-        if(pt.htx) tx = pt.htx;
-        else if(trace.hovertext) tx = trace.hovertext;
-        else if(pt.tx) tx = pt.tx;
-        else if(trace.text) tx = trace.text;
-
-        if(!Array.isArray(tx)) text.push(tx);
+        fillHoverText(pt, trace, text);
     }
 
     return text.join('<br>');