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 1 commit
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
1 change: 1 addition & 0 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
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
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
3 changes: 2 additions & 1 deletion src/traces/sunburst/plot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,18 +581,19 @@ function attachFxHandlers(sliceTop, gd, cd) {
if(hasFlag('text') && hoverPt.text) thisText.push(hoverPt.text);

Fx.loneHover({
trace: traceNow,
x0: hoverCenterX - rInscribed * pt.rpx1,
x1: hoverCenterX + rInscribed * pt.rpx1,
y: hoverCenterY,
idealAlign: pt.pxmid[0] < 0 ? 'left' : 'right',
trace: traceNow,
text: thisText.join('<br>'),
name: (hovertemplate || hasFlag('name')) ? traceNow.name : undefined,
color: _cast('hoverlabel.bgcolor') || cdi.color,
borderColor: _cast('hoverlabel.bordercolor'),
fontFamily: _cast('hoverlabel.font.family'),
fontSize: _cast('hoverlabel.font.size'),
fontColor: _cast('hoverlabel.font.color'),
nameLength: _cast('hoverlabel.namelength'),
hovertemplate: hovertemplate,
hovertemplateLabels: hoverPt,
eventData: [makeEventData(pt, traceNow)]
Expand Down
12 changes: 12 additions & 0 deletions test/jasmine/tests/gl3d_plot_interact_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,18 @@ describe('Test gl3d plots', function() {
}
);
})
.then(function() { return Plotly.restyle(gd, 'hoverlabel.namelength', 3); })
.then(delay(20))
.then(_hover)
.then(delay(20))
.then(function() {
assertHoverLabelContent(
{
nums: ['x: 0', 'y: 0', 'z: 0'].join('\n'),
name: 'tra'
}
);
})
.catch(failTest)
.then(done);
});
Expand Down
18 changes: 18 additions & 0 deletions test/jasmine/tests/pie_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1169,6 +1169,24 @@ describe('pie hovering', function() {
.catch(failTest)
.then(done);
});

it('should honor *hoverlabel.namelength*', function(done) {
mockCopy.data[0].name = 'loooooooooooooooooooooooong';
mockCopy.data[0].hoverinfo = 'all';

Plotly.plot(gd, mockCopy.data, mockCopy.layout)
.then(_hover)
.then(function() {
assertHoverLabelContent({nums: '4\n5\n33.3%', name: 'looooooooooo...'}, 'base');
})
.then(function() { return Plotly.restyle(gd, 'hoverlabel.namelength', 2); })
.then(_hover)
.then(function() {
assertHoverLabelContent({nums: '4\n5\n33.3%', name: 'lo'}, 'base');
})
.catch(failTest)
.then(done);
});
});

describe('should fit labels within graph div', function() {
Expand Down
27 changes: 27 additions & 0 deletions test/jasmine/tests/sankey_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ var destroyGraphDiv = require('../assets/destroy_graph_div');
var failTest = require('../assets/fail_test');
var mouseEvent = require('../assets/mouse_event');
var getNodeCoords = require('../assets/get_node_coords');
var assertHoverLabelContent = require('../assets/custom_assertions').assertHoverLabelContent;
var assertHoverLabelStyle = require('../assets/custom_assertions').assertHoverLabelStyle;
var supplyAllDefaults = require('../assets/supply_defaults');
var defaultColors = require('@src/components/color/attributes').defaults;
Expand Down Expand Up @@ -866,6 +867,32 @@ describe('sankey tests', function() {
.catch(failTest)
.then(done);
});

it('should honor *hoverlabel.namelength*', function(done) {
var gd = createGraphDiv();
var mockCopy = Lib.extendDeep({}, mock);

Plotly.plot(gd, mockCopy)
.then(function() { _hover(404, 302); })
.then(function() {
assertHoverLabelContent({
nums: 'Solid\nincoming flow count: 4\noutgoing flow count: 3',
name: '447TWh'
});
})
.then(function() {
return Plotly.restyle(gd, 'hoverlabel.namelength', 3);
})
.then(function() { _hover(404, 302); })
.then(function() {
assertHoverLabelContent({
nums: 'Solid\nincoming flow count: 4\noutgoing flow count: 3',
name: '447'
});
})
.catch(failTest)
.then(done);
});
});

describe('Test hover/click event data:', function() {
Expand Down
19 changes: 19 additions & 0 deletions test/jasmine/tests/sunburst_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,25 @@ describe('Test sunburst hover:', function() {
parent: 'Eve'
}
}
}, {
desc: 'with hoverlabel.namelength set ',
traces: [{
hoverlabel: {namelength: 4},
hoverinfo: 'all'
}],
pos: 4,
exp: {
label: {
nums: 'Abel',
name: 't...'
},
ptData: {
curveNumber: 0,
pointNumber: 5,
label: 'Abel',
parent: 'Eve'
}
}
}, {
desc: 'with values',
traces: [{
Expand Down