Skip to content

Add 'hovertext' attribute in scatter* traces #1523

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 13 commits into from
Apr 7, 2017
Merged

Add 'hovertext' attribute in scatter* traces #1523

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
b91fd3b
[proof of concept] add 'hovertext' attribute
etpinard Mar 28, 2017
2dc0250
add 'hovertext' to text_chart_arrays mock
etpinard Mar 29, 2017
253a29b
:lock: down 'hovertext' / 'text' logic for scatter traces
etpinard Mar 29, 2017
2b02c5f
add 'hovertext' to scatterternary traces
etpinard Mar 29, 2017
7b6911f
add 'hovertext' to bar traces
etpinard Mar 29, 2017
a7b76ee
add 'hovertext' to scatter3d traces
etpinard Mar 29, 2017
55a51f8
DRY up scattermapbox 'mode' attribute declaration
etpinard Mar 29, 2017
aa6eadf
add 'hoverinfo' to scattermapbox traces
etpinard Mar 29, 2017
b3448f0
Merge branch 'master' into hovertext-on-hover
etpinard Mar 31, 2017
4148cd4
add 'hovertext' attribute to pie traces
etpinard Mar 31, 2017
b760fe8
Merge branch 'master' into hovertext-on-hover
etpinard Apr 7, 2017
a859464
add 'hoverinfo' to scattergeo traces
etpinard Apr 7, 2017
1bf54e2
use d3's .size() instead of diving into selection items
etpinard Apr 7, 2017
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
22 changes: 21 additions & 1 deletion src/traces/pie/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,27 @@ module.exports = {

text: {
valType: 'data_array',
Copy link
Contributor Author

@etpinard etpinard Apr 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@alexcjohnson I just noticed that text for pie is declared as a data_array whereas other trace types have it as a string with arrayOk: true. Was that on purpose? Or should we allow single-value text input for pie traces?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That was on purpose - though probably never explicitly discussed. My rationale is that text is essentially the x data for pies, and they would be meaningless without this being an array of data. Can you think of a counterexample?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree for the on-graph textinfo: 'text' part. But if someone wants to show the same text piece in all hover labels? But I guess after this PR, hovertext will cover that case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh sorry, that's label... I still don't see a good usage for a single-value text but perhaps it would be better to define it consistently with other trace types anyway.

description: 'Sets text elements associated with each sector.'
description: [
'Sets text elements associated with each sector.',
'If trace `textinfo` contains a *text* flag, these elements will seen',
'on the chart.',
'If trace `hoverinfo` contains a *text* flag and *hovertext* is not set,',
'these elements will be seen in the hover labels.'
].join(' ')
},
hovertext: {
valType: 'string',
role: 'info',
dflt: '',
arrayOk: true,
description: [
'Sets hover text elements associated with each sector.',
'If a single string, the same string appears for',
'all data points.',
'If an array of string, the items are mapped in order of',
'this trace\'s sectors.',
'To be seen, trace `hoverinfo` must contain a *text* flag.'
].join(' ')
},

// 'see eg:'
Expand Down
1 change: 1 addition & 0 deletions src/traces/pie/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout

var textData = coerce('text');
var textInfo = coerce('textinfo', Array.isArray(textData) ? 'text+percent' : 'percent');
coerce('hovertext');

coerce('hoverinfo', (layout._dataLength === 1) ? 'label+text+value+percent' : undefined);

Expand Down
12 changes: 10 additions & 2 deletions src/traces/pie/plot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,16 @@ module.exports = function plot(gd, cdpie) {
thisText = [];

if(hoverinfo.indexOf('label') !== -1) thisText.push(pt.label);
if(trace2.text && trace2.text[pt.i] && hoverinfo.indexOf('text') !== -1) {
thisText.push(trace2.text[pt.i]);
if(hoverinfo.indexOf('text') !== -1) {
if(trace2.hovertext) {
thisText.push(
Array.isArray(trace2.hovertext) ?
trace2.hovertext[pt.i] :
trace2.hovertext
);
} else if(trace2.text && trace2.text[pt.i]) {
thisText.push(trace2.text[pt.i]);
}
}
if(hoverinfo.indexOf('value') !== -1) thisText.push(helpers.formatPieValue(pt.v, separators));
if(hoverinfo.indexOf('percent') !== -1) thisText.push(helpers.formatPiePercent(pt.v / cd0.vTotal, separators));
Expand Down
75 changes: 45 additions & 30 deletions test/jasmine/tests/hover_pie_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ var customMatchers = require('../assets/custom_matchers');
var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');

var d3 = require('d3');
var click = require('../assets/click');
var getClientPosition = require('../assets/get_client_position');
var mouseEvent = require('../assets/mouse_event');
Expand Down Expand Up @@ -175,9 +176,7 @@ describe('pie hovering', function() {
});

describe('labels', function() {

var gd,
mockCopy;
var gd, mockCopy;

beforeEach(function() {
gd = createGraphDiv();
Expand All @@ -186,44 +185,60 @@ describe('pie hovering', function() {

afterEach(destroyGraphDiv);

it('should show the default selected values', function(done) {

var expected = ['4', '5', '33.3%'];

Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(function() {
function _hover() {
mouseEvent('mouseover', 223, 143);
}

mouseEvent('mouseover', 223, 143);
function assertLabel(expected) {
var labels = d3.selectAll('.hovertext .nums .line');

var labels = Plotly.d3.selectAll('.hovertext .nums .line');
expect(labels[0].length).toBe(expected.length);
Copy link
Collaborator

Choose a reason for hiding this comment

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

🐄 labels.size()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good 👁️ thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in 1bf54e2


expect(labels[0].length).toBe(3);
labels.each(function(_, i) {
expect(d3.select(this).text()).toBe(expected[i]);
});
}

labels.each(function(_, i) {
expect(Plotly.d3.select(this).text()).toBe(expected[i]);
});
}).then(done);
it('should show the default selected values', function(done) {
Plotly.plot(gd, mockCopy.data, mockCopy.layout)
.then(_hover)
.then(function() {
assertLabel(['4', '5', '33.3%']);

return Plotly.restyle(gd, 'text', [['A', 'B', 'C', 'D', 'E']]);
})
.then(_hover)
.then(function() {
assertLabel(['4', 'E', '5', '33.3%']);

return Plotly.restyle(gd, 'hovertext', [[
'Apple', 'Banana', 'Clementine', 'Dragon Fruit', 'Eggplant'
]]);
})
.then(_hover)
.then(function() {
assertLabel(['4', 'Eggplant', '5', '33.3%']);

return Plotly.restyle(gd, 'hovertext', 'SUP');
})
.then(_hover)
.then(function() {
assertLabel(['4', 'SUP', '5', '33.3%']);
})
.then(done);
});

it('should show the correct separators for values', function(done) {

var expected = ['0', '12|345|678@91', '99@9%'];

mockCopy.layout.separators = '@|';
mockCopy.data[0].values[0] = 12345678.912;
mockCopy.data[0].values[1] = 10000;

Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(function() {

mouseEvent('mouseover', 223, 143);

var labels = Plotly.d3.selectAll('.hovertext .nums .line');

expect(labels[0].length).toBe(3);

labels.each(function(_, i) {
expect(Plotly.d3.select(this).text()).toBe(expected[i]);
});
}).then(done);
Plotly.plot(gd, mockCopy.data, mockCopy.layout)
.then(_hover)
.then(function() {
assertLabel(['0', '12|345|678@91', '99@9%']);
})
.then(done);
});
});
});
Expand Down