Skip to content

Commit 2b4de3e

Browse files
etpinardetpinard
authored
Merge pull request #3968 from plotly/choropleth-z-hovertemplate-formatting-fix
Ensure hoverinfo <--> hovertemplate number formatting equivalence
2 parents 56f3bb7 + ea3a90f commit 2b4de3e

File tree

11 files changed

+102
-44
lines changed

11 files changed

+102
-44
lines changed

Diff for: ‎src/components/fx/hover.js

+6-2
Original file line numberDiff line numberDiff line change
@@ -896,11 +896,15 @@ function createHoverText(hoverData, opts, gd) {
896896
if(d.zLabel !== undefined) {
897897
if(d.xLabel !== undefined) text += 'x: ' + d.xLabel + '<br>';
898898
if(d.yLabel !== undefined) text += 'y: ' + d.yLabel + '<br>';
899-
text += (text ? 'z: ' : '') + d.zLabel;
899+
if(d.trace.type !== 'choropleth') {
900+
text += (text ? 'z: ' : '') + d.zLabel;
901+
}
900902
} else if(showCommonLabel && d[hovermode + 'Label'] === t0) {
901903
text = d[(hovermode === 'x' ? 'y' : 'x') + 'Label'] || '';
902904
} else if(d.xLabel === undefined) {
903-
if(d.yLabel !== undefined && d.trace.type !== 'scattercarpet') text = d.yLabel;
905+
if(d.yLabel !== undefined && d.trace.type !== 'scattercarpet') {
906+
text = d.yLabel;
907+
}
904908
} else if(d.yLabel === undefined) text = d.xLabel;
905909
else text = '(' + d.xLabel + ', ' + d.yLabel + ')';
906910

Diff for: ‎src/traces/choropleth/hover.js

+6-10
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
* LICENSE file in the root directory of this source tree.
77
*/
88

9-
109
'use strict';
1110

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

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

5454
return [pointData];
5555
};
5656

