chartjs / chartjs/Chart.js

'toFont' helper can affect lineHeight if called with a result object from a previous 'toFont' invocation

Open
#11,297 3 comments 0 reactions 0 assignees View on GitHub
type: enhancement
Dominant language
JavaScript
Stars
67.7k
Forks
11.9k
Avg merge
7h 39m
Merged PRs (30d)
5

Description

### Feature Proposal

Currently, if you invoke `toFont` helpers function passing as an argument the result object (anyway a font object), the `lineHeight `property is not correct because the original `lineHeight` overridden.

```javascript
const original = {
size: 12
};
const first = Chart.helpers.toFont(original);
const second = Chart.helpers.toFont(first);
```
Result:

```javascript
Original: {
"size": 12
}

First: {
"family": "'Helvetica Neue', 'Helvetica', 'Arial', sans-serif",
"lineHeight": 14.399999999999999,
"size": 12,
"style": "normal",
"weight": null,
"string": "normal 12px 'Helvetica Neue', 'Helvetica', 'Arial', sans-serif"
}

Second: {
"family": "'Helvetica Neue', 'Helvetica', 'Arial', sans-serif",
"lineHeight": 172.79999999999998,
"size": 12,
"style": "normal",
"weight": null,
"string": "normal 12px 'Helvetica Neue', 'Helvetica', 'Arial', sans-serif"
}
```

I think the `lineHeight` property should maintain always the original value and adding another property for the result `lineHeight`.

### Possible Implementation

Current Implementation (`helpers.options.ts`, row 130):

```javascript
const font = {
family: valueOrDefault(options.family, fallback.family),
lineHeight: toLineHeight(valueOrDefault(options.lineHeight, fallback.lineHeight), size),
size,
style,
weight: valueOrDefault(options.weight, fallback.weight),
string: ''
};

font.string = toFontString(font);
return font;
```

Possible implementation;

```javascript
const font = {
family: valueOrDefault(options.family, fallback.family),
lineHeight: valueOrDefault(options.lineHeight, fallback.lineHeight),
size,
style,
weight: valueOrDefault(options.weight, fallback.weight),
// new calculated properties
string: '',
lh: toLineHeight(valueOrDefault(options.lineHeight, fallback.lineHeight), size),
};

font.string = toFontString(font);
return font;
```

Of course, if implemented, it will be a breaking change.
What do you think?

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.