Skip to content

Fix namelength=0 case + implement namelength to loneHover traces #3734

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.

Sign up for GitHub

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 8 commits into from
Apr 9, 2019
Merged

Fix namelength=0 case + implement namelength to loneHover traces #3734

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d473560
fix namelength=0 case
etpinard Apr 5, 2019
08a0e22
add noHover trace module category,
etpinard Apr 8, 2019
2e50941
fix #3058 - prune-out base attrs based on module categories
etpinard Apr 8, 2019
e9f8403
honour hoverlabel.namelength when hovertemplate is set
etpinard Apr 8, 2019
918d22e
implement hoverlabel.namelength for traces using Fx.loneHover
etpinard Apr 8, 2019
08d3fe0
Merge branch 'master' into namelength-zero-fix
etpinard Apr 8, 2019
7f1645d
handle hoverlabel.namelength in Fx.multiHovers
etpinard Apr 8, 2019
b2975c9
split `namelength` test to reduce flakiness
etpinard Apr 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/components/fx/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ module.exports = {
namelength: {
valType: 'integer',
min: -1,
dflt: 15,
arrayOk: true,
role: 'style',
editType: 'none',
Expand All @@ -47,7 +48,8 @@ module.exports = {
'regardless of length. 0-3 shows the first 0-3 characters, and',
'an integer >3 will show the whole name if it is less than that',
'many characters, but if it is longer, will truncate to',
'`namelength - 3` characters and add an ellipsis.'
'`namelength - 3` characters and add an ellipsis.',
'Note that when `hovertemplate` is set, `namelength` defaults to *-1*.'
].join(' ')
},
editType: 'calc'
Expand Down
5 changes: 4 additions & 1 deletion src/components/fx/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,8 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
return Lib.coerce(traceIn, traceOut, attributes, attr, dflt);
}

handleHoverLabelDefaults(traceIn, traceOut, coerce, layout.hoverlabel);
var opts = Lib.extendFlat({}, layout.hoverlabel);
if(traceOut.hovertemplate) opts.namelength = -1;

