Skip to content

Hover label namelength #1822

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 6 commits into from
Jul 19, 2017
Merged

Hover label namelength #1822

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e7aeee4
added hoverlabel namelength option
ckiss Jun 26, 2017
8abf2ea
zero means no trimming at all
ckiss Jun 26, 2017
ac2f7f4
fixed tests
ckiss Jun 26, 2017
8a308a8
review fixes
ckiss Jul 5, 2017
94f660a
finish and test hoverlabel.namelength
alexcjohnson Jul 19, 2017
e937519
integer can be arrayOk
alexcjohnson Jul 19, 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
14 changes: 10 additions & 4 deletions src/components/fx/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,18 @@ module.exports = {
color: extendFlat({}, fontAttrs.color, { arrayOk: true })
},
namelength: {
valType: 'number',
min: 0,
dflt: 15,
valType: 'integer',
min: -1,
arrayOk: true,
role: 'style',
Copy link
Contributor

Choose a reason for hiding this comment

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

We should try adding arrayOk support here, so that

trace = {
  x: [1, 2, 3],
  y: [2, 1, 2],
  namelength: [15, 10, 40]
}

would work.

Copy link
Contributor

Choose a reason for hiding this comment

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

see #1761 for an example on how to proceed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should try adding arrayOk support here

good call - at first I was worried that this would just be confusing to look at ("I thought this was the same trace, why does it have a different name?") but it could actually come in handy for example if you vary the hover label font size, a bigger one may need more truncation of the name 👍

description: 'Sets the length (in number of characters) of the hover labels for this trace'
description: [
'Sets the length (in number of characters) of the trace name in',
'the hover labels for this trace. -1 shows the whole name',
'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.'
].join(' ')
}
}
};
1 change: 1 addition & 0 deletions src/components/fx/calc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ module.exports = function calc(gd) {
fillFn(trace.hoverlabel.font.size, cd, 'hts');
fillFn(trace.hoverlabel.font.color, cd, 'htc');
fillFn(trace.hoverlabel.font.family, cd, 'htf');
fillFn(trace.hoverlabel.namelength, cd, 'hnl');
}
};

Expand Down
8 changes: 6 additions & 2 deletions src/components/fx/hover.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -689,8 +689,11 @@ function createHoverText(hoverData, opts, gd) {
// strip out our pseudo-html elements from d.name (if it exists at all)
name = svgTextUtils.plainText(d.name || '');

if(commonLabelOpts.namelength > 0 && name.length > commonLabelOpts.namelength) {
name = name.substr(0, commonLabelOpts.namelength - 3) + '...';
var nameLength = Math.round(d.nameLength);

if(nameLength > -1 && name.length > nameLength) {
if(nameLength > 3) name = name.substr(0, nameLength - 3) + '...';
else name = name.substr(0, nameLength);
}
}

Expand Down Expand Up @@ -1063,6 +1066,7 @@ function cleanPoint(d, hovermode) {
fill('fontFamily', 'htf', 'hoverlabel.font.family');
fill('fontSize', 'hts', 'hoverlabel.font.size');
fill('fontColor', 'htc', 'hoverlabel.font.color');
fill('nameLength', 'hnl', 'hoverlabel.namelength');

d.posref = hovermode === 'y' ? (d.x0 + d.x1) / 2 : (d.y0 + d.y1) / 2;

Expand Down
13 changes: 10 additions & 3 deletions src/components/fx/layout_attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,18 @@ module.exports = {
color: extendFlat({}, fontAttrs.color)
},
namelength: {
valType: 'number',
min: 0,
valType: 'integer',
min: -1,
dflt: 15,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't you need to remove dflt here too?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I kept only the global default, because the trace-level default inherits from the global so doesn't use a default of its own.

You're right that we want a default - in fact, we need a default of 15 to preserve backward compatibility - this is just how that default gets propagated.

role: 'style',
description: 'Sets the length (in number of characters) of the hover labels for this trace'
description: [
'Sets the default length (in number of characters) of the trace name in',
'the hover labels for all traces. -1 shows the whole name',
'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.'
].join(' ')
}
}
};
14 changes: 10 additions & 4 deletions test/jasmine/tests/hover_label_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,13 @@ describe('hover info on stacked subplots', function() {
});

describe('hover info on stacked subplots with shared y-axis', function() {
var mock = require('@mocks/stacked_subplots_shared_yaxis.json');
var mock = Lib.extendDeep(require('@mocks/stacked_subplots_shared_yaxis.json'));
mock.data[0].name = 'Who put the bomp in the bomp bah bomp bah bomp';
mock.data[0].hoverlabel = {namelength: -1};
mock.data[1].name = 'Who put the ram in the rama lama ding dong';
mock.data[1].hoverlabel = {namelength: [2, 4]};
mock.data[2].name = 'Who put the bop in the bop shoo bop shoo bop';
mock.layout.hoverlabel = {namelength: 10};

beforeEach(function(done) {
Plotly.plot(createGraphDiv(), mock.data, mock.layout).then(done);
Expand Down Expand Up @@ -727,11 +733,11 @@ describe('hover info on stacked subplots', function() {
expect(d3.selectAll('g.hovertext').size()).toEqual(3);
var textNodes = d3.selectAll('g.hovertext').selectAll('text');

expect(textNodes[0][0].innerHTML).toEqual('trace 0');
expect(textNodes[0][0].innerHTML).toEqual('Who put the bomp in the bomp bah bomp bah bomp');
expect(textNodes[0][1].innerHTML).toEqual('1');
expect(textNodes[1][0].innerHTML).toEqual('trace 1');
expect(textNodes[1][0].innerHTML).toEqual('Wh');
expect(textNodes[1][1].innerHTML).toEqual('2.1');
expect(textNodes[2][0].innerHTML).toEqual('trace 2');
expect(textNodes[2][0].innerHTML).toEqual('Who put...');
expect(textNodes[2][1].innerHTML).toEqual('3');
});
});
Expand Down