Skip to content

Ensure hoverinfo <--> hovertemplate number formatting equivalence #3968

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 4 commits into from
Jun 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 6 additions & 2 deletions src/components/fx/hover.js
Original file line number Diff line number Diff line change
Expand Up @@ -896,11 +896,15 @@ function createHoverText(hoverData, opts, gd) {
if(d.zLabel !== undefined) {
if(d.xLabel !== undefined) text += 'x: ' + d.xLabel + '<br>';
if(d.yLabel !== undefined) text += 'y: ' + d.yLabel + '<br>';
text += (text ? 'z: ' : '') + d.zLabel;
if(d.trace.type !== 'choropleth') {
text += (text ? 'z: ' : '') + d.zLabel;
}
} else if(showCommonLabel && d[hovermode + 'Label'] === t0) {
text = d[(hovermode === 'x' ? 'y' : 'x') + 'Label'] || '';
} else if(d.xLabel === undefined) {
if(d.yLabel !== undefined && d.trace.type !== 'scattercarpet') text = d.yLabel;
if(d.yLabel !== undefined && d.trace.type !== 'scattercarpet') {
text = d.yLabel;
}
} else if(d.yLabel === undefined) text = d.xLabel;
else text = '(' + d.xLabel + ', ' + d.yLabel + ')';

Expand Down
16 changes: 6 additions & 10 deletions src/traces/choropleth/hover.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
* LICENSE file in the root directory of this source tree.
*/


'use strict';

var Axes = require('../../plots/cartesian/axes');
Expand Down Expand Up @@ -47,17 +46,16 @@ module.exports = function hoverPoints(pointData, xval, yval) {
pointData.index = pt.index;
pointData.location = pt.loc;
pointData.z = pt.z;
pointData.zLabel = Axes.tickText(geo.mockAxis, geo.mockAxis.c2l(pt.z), 'hover').text;
pointData.hovertemplate = pt.hovertemplate;

makeHoverInfo(pointData, trace, pt, geo.mockAxis);

return [pointData];
};

function makeHoverInfo(pointData, trace, pt, axis) {
if(trace.hovertemplate) {
return;
}
function makeHoverInfo(pointData, trace, pt) {
if(trace.hovertemplate) return;

var hoverinfo = pt.hi || trace.hoverinfo;

Expand All @@ -73,18 +71,16 @@ function makeHoverInfo(pointData, trace, pt, axis) {

var text = [];

function formatter(val) {
return Axes.tickText(axis, axis.c2l(val), 'hover').text;
}

if(hasIdAsNameLabel) {
pointData.nameOverride = pt.loc;
} else {
if(hasName) pointData.nameOverride = trace.name;
if(hasLocation) text.push(pt.loc);
}

if(hasZ) text.push(formatter(pt.z));
if(hasZ) {
text.push(pointData.zLabel);
}
if(hasText) {
fillText(pt, trace, text);
}
Expand Down
23 changes: 11 additions & 12 deletions src/traces/scattergeo/hover.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
* LICENSE file in the root directory of this source tree.
*/


'use strict';

var Fx = require('../../components/fx');
Expand Down Expand Up @@ -64,17 +63,19 @@ module.exports = function hoverPoints(pointData, xval, yval) {
pointData.lon = lonlat[0];
pointData.lat = lonlat[1];

var ax = geo.mockAxis;
pointData.lonLabel = Axes.tickText(ax, ax.c2l(pointData.lon), 'hover').text;
pointData.latLabel = Axes.tickText(ax, ax.c2l(pointData.lat), 'hover').text;

pointData.color = getTraceColor(trace, di);
pointData.extraText = getExtraText(trace, di, geo.mockAxis, cd[0].t.labels);
pointData.extraText = getExtraText(trace, di, pointData, cd[0].t.labels);
pointData.hovertemplate = trace.hovertemplate;

return [pointData];
};

function getExtraText(trace, pt, axis, labels) {
if(trace.hovertemplate) {
return;
}
function getExtraText(trace, pt, pointData, labels) {
if(trace.hovertemplate) return;

var hoverinfo = pt.hi || trace.hoverinfo;

Expand All @@ -88,18 +89,16 @@ function getExtraText(trace, pt, axis, labels) {
var hasText = (parts.indexOf('text') !== -1);
var text = [];

function format(val) {
return Axes.tickText(axis, axis.c2l(val), 'hover').text + '\u00B0';
}
function format(val) { return val + '\u00B0'; }

if(hasLocation) {
text.push(pt.loc);
} else if(hasLon && hasLat) {
text.push('(' + format(pt.lonlat[0]) + ', ' + format(pt.lonlat[1]) + ')');
text.push('(' + format(pointData.lonLabel) + ', ' + format(pointData.latLabel) + ')');
} else if(hasLon) {
text.push(labels.lon + format(pt.lonlat[0]));
text.push(labels.lon + format(pointData.lonLabel));
} else if(hasLat) {
text.push(labels.lat + format(pt.lonlat[1]));
text.push(labels.lat + format(pointData.latLabel));
}

if(hasText) {
Expand Down
22 changes: 11 additions & 11 deletions src/traces/scatterpolar/hover.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,26 +42,26 @@ function makeHoverPointText(cdi, trace, subplot, pointData) {
radialAxis._hovertitle = 'r';
angularAxis._hovertitle = 'θ';

var rVal = radialAxis.c2l(cdi.r);
pointData.rLabel = Axes.tickText(radialAxis, rVal, 'hover').text;

// N.B here the ° sign is part of the formatted value for thetaunit:'degrees'
var thetaVal = angularAxis.thetaunit === 'degrees' ? Lib.rad2deg(cdi.theta) : cdi.theta;
pointData.thetaLabel = Axes.tickText(angularAxis, thetaVal, 'hover').text;
Copy link
Contributor Author

@etpinard etpinard Jun 17, 2019

Choose a reason for hiding this comment

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

This here is an interesting case. In a similar scenario, note also that:

Plotly.newPlot(gd, [{
  y: [1],
  hovertemplate: '%{y}'
}], {
  yaxis: {ticksuffix: ' --', tickprefix: '-- '}
})

shows -- 1 -- in the hover labels.


Please let me know if you think ticksuffix and tickprefix should be left out.


var hoverinfo = cdi.hi || trace.hoverinfo;
var text = [];
function textPart(ax, val) {
text.push(ax._hovertitle + ': ' + Axes.tickText(ax, val, 'hover').text);
text.push(ax._hovertitle + ': ' + val);
}

if(!trace.hovertemplate) {
var parts = hoverinfo.split('+');

if(parts.indexOf('all') !== -1) parts = ['r', 'theta', 'text'];
if(parts.indexOf('r') !== -1) {
textPart(radialAxis, radialAxis.c2l(cdi.r));
}
if(parts.indexOf('theta') !== -1) {
var theta = cdi.theta;
textPart(
angularAxis,
angularAxis.thetaunit === 'degrees' ? Lib.rad2deg(theta) : theta
);
}
if(parts.indexOf('r') !== -1) textPart(radialAxis, pointData.rLabel);
if(parts.indexOf('theta') !== -1) textPart(angularAxis, pointData.thetaLabel);

if(parts.indexOf('text') !== -1 && pointData.text) {
text.push(pointData.text);
delete pointData.text;
Expand Down
15 changes: 9 additions & 6 deletions src/traces/scatterternary/hover.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,21 +47,24 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {

newPointData.xLabelVal = undefined;
newPointData.yLabelVal = undefined;
// TODO: nice formatting, and label by axis title, for a, b, and c?

var trace = newPointData.trace;
var ternary = newPointData.subplot;
newPointData.aLabel = Axes.tickText(ternary.aaxis, cdi.a, 'hover').text;
newPointData.bLabel = Axes.tickText(ternary.baxis, cdi.b, 'hover').text;
newPointData.cLabel = Axes.tickText(ternary.caxis, cdi.c, 'hover').text;

var trace = newPointData.trace;
var hoverinfo = cdi.hi || trace.hoverinfo;
var text = [];
function textPart(ax, val) {
text.push(ax._hovertitle + ': ' + Axes.tickText(ax, val, 'hover').text);
text.push(ax._hovertitle + ': ' + val);
}
if(!trace.hovertemplate) {
var parts = hoverinfo.split('+');
if(parts.indexOf('all') !== -1) parts = ['a', 'b', 'c'];
if(parts.indexOf('a') !== -1) textPart(ternary.aaxis, cdi.a);
if(parts.indexOf('b') !== -1) textPart(ternary.baxis, cdi.b);
if(parts.indexOf('c') !== -1) textPart(ternary.caxis, cdi.c);
if(parts.indexOf('a') !== -1) textPart(ternary.aaxis, newPointData.aLabel);
if(parts.indexOf('b') !== -1) textPart(ternary.baxis, newPointData.bLabel);
if(parts.indexOf('c') !== -1) textPart(ternary.caxis, newPointData.cLabel);
}
newPointData.extraText = text.join('<br>');
newPointData.hovertemplate = trace.hovertemplate;
Expand Down
2 changes: 2 additions & 0 deletions test/jasmine/tests/barpolar_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ describe('Test barpolar hover:', function() {
index: 0,
x: 263.33,
y: 200,
rLabel: '1',
thetaLabel: '0°',
hovertemplate: 'tpl',
color: '#1f77b4'
}
Expand Down
25 changes: 25 additions & 0 deletions test/jasmine/tests/choropleth_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,31 @@ describe('Test choropleth hover:', function() {
)
.then(done);
});

describe('should preserve z formatting hovetemplate equivalence', function() {
var base = function() {
return {
data: [{
type: 'choropleth',
locations: ['RUS'],
z: [10.02132132143214321]
}]
};
};

var pos = [400, 160];
var exp = ['10.02132', 'RUS'];

it('- base case (truncate z decimals)', function(done) {
run(pos, base(), exp).then(done);
});

it('- hovertemplate case (same z truncation)', function(done) {
var fig = base();
fig.hovertemplate = '%{z}<extra>%{location}</extra>';
run(pos, fig, exp).then(done);
});
});
});

describe('choropleth drawing', function() {
Expand Down
26 changes: 26 additions & 0 deletions test/jasmine/tests/scattergeo_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,32 @@ describe('Test scattergeo hover', function() {
.catch(failTest)
.then(done);
});

describe('should preserve lon/lat formatting hovetemplate equivalence', function() {
var pos = [381, 221];
var exp = ['(10.00012°, 10.00088°)\nA'];

it('- base case (truncate z decimals)', function(done) {
Plotly.restyle(gd, {
lon: [[10.0001221321]],
lat: [[10.00087683]]
})
.then(function() { check(pos, exp); })
.catch(failTest)
.then(done);
});

it('- hovertemplate case (same lon/lat truncation)', function(done) {
Plotly.restyle(gd, {
lon: [[10.0001221321]],
lat: [[10.00087683]],
hovertemplate: '(%{lon}°, %{lat}°)<br>%{text}<extra></extra>'
})
.then(function() { check(pos, exp); })
.catch(failTest)
.then(done);
});
});
});

describe('scattergeo drawing', function() {
Expand Down
4 changes: 2 additions & 2 deletions test/jasmine/tests/scatterpolar_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ describe('Test scatterpolar hover:', function() {
fig.data[2].hovertemplate = 'template %{r} %{theta}';
return fig;
},
nums: 'template 4.02289202968 128.342009045',
nums: 'template 4.022892 128.342°',
name: 'Trial 3'
}, {
desc: 'with hovertemplate and empty trace name',
Expand All @@ -124,7 +124,7 @@ describe('Test scatterpolar hover:', function() {
fig.data[2].name = '';
return fig;
},
nums: 'template 4.02289202968 128.342009045',
nums: 'template 4.022892 128.342°',
name: ''
}, {
desc: '(no labels - out of sector)',
Expand Down
2 changes: 1 addition & 1 deletion test/jasmine/tests/scatterpolargl_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('Test scatterpolargl hover:', function() {
fig.data[2].hovertemplate = 'template %{r} %{theta}';
return fig;
},
nums: 'template 3.88601339194 125.282157112',
nums: 'template 3.886013 125.2822°',
name: 'Trial 3'
}, {
desc: '(no labels - out of sector)',
Expand Down
3 changes: 3 additions & 0 deletions test/jasmine/tests/scatterternary_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,9 @@ describe('scatterternary hover', function() {
.then(function() {
scatterPointData = _hover(gd, xval, yval, hovermode);
expect(scatterPointData[0].hovertemplate).toEqual('tpl');
expect(scatterPointData[0].aLabel).toBe('0.3333333');
expect(scatterPointData[0].bLabel).toBe('0.1111111');
expect(scatterPointData[0].cLabel).toBe('0.5555556');
})
.catch(failTest)
.then(done);
Expand Down