57-
function makeHoverInfo(pointData, trace, pt, axis) {
58-
if(trace.hovertemplate) {
59-
return;
60-
}
57+
function makeHoverInfo(pointData, trace, pt) {
58+
if(trace.hovertemplate) return;
6159

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

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

7472
var text = [];
7573

76-
function formatter(val) {
77-
return Axes.tickText(axis, axis.c2l(val), 'hover').text;
78-
}
79-
8074
if(hasIdAsNameLabel) {
8175
pointData.nameOverride = pt.loc;
8276
} else {
8377
if(hasName) pointData.nameOverride = trace.name;
8478
if(hasLocation) text.push(pt.loc);
8579
}
8680

87-
if(hasZ) text.push(formatter(pt.z));
81+
if(hasZ) {
82+
text.push(pointData.zLabel);
83+
}
8884
if(hasText) {
8985
fillText(pt, trace, text);
9086
}

Diff for: ‎src/traces/scattergeo/hover.js

+11-12
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
* LICENSE file in the root directory of this source tree.
77
*/
88

9-
109
'use strict';
1110

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

66+
var ax = geo.mockAxis;
67+
pointData.lonLabel = Axes.tickText(ax, ax.c2l(pointData.lon), 'hover').text;
68+
pointData.latLabel = Axes.tickText(ax, ax.c2l(pointData.lat), 'hover').text;
69+
6770
pointData.color = getTraceColor(trace, di);
68-
pointData.extraText = getExtraText(trace, di, geo.mockAxis, cd[0].t.labels);
71+
pointData.extraText = getExtraText(trace, di, pointData, cd[0].t.labels);
6972
pointData.hovertemplate = trace.hovertemplate;
7073

7174
return [pointData];
7275
};
7376

74-
function getExtraText(trace, pt, axis, labels) {
75-
if(trace.hovertemplate) {
76-
return;
77-
}
77+
function getExtraText(trace, pt, pointData, labels) {
78+
if(trace.hovertemplate) return;
7879

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

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

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

9594
if(hasLocation) {
9695
text.push(pt.loc);
9796
} else if(hasLon && hasLat) {
98-
text.push('(' + format(pt.lonlat[0]) + ', ' + format(pt.lonlat[1]) + ')');
97+
text.push('(' + format(pointData.lonLabel) + ', ' + format(pointData.latLabel) + ')');
9998
} else if(hasLon) {
100-
text.push(labels.lon + format(pt.lonlat[0]));
99+
text.push(labels.lon + format(pointData.lonLabel));
101100
} else if(hasLat) {
102-
text.push(labels.lat + format(pt.lonlat[1]));
101+
text.push(labels.lat + format(pointData.latLabel));
103102
}
104103

105104
if(hasText) {

Diff for: ‎src/traces/scatterpolar/hover.js

+11-11
Original file line numberDiff line numberDiff line change
@@ -42,26 +42,26 @@ function makeHoverPointText(cdi, trace, subplot, pointData) {
4242
radialAxis._hovertitle = 'r';
4343
angularAxis._hovertitle = 'θ';
4444

45+
var rVal = radialAxis.c2l(cdi.r);
46+
pointData.rLabel = Axes.tickText(radialAxis, rVal, 'hover').text;
47+
48+
// N.B here the ° sign is part of the formatted value for thetaunit:'degrees'
49+
var thetaVal = angularAxis.thetaunit === 'degrees' ? Lib.rad2deg(cdi.theta) : cdi.theta;
50+
pointData.thetaLabel = Axes.tickText(angularAxis, thetaVal, 'hover').text;
51+
4552
var hoverinfo = cdi.hi || trace.hoverinfo;
4653
var text = [];
4754
function textPart(ax, val) {
48-
text.push(ax._hovertitle + ': ' + Axes.tickText(ax, val, 'hover').text);
55+
text.push(ax._hovertitle + ': ' + val);
4956
}
5057

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

5461
if(parts.indexOf('all') !== -1) parts = ['r', 'theta', 'text'];
55-
if(parts.indexOf('r') !== -1) {
56-
textPart(radialAxis, radialAxis.c2l(cdi.r));
57-
}
58-
if(parts.indexOf('theta') !== -1) {
59-
var theta = cdi.theta;
60-
textPart(
61-
angularAxis,
62-
angularAxis.thetaunit === 'degrees' ? Lib.rad2deg(theta) : theta
63-
);
64-
}
62+
if(parts.indexOf('r') !== -1) textPart(radialAxis, pointData.rLabel);
63+
if(parts.indexOf('theta') !== -1) textPart(angularAxis, pointData.thetaLabel);
64+
6565
if(parts.indexOf('text') !== -1 && pointData.text) {
6666
text.push(pointData.text);
6767
delete pointData.text;

Diff for: ‎src/traces/scatterternary/hover.js

+9-6
Original file line numberDiff line numberDiff line change
@@ -47,21 +47,24 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
4747

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

52-
var trace = newPointData.trace;
5351
var ternary = newPointData.subplot;
52+
newPointData.aLabel = Axes.tickText(ternary.aaxis, cdi.a, 'hover').text;
53+
newPointData.bLabel = Axes.tickText(ternary.baxis, cdi.b, 'hover').text;
54+
newPointData.cLabel = Axes.tickText(ternary.caxis, cdi.c, 'hover').text;
55+
56+
var trace = newPointData.trace;
5457
var hoverinfo = cdi.hi || trace.hoverinfo;
5558
var text = [];
5659
function textPart(ax, val) {
57-
text.push(ax._hovertitle + ': ' + Axes.tickText(ax, val, 'hover').text);
60+
text.push(ax._hovertitle + ': ' + val);
5861
}
5962
if(!trace.hovertemplate) {
6063
var parts = hoverinfo.split('+');
6164
if(parts.indexOf('all') !== -1) parts = ['a', 'b', 'c'];
62-
if(parts.indexOf('a') !== -1) textPart(ternary.aaxis, cdi.a);
63-
if(parts.indexOf('b') !== -1) textPart(ternary.baxis, cdi.b);
64-
if(parts.indexOf('c') !== -1) textPart(ternary.caxis, cdi.c);
65+
if(parts.indexOf('a') !== -1) textPart(ternary.aaxis, newPointData.aLabel);
66+
if(parts.indexOf('b') !== -1) textPart(ternary.baxis, newPointData.bLabel);
67+
if(parts.indexOf('c') !== -1) textPart(ternary.caxis, newPointData.cLabel);
6568
}
6669
newPointData.extraText = text.join('<br>');
6770
newPointData.hovertemplate = trace.hovertemplate;

Diff for: ‎test/jasmine/tests/barpolar_test.js

+2
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,8 @@ describe('Test barpolar hover:', function() {
128128
index: 0,
129129
x: 263.33,
130130
y: 200,
131+
rLabel: '1',
132+
thetaLabel: '0°',
131133
hovertemplate: 'tpl',
132134
color: '#1f77b4'
133135
}

Diff for: ‎test/jasmine/tests/choropleth_test.js

+25
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,31 @@ describe('Test choropleth hover:', function() {
200200
)
201201
.then(done);
202202
});
203+
204+
describe('should preserve z formatting hovetemplate equivalence', function() {
205+
var base = function() {
206+
return {
207+
data: [{
208+
type: 'choropleth',
209+
locations: ['RUS'],
210+
z: [10.02132132143214321]
211+
}]
212+
};
213+
};
214+
215+
var pos = [400, 160];
216+
var exp = ['10.02132', 'RUS'];
217+
218+
it('- base case (truncate z decimals)', function(done) {
219+
run(pos, base(), exp).then(done);
220+
});
221+
222+
it('- hovertemplate case (same z truncation)', function(done) {
223+
var fig = base();
224+
fig.hovertemplate = '%{z}<extra>%{location}</extra>';
225+
run(pos, fig, exp).then(done);
226+
});
227+
});
203228
});
204229

205230
describe('choropleth drawing', function() {

Diff for: ‎test/jasmine/tests/scattergeo_test.js

+26
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,32 @@ describe('Test scattergeo hover', function() {
358358
.catch(failTest)
359359
.then(done);
360360
});
361+
362+
describe('should preserve lon/lat formatting hovetemplate equivalence', function() {
363+
var pos = [381, 221];
364+
var exp = ['(10.00012°, 10.00088°)\nA'];
365+
366+
it('- base case (truncate z decimals)', function(done) {
367+
Plotly.restyle(gd, {
368+
lon: [[10.0001221321]],
369+
lat: [[10.00087683]]
370+
})
371+
.then(function() { check(pos, exp); })
372+
.catch(failTest)
373+
.then(done);
374+
});
375+
376+
it('- hovertemplate case (same lon/lat truncation)', function(done) {
377+
Plotly.restyle(gd, {
378+
lon: [[10.0001221321]],
379+
lat: [[10.00087683]],
380+
hovertemplate: '(%{lon}°, %{lat}°)<br>%{text}<extra></extra>'
381+
})
382+
.then(function() { check(pos, exp); })
383+
.catch(failTest)
384+
.then(done);
385+
});
386+
});
361387
});
362388

363389
describe('scattergeo drawing', function() {

Diff for: ‎test/jasmine/tests/scatterpolar_test.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ describe('Test scatterpolar hover:', function() {
115115
fig.data[2].hovertemplate = 'template %{r} %{theta}';
116116
return fig;
117117
},
118-
nums: 'template 4.02289202968 128.342009045',
118+
nums: 'template 4.022892 128.342°',
119119
name: 'Trial 3'
120120
}, {
121121
desc: 'with hovertemplate and empty trace name',
@@ -124,7 +124,7 @@ describe('Test scatterpolar hover:', function() {
124124
fig.data[2].name = '';
125125
return fig;
126126
},
127-
nums: 'template 4.02289202968 128.342009045',
127+
nums: 'template 4.022892 128.342°',
128128
name: ''
129129
}, {
130130
desc: '(no labels - out of sector)',

Diff for: ‎test/jasmine/tests/scatterpolargl_test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ describe('Test scatterpolargl hover:', function() {
5050
fig.data[2].hovertemplate = 'template %{r} %{theta}';
5151
return fig;
5252
},
53-
nums: 'template 3.88601339194 125.282157112',
53+
nums: 'template 3.886013 125.2822°',
5454
name: 'Trial 3'
5555
}, {
5656
desc: '(no labels - out of sector)',

Diff for: ‎test/jasmine/tests/scatterternary_test.js

+3
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,9 @@ describe('scatterternary hover', function() {
422422
.then(function() {
423423
scatterPointData = _hover(gd, xval, yval, hovermode);
424424
expect(scatterPointData[0].hovertemplate).toEqual('tpl');
425+
expect(scatterPointData[0].aLabel).toBe('0.3333333');
426+
expect(scatterPointData[0].bLabel).toBe('0.1111111');
427+
expect(scatterPointData[0].cLabel).toBe('0.5555556');
425428
})
426429
.catch(failTest)
427430
.then(done);

0 commit comments

Comments
 (0)