Skip to content

ArrayOk hoverinfo fixups #2055

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.

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
merged 13 commits into from
Oct 5, 2017
Merged
11 changes: 6 additions & 5 deletions src/components/drawing/index.js
Original file line number Diff line number Diff line change
@@ -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;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Caught another bug. dragmode: 'pan' is broken on http://rickyreusser.com/plotly-mock-viewer/#text_chart_arrays

This fixes it:

peek 2017-10-04 18-24

Copy link
Collaborator

Choose a reason for hiding this comment

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

ha, good catch! 🎉


var x = parseFloat(text.attr('x') || 0);
var y = parseFloat(text.attr('y') || 0);

26 changes: 12 additions & 14 deletions src/components/fx/hover.js
Original file line number Diff line number Diff line change
@@ -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;
}

20 changes: 20 additions & 0 deletions src/lib/index.js
Original file line number Diff line number Diff line change
@@ -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;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice, I like it. Thanks for accepting 0.

not super happy with the name

meh, it's a very specific concept that we probably don't want to go into any more detail with in the name. Seems fine to me.

not super happy about it being on Lib

It definitely belongs next to castOption but yeah, if we make a common/ that might be more appropriate for these.

Also a style question: I just noticed all over this file we start the docs immediately after /**, I thought the convention was to leave that first line blank and start on the next line?

Copy link
Contributor Author

@etpinard etpinard Oct 4, 2017

Choose a reason for hiding this comment

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

Also a style question: I just noticed all over this file we start the docs immediately after /**, I thought the convention was to leave that first line blank and start on the next line?

image

from http://usejsdoc.org/howto-commonjs-modules.html

So, I think both are acceptable.

Copy link
Collaborator

Choose a reason for hiding this comment

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

perhaps overly pedantic, but I read that as /** Single-line description */ or

/**
 * Multiline still looks like it always
 * leaves the first line blank.
 */

};

/** Returns target as set by 'target' transform attribute
*
* @param {object} trace : full trace object
8 changes: 2 additions & 6 deletions src/traces/bar/hover.js
Original file line number Diff line number Diff line change
@@ -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];
6 changes: 2 additions & 4 deletions src/traces/choropleth/attributes.js
Original file line number Diff line number Diff line change
@@ -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,
17 changes: 10 additions & 7 deletions src/traces/choropleth/hover.js
Original file line number Diff line number Diff line change
@@ -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>');
}
41 changes: 41 additions & 0 deletions src/traces/scatter/fill_hover_text.js
Original file line number Diff line number Diff line change
@@ -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;
}
8 changes: 2 additions & 6 deletions src/traces/scatter/hover.js
Original file line number Diff line number Diff line change
@@ -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];
25 changes: 17 additions & 8 deletions src/traces/scattercarpet/hover.js
Original file line number Diff line number Diff line change
@@ -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]);
35 changes: 15 additions & 20 deletions src/traces/scattergeo/hover.js
Original file line number Diff line number Diff line change
@@ -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>');
31 changes: 14 additions & 17 deletions src/traces/scattermapbox/hover.js
Original file line number Diff line number Diff line change
@@ -11,6 +11,7 @@

var Fx = require('../../components/fx');
var getTraceColor = require('../scatter/get_trace_color');
var fillHoverText = require('../scatter/fill_hover_text');
var BADNUM = require('../../constants/numerical').BADNUM;

module.exports = function hoverPoints(pointData, xval, yval) {
@@ -66,13 +67,14 @@ module.exports = function hoverPoints(pointData, xval, yval) {
};

function getExtraText(trace, di) {
var hoverinfo = trace.hoverinfo.split('+'),
isAll = (hoverinfo.indexOf('all') !== -1),
hasLon = (hoverinfo.indexOf('lon') !== -1),
hasLat = (hoverinfo.indexOf('lat') !== -1);
var hoverinfo = di.hi || trace.hoverinfo;
var parts = hoverinfo.split('+');
var isAll = parts.indexOf('all') !== -1;
var hasLon = parts.indexOf('lon') !== -1;
var hasLat = parts.indexOf('lat') !== -1;

var lonlat = di.lonlat,
text = [];
var lonlat = di.lonlat;
var text = [];

// TODO should we use a mock axis to format hover?
// If so, we'll need to make precision be zoom-level dependent
@@ -82,19 +84,14 @@ function getExtraText(trace, di) {

if(isAll || (hasLon && hasLat)) {
text.push('(' + format(lonlat[0]) + ', ' + format(lonlat[1]) + ')');
} else if(hasLon) {
text.push('lon: ' + format(lonlat[0]));
} else if(hasLat) {
text.push('lat: ' + format(lonlat[1]));
}
else if(hasLon) text.push('lon: ' + format(lonlat[0]));
else if(hasLat) text.push('lat: ' + format(lonlat[1]));

if(isAll || hoverinfo.indexOf('text') !== -1) {
var tx;

if(di.htx) tx = di.htx;
else if(trace.hovertext) tx = trace.hovertext;
else if(di.tx) tx = di.tx;
else if(trace.text) tx = trace.text;

if(!Array.isArray(tx)) text.push(tx);
if(isAll || parts.indexOf('text') !== -1) {
fillHoverText(di, trace, text);
}

return text.join('<br>');
Loading