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.

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

Line height setting for scale titles #4387

Merged
merged 1 commit into from
Jun 23, 2017
Merged

Line height setting for scale titles #4387

merged 1 commit into from
Jun 23, 2017

Conversation

etimberg
Copy link
Member

The text is centered within the line height, so setting the line height to a size greater than the font size moves it away from the axis edge.

Resolves #3562

… line height, so setting the line height to a size

greater than the font size moves it away from the axis edge.
@etimberg
Copy link
Member Author

@simonbrunel as we look at these line height settings, should we also consider adding settings for the text baseline and alignment or do you think that's too much?

@simonbrunel
Copy link
Member

I implemented textAlign for the datalabels plugin so I can have a look at it as well (I will refactor text size computation and drawing in helpers). I'm not sure we need baseline except if there is tickets asking for it.

@@ -308,7 +308,7 @@ module.exports = function(Chart) {
var isHorizontal = me.isHorizontal();

var tickFont = parseFontOptions(tickOpts);
var scaleLabelFontSize = parseFontOptions(scaleLabelOpts).size * 1.5;
var scaleLabelLineHeight = helpers.getValueOrDefault(scaleLabelOpts.lineHeight, parseFontOptions(scaleLabelOpts).size * 1.5);
Copy link
Member

Choose a reason for hiding this comment

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

Why font size *1.5 by default?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this was to give some spacing between the bottom of the text and the ticks. I didn't want to change that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants