Skip to content

Hoverformat bug with showTickLabels #267

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 7 commits into from
Feb 24, 2016
Merged

Hoverformat bug with showTickLabels #267

merged 7 commits into from
Feb 24, 2016

Conversation

mdtusz
Copy link
Contributor

@mdtusz mdtusz commented Feb 19, 2016

Fixes #261.

Pulled some of the coersions out from the block that checks for showTickLabels so things act as expected, and added a quick testeroonie in there for good measure in the future.

@mdtusz mdtusz changed the title Bug hoverformat Hoverformat bug with showTickLabels Feb 19, 2016
};

beforeEach(function() {
this. gd = createGraphDiv();
Copy link
Contributor

Choose a reason for hiding this comment

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

Haha. Look at that. A jasmine this. Maybe I should start using those more.

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'm ok with this when it's idiomatic.

@etpinard
Copy link
Contributor

Great.

Once you figured out if tickformat and exponentformat can affect the hover labels, could you add some jasmine tests for handleTickDefaults in https://github.com/plotly/plotly.js/blob/master/test/jasmine/tests/axes_test.js ?

@mdtusz
Copy link
Contributor Author

mdtusz commented Feb 22, 2016

@etpinard let's try to get this one in for the next release too if we can.

coerce('exponentformat');
}
}

var tickPrefix = coerce('tickprefix');
if(tickPrefix) coerce('showtickprefix', showAttrDflt);

var tickSuffix = coerce('ticksuffix');
if(tickSuffix) coerce('showticksuffix', showAttrDflt);
Copy link
Contributor

Choose a reason for hiding this comment

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

These are not quite right still.

http://codepen.io/etpinard/pen/MKMNXe

tickprefix and ticksuffix have an effect (and should) on the hover labels; they should be coerced regardless of the showTickLabels value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yeah that's what I had done before but I thought you were saying they should be in the showTickLabels block. I'll pull them out again then.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, my bad. I wasn't very clear in my previous comment.

@mdtusz mdtusz added bug something broken status: in progress labels Feb 23, 2016
@mdtusz mdtusz self-assigned this Feb 23, 2016
coerce('exponentformat');
}
var tickFormat = coerce('tickformat');
if(!tickFormat && axType !== 'date') {
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this block not wrapped inside axType !== 'category' ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoa good catch. When I pulled out coerce('hoverformat') I completely goofed on that. I'll add it into the if condition right now.

@etpinard
Copy link
Contributor

yeah man 💃

mdtusz added a commit that referenced this pull request Feb 24, 2016
Hoverformat bug with showTickLabels
@mdtusz mdtusz merged commit 7b0036e into master Feb 24, 2016
@mdtusz mdtusz deleted the bug-hoverformat branch February 24, 2016 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants