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

Jump to bottom

Add scale.pointLabels.lineHeight and scale.ticks.lineHeight options #5914

Merged
merged 4 commits into from Dec 20, 2018
Merged

Add scale.pointLabels.lineHeight and scale.ticks.lineHeight options #5914

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6535e7f
Add scale.pointLabels.lineHeight and scale.ticks.lineHeight options
nagix Dec 14, 2018
6b44d39
Add helpers.options.toFontString and helpers.options._parseFont
nagix Dec 16, 2018
b6f1d94
Move toFontString outside the export to make it fully private
nagix Dec 18, 2018
4780832
Remove code duplication
nagix Dec 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/axes/radial/linear.md
View file
Open in desktop
Expand Up @@ -109,3 +109,4 @@ The following options are used to configure the point labels that are shown on t
| `fontFamily` | `String` | `"'Helvetica Neue', 'Helvetica', 'Arial', sans-serif"` | Font family to use when rendering labels.
| `fontSize` | `Number` | 10 | font size in pixels.
| `fontStyle` | `String` | `'normal'` | Font style to use when rendering point labels.
| `lineHeight` | `Number/String` | `1.2` | Height of an individual line of text (see [MDN](https://developer.mozilla.org/en-US/docs/Web/CSS/line-height)).
3 changes: 3 additions & 0 deletions docs/axes/styling.md
View file
Open in desktop
Expand Up @@ -35,6 +35,7 @@ The tick configuration is nested under the scale configuration in the `ticks` ke
| `fontFamily` | `String` | `"'Helvetica Neue', 'Helvetica', 'Arial', sans-serif"` | Font family for the tick labels, follows CSS font-family options.
| `fontSize` | `Number` | `12` | Font size for the tick labels.
| `fontStyle` | `String` | `'normal'` | Font style for the tick labels, follows CSS font-style options (i.e. normal, italic, oblique, initial, inherit).
| `lineHeight` | `Number/String` | `1.2` | Height of an individual line of text (see [MDN](https://developer.mozilla.org/en-US/docs/Web/CSS/line-height)).
| `reverse` | `Boolean` | `false` | Reverses order of tick labels.
| `minor` | `object` | `{}` | Minor ticks configuration. Omitted options are inherited from options above.
| `major` | `object` | `{}` | Major ticks configuration. Omitted options are inherited from options above.
Expand All @@ -50,6 +51,7 @@ The minorTick configuration is nested under the ticks configuration in the `mino
| `fontFamily` | `String` | `"'Helvetica Neue', 'Helvetica', 'Arial', sans-serif"` | Font family for the tick labels, follows CSS font-family options.
| `fontSize` | `Number` | `12` | Font size for the tick labels.
| `fontStyle` | `String` | `'normal'` | Font style for the tick labels, follows CSS font-style options (i.e. normal, italic, oblique, initial, inherit).
| `lineHeight` | `Number/String` | `1.2` | Height of an individual line of text (see [MDN](https://developer.mozilla.org/en-US/docs/Web/CSS/line-height)).

## Major Tick Configuration
The majorTick configuration is nested under the ticks configuration in the `major` key. It defines options for the major tick marks that are generated by the axis. Omitted options are inherited from `ticks` configuration.
Expand All @@ -61,3 +63,4 @@ The majorTick configuration is nested under the ticks configuration in the `majo
| `fontFamily` | `String` | `"'Helvetica Neue', 'Helvetica', 'Arial', sans-serif"` | Font family for the tick labels, follows CSS font-family options.
| `fontSize` | `Number` | `12` | Font size for the tick labels.
| `fontStyle` | `String` | `'normal'` | Font style for the tick labels, follows CSS font-style options (i.e. normal, italic, oblique, initial, inherit).
| `lineHeight` | `Number/String` | `1.2` | Height of an individual line of text (see [MDN](https://developer.mozilla.org/en-US/docs/Web/CSS/line-height)).
2 changes: 1 addition & 1 deletion src/core/core.defaults.js
View file
Open in desktop
@@ -1,6 +1,6 @@
'use strict';

var helpers = require('../helpers/index');
var helpers = require('../helpers/helpers.core');

module.exports = {
/**
Expand Down
1 change: 1 addition & 0 deletions src/core/core.js
View file
Open in desktop
Expand Up @@ -19,6 +19,7 @@ defaults._set('global', {
defaultFontFamily: "'Helvetica Neue', 'Helvetica', 'Arial', sans-serif",
defaultFontSize: 12,
defaultFontStyle: 'normal',
defaultLineHeight: 1.2,
showLines: true,

// Element defaults defined in element extensions
Expand Down
92 changes: 34 additions & 58 deletions src/core/core.scale.js
View file
Open in desktop
Expand Up @@ -36,9 +36,6 @@ defaults._set('scale', {
// actual label
labelString: '',

// line height
lineHeight: 1.2,

// top/bottom padding
padding: {
top: 4,
Expand Down Expand Up @@ -99,27 +96,6 @@ function computeTextSize(context, tick, font) {
context.measureText(tick).width;
}

function parseFontOptions(options) {
var valueOrDefault = helpers.valueOrDefault;
var globalDefaults = defaults.global;
var size = valueOrDefault(options.fontSize, globalDefaults.defaultFontSize);
var style = valueOrDefault(options.fontStyle, globalDefaults.defaultFontStyle);
var family = valueOrDefault(options.fontFamily, globalDefaults.defaultFontFamily);

return {
size: size,
style: style,
family: family,
font: helpers.fontString(size, style, family)
};
}

function parseLineHeight(options) {
return helpers.options.toLineHeight(
helpers.valueOrDefault(options.lineHeight, 1.2),
helpers.valueOrDefault(options.fontSize, defaults.global.defaultFontSize));
}

module.exports = Element.extend({
/**
* Get the padding needed for the scale
Expand Down Expand Up @@ -341,13 +317,13 @@ module.exports = Element.extend({

// Get the width of each grid by calculating the difference
// between x offsets between 0 and 1.
var tickFont = parseFontOptions(tickOpts);
context.font = tickFont.font;
var tickFont = helpers.options._parseFont(tickOpts);
context.font = tickFont.string;

var labelRotation = tickOpts.minRotation || 0;

if (labels.length && me.options.display && me.isHorizontal()) {
var originalLabelWidth = helpers.longestText(context, tickFont.font, labels, me.longestTextCache);
var originalLabelWidth = helpers.longestText(context, tickFont.string, labels, me.longestTextCache);
var labelWidth = originalLabelWidth;
var cosRotation, sinRotation;

Expand Down Expand Up @@ -400,7 +376,8 @@ module.exports = Element.extend({
var position = opts.position;
var isHorizontal = me.isHorizontal();

var tickFont = parseFontOptions(tickOpts);
var parseFont = helpers.options._parseFont;
var tickFont = parseFont(tickOpts);
var tickMarkLength = opts.gridLines.tickMarkLength;

// Width
Expand All @@ -420,9 +397,9 @@ module.exports = Element.extend({

// Are we showing a title for the scale?
if (scaleLabelOpts.display && display) {
var scaleLabelLineHeight = parseLineHeight(scaleLabelOpts);
var scaleLabelFont = parseFont(scaleLabelOpts);
var scaleLabelPadding = helpers.options.toPadding(scaleLabelOpts.padding);
var deltaHeight = scaleLabelLineHeight + scaleLabelPadding.height;
var deltaHeight = scaleLabelFont.lineHeight + scaleLabelPadding.height;

if (isHorizontal) {
minSize.height += deltaHeight;
Expand All @@ -433,7 +410,7 @@ module.exports = Element.extend({

// Don't bother fitting the ticks if we are not showing them
if (tickOpts.display && display) {
var largestTextWidth = helpers.longestText(me.ctx, tickFont.font, labels, me.longestTextCache);
var largestTextWidth = helpers.longestText(me.ctx, tickFont.string, labels, me.longestTextCache);
var tallestLabelHeightInLines = helpers.numberOfLabelLines(labels);
var lineSpace = tickFont.size * 0.5;
var tickPadding = me.options.ticks.padding;
Expand All @@ -448,15 +425,14 @@ module.exports = Element.extend({

// TODO - improve this calculation
var labelHeight = (sinRotation * largestTextWidth)
+ (tickFont.size * tallestLabelHeightInLines)
+ (lineSpace * (tallestLabelHeightInLines - 1))
+ (tickFont.lineHeight * tallestLabelHeightInLines)
+ lineSpace; // padding

minSize.height = Math.min(me.maxHeight, minSize.height + labelHeight + tickPadding);

me.ctx.font = tickFont.font;
var firstLabelWidth = computeTextSize(me.ctx, labels[0], tickFont.font);
var lastLabelWidth = computeTextSize(me.ctx, labels[labels.length - 1], tickFont.font);
me.ctx.font = tickFont.string;
var firstLabelWidth = computeTextSize(me.ctx, labels[0], tickFont.string);
var lastLabelWidth = computeTextSize(me.ctx, labels[labels.length - 1], tickFont.string);
var offsetLeft = me.getPixelForTick(0) - me.left;
var offsetRight = me.right - me.getPixelForTick(labels.length - 1);
var paddingLeft, paddingRight;
Expand Down Expand Up @@ -690,6 +666,7 @@ module.exports = Element.extend({
var chart = me.chart;
var context = me.ctx;
var globalDefaults = defaults.global;
var defaultFontColor = globalDefaults.defaultFontColor;
var optionTicks = options.ticks.minor;
var optionMajorTicks = options.ticks.major || optionTicks;
var gridLines = options.gridLines;
Expand All @@ -700,18 +677,20 @@ module.exports = Element.extend({
var isMirrored = optionTicks.mirror;
var isHorizontal = me.isHorizontal();

var parseFont = helpers.options._parseFont;
var ticks = optionTicks.autoSkip ? me._autoSkip(me.getTicks()) : me.getTicks();
var tickFontColor = helpers.valueOrDefault(optionTicks.fontColor, globalDefaults.defaultFontColor);
var tickFont = parseFontOptions(optionTicks);
var majorTickFontColor = helpers.valueOrDefault(optionMajorTicks.fontColor, globalDefaults.defaultFontColor);
var majorTickFont = parseFontOptions(optionMajorTicks);
var tickFontColor = helpers.valueOrDefault(optionTicks.fontColor, defaultFontColor);
var tickFont = parseFont(optionTicks);
var lineHeight = tickFont.lineHeight;
var majorTickFontColor = helpers.valueOrDefault(optionMajorTicks.fontColor, defaultFontColor);
var majorTickFont = parseFont(optionMajorTicks);
var tickPadding = optionTicks.padding;
var labelOffset = optionTicks.labelOffset;

var tl = gridLines.drawTicks ? gridLines.tickMarkLength : 0;

var scaleLabelFontColor = helpers.valueOrDefault(scaleLabel.fontColor, globalDefaults.defaultFontColor);
var scaleLabelFont = parseFontOptions(scaleLabel);
var scaleLabelFontColor = helpers.valueOrDefault(scaleLabel.fontColor, defaultFontColor);
var scaleLabelFont = parseFont(scaleLabel);
var scaleLabelPadding = helpers.options.toPadding(scaleLabel.padding);
var labelRotationRadians = helpers.toRadians(me.labelRotation);

Expand Down Expand Up @@ -763,8 +742,8 @@ module.exports = Element.extend({
}

// Common properties
var tx1, ty1, tx2, ty2, x1, y1, x2, y2, labelX, labelY, textAlign;
var textBaseline = 'middle';
var tx1, ty1, tx2, ty2, x1, y1, x2, y2, labelX, labelY, textOffset, textAlign;
var labelCount = helpers.isArray(label) ? label.length : 1;
var lineValue = getPixelForGridLine(me, index, gridLines.offsetGridLines);

if (isHorizontal) {
Expand All @@ -782,13 +761,13 @@ module.exports = Element.extend({
if (position === 'top') {
y1 = alignPixel(chart, chartArea.top, axisWidth) + axisWidth / 2;
y2 = chartArea.bottom;
textBaseline = !isRotated ? 'bottom' : 'middle';
textOffset = ((!isRotated ? 0.5 : 1) - labelCount) * lineHeight;
textAlign = !isRotated ? 'center' : 'left';
labelY = me.bottom - labelYOffset;
} else {
y1 = chartArea.top;
y2 = alignPixel(chart, chartArea.bottom, axisWidth) - axisWidth / 2;
textBaseline = !isRotated ? 'top' : 'middle';
textOffset = (!isRotated ? 0.5 : 0) * lineHeight;
textAlign = !isRotated ? 'center' : 'right';
labelY = me.top + labelYOffset;
}
Expand All @@ -803,6 +782,7 @@ module.exports = Element.extend({
tx2 = tickEnd;
ty1 = ty2 = y1 = y2 = alignPixel(chart, lineValue, lineWidth);
labelY = me.getPixelForTick(index) + labelOffset;
textOffset = (1 - labelCount) * lineHeight / 2;

if (position === 'left') {
x1 = alignPixel(chart, chartArea.left, axisWidth) + axisWidth / 2;
Expand Down Expand Up @@ -835,7 +815,7 @@ module.exports = Element.extend({
rotation: -1 * labelRotationRadians,
label: label,
major: tick.major,
textBaseline: textBaseline,
textOffset: textOffset,
textAlign: textAlign
});
});
Expand Down Expand Up @@ -875,25 +855,21 @@ module.exports = Element.extend({
context.save();
context.translate(itemToDraw.labelX, itemToDraw.labelY);
context.rotate(itemToDraw.rotation);
context.font = itemToDraw.major ? majorTickFont.font : tickFont.font;
context.font = itemToDraw.major ? majorTickFont.string : tickFont.string;
context.fillStyle = itemToDraw.major ? majorTickFontColor : tickFontColor;
context.textBaseline = itemToDraw.textBaseline;
context.textBaseline = 'middle';
context.textAlign = itemToDraw.textAlign;

var label = itemToDraw.label;
var y = itemToDraw.textOffset;
if (helpers.isArray(label)) {
var lineCount = label.length;
var lineHeight = tickFont.size * 1.5;
var y = isHorizontal ? 0 : -lineHeight * (lineCount - 1) / 2;

for (var i = 0; i < lineCount; ++i) {
for (var i = 0; i < label.length; ++i) {
// We just make sure the multiline element is a string here..
context.fillText('' + label[i], 0, y);
// apply same lineSpacing as calculated @ L#320
y += lineHeight;
}
} else {
context.fillText(label, 0, 0);
context.fillText(label, 0, y);
}
context.restore();
}
Expand All @@ -904,7 +880,7 @@ module.exports = Element.extend({
var scaleLabelX;
var scaleLabelY;
var rotation = 0;
var halfLineHeight = parseLineHeight(scaleLabel) / 2;
var halfLineHeight = scaleLabelFont.lineHeight / 2;

if (isHorizontal) {
scaleLabelX = me.left + ((me.right - me.left) / 2); // midpoint of the width
Expand All @@ -926,7 +902,7 @@ module.exports = Element.extend({
context.textAlign = 'center';
context.textBaseline = 'middle';
context.fillStyle = scaleLabelFontColor; // render in correct colour
context.font = scaleLabelFont.font;
context.font = scaleLabelFont.string;
context.fillText(scaleLabel.labelString, 0, 0);
context.restore();
}
Expand Down
7 changes: 4 additions & 3 deletions src/elements/element.line.js
View file
Open in desktop
Expand Up @@ -4,15 +4,15 @@ var defaults = require('../core/core.defaults');
var Element = require('../core/core.element');
var helpers = require('../helpers/index');

var globalDefaults = defaults.global;
var defaultColor = defaults.global.defaultColor;

defaults._set('global', {
elements: {
line: {
tension: 0.4,
backgroundColor: globalDefaults.defaultColor,
backgroundColor: defaultColor,
borderWidth: 3,
borderColor: globalDefaults.defaultColor,
borderColor: defaultColor,
borderCapStyle: 'butt',
borderDash: [],
borderDashOffset: 0.0,
Expand All @@ -30,6 +30,7 @@ module.exports = Element.extend({
var ctx = me._chart.ctx;
var spanGaps = vm.spanGaps;
var points = me._children.slice(); // clone array
var globalDefaults = defaults.global;
var globalOptionLineElements = globalDefaults.elements.line;
var lastDrawnIndex = -1;
var index, current, previous, currentVM;
Expand Down
4 changes: 3 additions & 1 deletion src/elements/element.point.js
View file
Open in desktop
Expand Up @@ -73,6 +73,8 @@ module.exports = Element.extend({
var x = vm.x;
var y = vm.y;
var epsilon = 0.0000001; // 0.0000001 is margin in pixels for Accumulated error.
var globalDefaults = defaults.global;
var defaultColor = globalDefaults.defaultColor; // eslint-disable-line no-shadow

if (vm.skip) {
return;
Expand All @@ -81,7 +83,7 @@ module.exports = Element.extend({
// Clipping for Points.
if (chartArea === undefined || (model.x > chartArea.left - epsilon && chartArea.right + epsilon > model.x && model.y > chartArea.top - epsilon && chartArea.bottom + epsilon > model.y)) {
ctx.strokeStyle = vm.borderColor || defaultColor;
ctx.lineWidth = helpers.valueOrDefault(vm.borderWidth, defaults.global.elements.point.borderWidth);
ctx.lineWidth = helpers.valueOrDefault(vm.borderWidth, globalDefaults.elements.point.borderWidth);
ctx.fillStyle = vm.backgroundColor || defaultColor;
helpers.canvas.drawPoint(ctx, pointStyle, radius, x, y, rotation);
}
Expand Down
6 changes: 4 additions & 2 deletions src/elements/element.rectangle.js
View file
Open in desktop
Expand Up @@ -3,11 +3,13 @@
var defaults = require('../core/core.defaults');
var Element = require('../core/core.element');

var defaultColor = defaults.global.defaultColor;

defaults._set('global', {
elements: {
rectangle: {
backgroundColor: defaults.global.defaultColor,
borderColor: defaults.global.defaultColor,
backgroundColor: defaultColor,
borderColor: defaultColor,
borderSkipped: 'bottom',
borderWidth: 0
}
Expand Down
41 changes: 41 additions & 0 deletions src/helpers/helpers.options.js
View file
Open in desktop
@@ -1,5 +1,6 @@
'use strict';

var defaults = require('../core/core.defaults');
var helpers = require('./helpers.core');

/**
Expand Down Expand Up @@ -65,6 +66,46 @@ module.exports = {
};
},

/**
* Converts the given font object into a CSS font string.
* @param {Object} font - A font object.
* @return {Stringg} The CSS font string. See https://developer.mozilla.org/en-US/docs/Web/CSS/font
*/
toFontString: function(font) {
Copy link
Member

Choose a reason for hiding this comment

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

I would not expose this method to the public API until we need to, but instead I would declare it outside the export (we can test this method via _parseFont). We should also deprecate helpers.fontString, which could be moved at the end of this file and fallback to this method (similar to what's done in the canvas helpers)

Copy link
Contributor Author

@nagix nagix Dec 18, 2018 •
edited

Choose a reason for hiding this comment

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

helpers.fontString is called from core.tooltip. I think tooltips.{body|title|footer}Font{Family|Size|Style|Color} should be replaced with tooltips.*.font.* in v3, but _parseFont cannot be used for these options in the current version. Should we leave helpers.fontString for now?

Copy link
Member

Choose a reason for hiding this comment

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

That's fine to keep using fontString in this case, though I would still move toFontString outside the export to make it fully private for now. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I agree.

if (!font || helpers.isNullOrUndef(font.size) || helpers.isNullOrUndef(font.family)) {
return null;
}

return (font.style ? font.style + ' ' : '')
+ (font.weight ? font.weight + ' ' : '')
+ font.size + 'px '
+ font.family;
},

/**
* Parses font options and returns the font object.
* @param {Object} options - A object that contains font opttons to be parsed.
* @return {Object} The font object.
* @todo Support font.* options and renamed to toFont().
* @private
*/
_parseFont: function(options) {
var valueOrDefault = helpers.valueOrDefault;
var globalDefaults = defaults.global;
var size = valueOrDefault(options.fontSize, globalDefaults.defaultFontSize);
var font = {
family: valueOrDefault(options.fontFamily, globalDefaults.defaultFontFamily),
lineHeight: helpers.options.toLineHeight(valueOrDefault(options.lineHeight, globalDefaults.defaultLineHeight), size),
size: size,
style: valueOrDefault(options.fontStyle, globalDefaults.defaultFontStyle),
weight: null,
string: ''
};

font.string = helpers.options.toFontString(font);
return font;
},

/**
* Evaluates the given `inputs` sequentially and returns the first defined value.
* @param {Array[]} inputs - An array of values, falling back to the last value.
Expand Down