Skip to content
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

Candlestick hover label fix #2264

Merged
merged 1 commit into from
Jan 18, 2018
+20 −3
Merged

Candlestick hover label fix #2264

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions src/traces/box/calc.js
View file
Original file line number Diff line number Diff line change
@@ -136,6 +136,11 @@ module.exports = function calc(gd, trace) {
}
};

// don't show labels in candlestick hover labels
if(trace._fullInput && trace._fullInput.type === 'candlestick') {
delete cd[0].t.labels;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

not a big deal, but 🐎 I might invert this logic and only add labels if we do want them.

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 thought about that. But, I prefer writing candlestick-only blocks like this one as exceptions to a rule (the rule here being fill up cd[0].t.labels). Here's another example.

I can't wait to rewrite candlestick and ohlc as good old (non-transform) trace types 🙃

Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer writing candlestick-only blocks like this one as exceptions to a rule

Good point.


fullLayout[numKey]++;
return cd;
} else {
2 changes: 1 addition & 1 deletion src/traces/box/hover.js
View file
Original file line number Diff line number Diff line change
@@ -159,7 +159,7 @@ function hoverOnBoxes(pointData, xval, yval, hovermode) {

pointData2[vLetter + '0'] = pointData2[vLetter + '1'] = valPx;
pointData2[vLetter + 'LabelVal'] = val;
pointData2[vLetter + 'Label'] = t.labels[attr] + ' ' + Axes.hoverLabelText(vAxis, val);
pointData2[vLetter + 'Label'] = (t.labels ? t.labels[attr] + ' ' : '') + Axes.hoverLabelText(vAxis, val);

if(attr === 'mean' && ('sd' in di) && trace.boxmean === 'sd') {
pointData2[vLetter + 'err'] = di.sd;
16 changes: 14 additions & 2 deletions test/jasmine/tests/finance_test.js
View file
Original file line number Diff line number Diff line change
@@ -384,16 +384,19 @@ describe('finance charts calc transforms:', function() {
return calcTrace[0].trace;
}

function _calc(data, layout) {
function _calcRaw(data, layout) {
var gd = {
data: data,
layout: layout || {}
};

supplyAllDefaults(gd);
Plots.doCalcdata(gd);
return gd.calcdata;
}

return gd.calcdata.map(calcDatatoTrace);
function _calc(data, layout) {
return _calcRaw(data, layout).map(calcDatatoTrace);
}

// add some points that shouldn't make it into calcdata because
@@ -758,6 +761,15 @@ describe('finance charts calc transforms:', function() {
2, 2, 2, 2, 2, 2
]);
});

it('should not include box hover labels prefix in candlestick calcdata', function() {
var trace0 = Lib.extendDeep({}, mock0, {
type: 'candlestick',
});
var out = _calcRaw([trace0]);

expect(out[0][0].t.labels).toBeUndefined();
});
});

describe('finance charts updates:', function() {