handleHoverLabelDefaults(traceIn, traceOut, coerce, opts);
};
30 changes: 20 additions & 10 deletions src/components/fx/hover.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ exports.loneHover = function loneHover(hoverItem, opts) {
fontFamily: hoverItem.fontFamily,
fontSize: hoverItem.fontSize,
fontColor: hoverItem.fontColor,
nameLength: hoverItem.nameLength,

// filler to make createHoverText happy
trace: hoverItem.trace || {
Expand Down Expand Up @@ -933,11 +934,7 @@ function createHoverText(hoverData, opts, gd) {
if(fullLayout.meta) {
d.name = Lib.templateString(d.name, {meta: fullLayout.meta});
}

name = svgTextUtils.plainText(d.name || '', {
len: d.nameLength,
allowedTags: ['br', 'sub', 'sup', 'b', 'i', 'em']
});
name = plainText(d.name, d.nameLength);
}

if(d.zLabel !== undefined) {
Expand Down Expand Up @@ -989,8 +986,10 @@ function createHoverText(hoverData, opts, gd) {
);

text = text.replace(EXTRA_STRING_REGEX, function(match, extra) {
name = extra; // Assign name for secondary text label
return ''; // Remove from main text label
// assign name for secondary text label
name = plainText(extra, d.nameLength);
// remove from main text label
return '';
});
}

Expand Down Expand Up @@ -1334,18 +1333,22 @@ function cleanPoint(d, hovermode) {
var cd0 = d.cd[0];
var cd = d.cd[index] || {};

function pass(v) {
return v || (isNumeric(v) && v === 0);
}

var getVal = Array.isArray(index) ?
function(calcKey, traceKey) {
return Lib.castOption(cd0, index, calcKey) ||
Lib.extractOption({}, trace, '', traceKey);
var v = Lib.castOption(cd0, index, calcKey);
return pass(v) ? v : Lib.extractOption({}, trace, '', traceKey);
} :
function(calcKey, traceKey) {
return Lib.extractOption(cd, trace, calcKey, traceKey);
};

function fill(key, calcKey, traceKey) {
var val = getVal(calcKey, traceKey);
if(val) d[key] = val;
if(pass(val)) d[key] = val;
}

fill('hoverinfo', 'hi', 'hoverinfo');
Expand Down Expand Up @@ -1613,3 +1616,10 @@ function spikesChanged(gd, oldspikepoints) {
) return true;
return false;
}

function plainText(s, len) {
return svgTextUtils.plainText(s || '', {
len: len,
allowedTags: ['br', 'sub', 'sup', 'b', 'i', 'em']
});
}
2 changes: 1 addition & 1 deletion src/lib/svg_text_utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ var COLORMATCH = /(^|;)\s*color:/;
*
* @param {string} _str : input string
* @param {object} opts :
* - maxLen {number} max length of output string
* - len {number} max length of output string
* - allowedTags {array} list of pseudo-html tags to NOT strip
* @return {string}
*/
Expand Down
20 changes: 17 additions & 3 deletions src/plot_api/plot_schema.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,8 +453,7 @@ function getTraceAttributes(type) {
if(type === 'area') {
_module = { attributes: polarAreaAttrs };
basePlotModule = {};
}
else {
} else {
_module = Registry.modules[type]._module,
basePlotModule = _module.basePlotModule;
}
Expand All @@ -464,7 +463,6 @@ function getTraceAttributes(type) {
// make 'type' the first attribute in the object
attributes.type = null;


var copyBaseAttributes = extendDeepAll({}, baseAttributes);
var copyModuleAttributes = extendDeepAll({}, _module.attributes);

Expand All @@ -478,6 +476,22 @@ function getTraceAttributes(type) {
// base attributes (same for all trace types)
extendDeepAll(attributes, copyBaseAttributes);

// prune-out base attributes based on trace module categories
if(Registry.traceIs(type, 'noOpacity')) {
delete attributes.opacity;
}
if(!Registry.traceIs(type, 'showLegend')) {
delete attributes.showlegend;
delete attributes.legendgroup;
}
if(Registry.traceIs(type, 'noHover')) {
delete attributes.hoverinfo;
delete attributes.hoverlabel;
}
if(!_module.selectPoints) {
delete attributes.selectedpoints;
}

// module attributes
extendDeepAll(attributes, copyModuleAttributes);

Expand Down
3 changes: 2 additions & 1 deletion src/plots/gl2d/scene2d.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,8 @@ proto.draw = function() {
borderColor: Fx.castHoverOption(trace, ptNumber, 'bordercolor'),
fontFamily: Fx.castHoverOption(trace, ptNumber, 'font.family'),
fontSize: Fx.castHoverOption(trace, ptNumber, 'font.size'),
fontColor: Fx.castHoverOption(trace, ptNumber, 'font.color')
fontColor: Fx.castHoverOption(trace, ptNumber, 'font.color'),
nameLength: Fx.castHoverOption(trace, ptNumber, 'namelength')
}, {
container: this.svgContainer,
gd: this.graphDiv
Expand Down
42 changes: 22 additions & 20 deletions src/plots/gl3d/scene.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ var computeTickMarks = require('./layout/tick_marks');
var STATIC_CANVAS, STATIC_CONTEXT;

function render(scene) {
var gd = scene.graphDiv;
var trace;

// update size of svg container
Expand Down Expand Up @@ -70,6 +71,7 @@ function render(scene) {
if(lastPicked !== null) {
var pdata = project(scene.glplot.cameraParams, selection.dataCoordinate);
trace = lastPicked.data;
var traceNow = gd._fullData[trace.index];
Copy link
Contributor

Choose a reason for hiding this comment

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

@etpinard Could you please describe why traceNow is needed here?
Thanks in advance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because hoverlabel.* edits don't (and shouldn't have to) replot the scene. As Plots.supplyDefaults creates new gd._fullData and gd._fullLayout refs on every call, we need to use the right "now" trace object when hovering.

var ptNumber = selection.index;

var labels = {
Expand All @@ -78,11 +80,11 @@ function render(scene) {
zLabel: formatter('zaxis', selection.traceCoordinate[2])
};

var hoverinfo = Fx.castHoverinfo(trace, scene.fullLayout, ptNumber);
var hoverinfo = Fx.castHoverinfo(traceNow, scene.fullLayout, ptNumber);
var hoverinfoParts = (hoverinfo || '').split('+');
var isHoverinfoAll = hoverinfo && hoverinfo === 'all';

if(!trace.hovertemplate && !isHoverinfoAll) {
if(!traceNow.hovertemplate && !isHoverinfoAll) {
if(hoverinfoParts.indexOf('x') === -1) labels.xLabel = undefined;
if(hoverinfoParts.indexOf('y') === -1) labels.yLabel = undefined;
if(hoverinfoParts.indexOf('z') === -1) labels.zLabel = undefined;
Expand Down Expand Up @@ -138,55 +140,55 @@ function render(scene) {
x: selection.traceCoordinate[0],
y: selection.traceCoordinate[1],
z: selection.traceCoordinate[2],
data: trace._input,
fullData: trace,
curveNumber: trace.index,
data: traceNow._input,
fullData: traceNow,
curveNumber: traceNow.index,
pointNumber: ptNumber
};

Fx.appendArrayPointValue(pointData, trace, ptNumber);
Fx.appendArrayPointValue(pointData, traceNow, ptNumber);

if(trace._module.eventData) {
pointData = trace._module.eventData(pointData, selection, trace, {}, ptNumber);
pointData = traceNow._module.eventData(pointData, selection, traceNow, {}, ptNumber);
}

var eventData = {points: [pointData]};

if(scene.fullSceneLayout.hovermode) {
Fx.loneHover({
trace: trace,
trace: traceNow,
x: (0.5 + 0.5 * pdata[0] / pdata[3]) * width,
y: (0.5 - 0.5 * pdata[1] / pdata[3]) * height,
xLabel: labels.xLabel,
yLabel: labels.yLabel,
zLabel: labels.zLabel,
text: tx,
name: lastPicked.name,
color: Fx.castHoverOption(trace, ptNumber, 'bgcolor') || lastPicked.color,
borderColor: Fx.castHoverOption(trace, ptNumber, 'bordercolor'),
fontFamily: Fx.castHoverOption(trace, ptNumber, 'font.family'),
fontSize: Fx.castHoverOption(trace, ptNumber, 'font.size'),
fontColor: Fx.castHoverOption(trace, ptNumber, 'font.color'),
hovertemplate: Lib.castOption(trace, ptNumber, 'hovertemplate'),
color: Fx.castHoverOption(traceNow, ptNumber, 'bgcolor') || lastPicked.color,
borderColor: Fx.castHoverOption(traceNow, ptNumber, 'bordercolor'),
fontFamily: Fx.castHoverOption(traceNow, ptNumber, 'font.family'),
fontSize: Fx.castHoverOption(traceNow, ptNumber, 'font.size'),
fontColor: Fx.castHoverOption(traceNow, ptNumber, 'font.color'),
nameLength: Fx.castHoverOption(traceNow, ptNumber, 'namelength'),
hovertemplate: Lib.castOption(traceNow, ptNumber, 'hovertemplate'),
hovertemplateLabels: Lib.extendFlat({}, pointData, labels),
eventData: [pointData]
}, {
container: svgContainer,
gd: scene.graphDiv
gd: gd
});
}

if(selection.buttons && selection.distance < 5) {
scene.graphDiv.emit('plotly_click', eventData);
gd.emit('plotly_click', eventData);
} else {
scene.graphDiv.emit('plotly_hover', eventData);
gd.emit('plotly_hover', eventData);
}

oldEventData = eventData;
}
else {
} else {
Fx.loneUnhover(svgContainer);
scene.graphDiv.emit('plotly_unhover', oldEventData);
gd.emit('plotly_unhover', oldEventData);
}

scene.drawAnnotations(scene);
Expand Down
41 changes: 14 additions & 27 deletions src/plots/plots.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1218,8 +1218,7 @@ plots.supplyTraceDefaults = function(traceIn, traceOut, colorIndex, layout, trac
if(subplots[attri]) Lib.pushUnique(subplots[attri], vali);
subplotId += vali;
}
}
else {
} else {
subplotId = Lib.coerce(traceIn, traceOut, subplotAttrs, subplotAttr);
}

Expand All @@ -1230,18 +1229,6 @@ plots.supplyTraceDefaults = function(traceIn, traceOut, colorIndex, layout, trac
}
}

function coerceUnlessPruned(attr, dflt, cb) {
if(_module && (attr in _module.attributes) && _module.attributes[attr] === undefined) {
// Pruned
} else {
if(cb && typeof cb === 'function') {
cb();
} else {
coerce(attr, dflt);
}
}
}

if(visible) {
coerce('customdata');
coerce('ids');
Expand All @@ -1250,33 +1237,33 @@ plots.supplyTraceDefaults = function(traceIn, traceOut, colorIndex, layout, trac
traceOut._dfltShowLegend = true;
coerce('showlegend');
coerce('legendgroup');
}
else {
} else {
traceOut._dfltShowLegend = false;
}

coerceUnlessPruned('hoverlabel', '', function() {
Registry.getComponentMethod(
'fx',
'supplyDefaults'
)(traceIn, traceOut, defaultColor, layout);
});

// TODO add per-base-plot-module trace defaults step

if(_module) {
_module.supplyDefaults(traceIn, traceOut, defaultColor, layout);
if(!traceOut.hovertemplate) Lib.coerceHoverinfo(traceIn, traceOut, layout);
}

if(!Registry.traceIs(traceOut, 'noOpacity')) coerce('opacity');
if(!Registry.traceIs(traceOut, 'noOpacity')) {
coerce('opacity');
}

if(Registry.traceIs(traceOut, 'notLegendIsolatable')) {
// This clears out the legendonly state for traces like carpet that
// cannot be isolated in the legend
traceOut.visible = !!traceOut.visible;
}

if(!Registry.traceIs(traceOut, 'noHover')) {
if(!traceOut.hovertemplate) Lib.coerceHoverinfo(traceIn, traceOut, layout);

// parcats support hover, but not hoverlabel stylings (yet)
if(traceOut.type !== 'parcats') {
Registry.getComponentMethod('fx', 'supplyDefaults')(traceIn, traceOut, defaultColor, layout);
}
}

if(_module && _module.selectPoints) {
coerce('selectedpoints');
}
Expand Down
2 changes: 0 additions & 2 deletions src/traces/parcoords/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ var templatedArray = require('../../plot_api/plot_template').templatedArray;
module.exports = {
domain: domainAttrs({name: 'parcoords', trace: true, editType: 'calc'}),

hoverlabel: undefined,

labelfont: fontAttrs({
editType: 'calc',
description: 'Sets the font for the `dimension` labels.'
Expand Down
2 changes: 1 addition & 1 deletion src/traces/parcoords/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ Parcoords.colorbar = {
Parcoords.moduleType = 'trace';
Parcoords.name = 'parcoords';
Parcoords.basePlotModule = require('./base_plot');
Parcoords.categories = ['gl', 'regl', 'noOpacity'];
Parcoords.categories = ['gl', 'regl', 'noOpacity', 'noHover'];
Parcoords.meta = {
description: [
'Parallel coordinates for multidimensional exploratory data analysis.',
Expand Down
6 changes: 3 additions & 3 deletions src/traces/pie/plot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,10 +363,11 @@ function attachFxHandlers(sliceTop, gd, cd) {
pt.percentLabel = helpers.formatPiePercent(pt.percent, separators);
if(hoverinfo && hoverinfo.indexOf('percent') !== -1) thisText.push(pt.percentLabel);

var hoverLabel = trace.hoverlabel;
var hoverLabel = trace2.hoverlabel;
var hoverFont = hoverLabel.font;

Fx.loneHover({
trace: trace,
x0: hoverCenterX - rInscribed * cd0.r,
x1: hoverCenterX + rInscribed * cd0.r,
y: hoverCenterY,
Expand All @@ -378,8 +379,7 @@ function attachFxHandlers(sliceTop, gd, cd) {
fontFamily: helpers.castOption(hoverFont.family, pt.pts),
fontSize: helpers.castOption(hoverFont.size, pt.pts),
fontColor: helpers.castOption(hoverFont.color, pt.pts),

trace: trace2,
nameLength: helpers.castOption(hoverLabel.namelength, pt.pts),
hovertemplate: helpers.castOption(trace2.hovertemplate, pt.pts),
hovertemplateLabels: pt,
eventData: [eventData(pt, trace2)]
Expand Down
2 changes: 2 additions & 0 deletions src/traces/sankey/plot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ module.exports = function plot(gd, calcData) {
fontFamily: castHoverOption(obj, 'font.family'),
fontSize: castHoverOption(obj, 'font.size'),
fontColor: castHoverOption(obj, 'font.color'),
nameLength: castHoverOption(obj, 'namelength'),
idealAlign: d3.event.x < hoverCenterX ? 'right' : 'left',

hovertemplate: obj.hovertemplate,
Expand Down Expand Up @@ -267,6 +268,7 @@ module.exports = function plot(gd, calcData) {
fontFamily: castHoverOption(obj, 'font.family'),
fontSize: castHoverOption(obj, 'font.size'),
fontColor: castHoverOption(obj, 'font.color'),
nameLength: castHoverOption(obj, 'namelength'),
idealAlign: 'left',

hovertemplate: obj.hovertemplate,
Expand Down
Loading