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

Font in root of the options does not get applied. #10375

Open
LeeLenaleee opened this issue May 25, 2022 · 8 comments
Open

Font in root of the options does not get applied. #10375

LeeLenaleee opened this issue May 25, 2022 · 8 comments

Comments

@LeeLenaleee
Copy link
Collaborator

Expected behavior

When specifieng font options in the root of the options object it should get applied to all the text which does not get overriden explicetly.

Current behavior

It does nothing. While according to the current typings it should make it the default base font:

Chart.js/types/index.esm.d.ts

Lines 1466 to 1470 in cf780a5

/**
* base font
* @see Defaults.font
*/
font: Partial<FontSpec>;

Reproducible sample

https://jsfiddle.net/2L1pdtsz/

Optional extra steps/info to reproduce

No response

Possible solution

No response

Context

Found this while working on #10364

chart.js version

3.7.1

Browser name and version

No response

Link to your project

No response

@nntcao
Copy link

nntcao commented Jun 13, 2022

I'm having some trouble looking at this issue and could use some guidance. From what I read in the docs, to change the default font for a chart you'd have to modify the defaults object directly:

Chart.defaults.font.size = 16;

But to support specifying the default font in the root of options would you have to modify the defaults object inside the Chart constructor?

Apologies if what I'm saying is confusing, it's my first time contributing to open source.

@LeeLenaleee
Copy link
Collaborator Author

LeeLenaleee commented Jun 14, 2022

Yes, like you said with underneath code you can modify the defaults. But this code modifies the default fontSize for all the charts. So if you have multiple charts in your application all of them now have a font size of 50 for all the elements in the chart that do not specify a different font size.

Chart.defaults.font.size = 50

Using below syntax should apply a fontSize of 50 to all the elements in only that specific chart. So if I have multiple charts all the other charts still have the default font size:

new Chart('canvasId', {
  data,
  options: {
    font: {
      size: 50
    }
  }
})

I hope this cleared it up for you

@nntcao
Copy link

nntcao commented Jun 14, 2022

Thank you for the clarification LeeLenaleee!

I looked a little deeper into the plugins that use the font, and I noticed they use the toFont helper function to generate the font:

const labelFont = toFont(labelOpts.font);

const fontOpts = toFont(opts.font);

Taking a deeper look at the toFont function lead me to this:

export function toFont(options, fallback) {
options = options || {};
fallback = fallback || defaults.font;
let size = valueOrDefault(options.size, fallback.size);
if (typeof size === 'string') {
size = parseInt(size, 10);
}
let style = valueOrDefault(options.style, fallback.style);
if (style && !('' + style).match(FONT_STYLE)) {
console.warn('Invalid font style specified: "' + style + '"');
style = '';
}
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;
}

The font is either the specified font option provided specifically by the plugin calling it or it defaults to fallback. For all the toFont calls however, there is no fallback provided and the fallback becomes the defaults object font. To resolve this issue, should the toFont function in all the plugins be supplied with a proper fallback (options.font)?

@LeeLenaleee
Copy link
Collaborator Author

I guess that everywhere that toFont is used the fallback has to be provided with options.font as done here:

const titleHeight = getTitleHeight(titleOpts, chart.options.font);

function getTitleHeight(options, fallback) {
if (!options.display) {
return 0;
}
const font = toFont(options.font, fallback);

Since the helper function does not have access to the chart instance I guess (can be wrong did not test) so then everywhere where toFont gets called it needs to be passed the default root font options. That should resolve the issue

@nntcao
Copy link

nntcao commented Jun 15, 2022

Thank you for the guidance, I'll make a pull request to change all the toFont() calls to include options.font as a proper fallback.

@kurkle
Copy link
Member

kurkle commented Jun 15, 2022

Another option is adding additionalOptionScopes: [''] to the plugins, but that affects all of the plugin options.

@nntcao
Copy link

nntcao commented Jun 18, 2022

@kurkle That solution looks a lot easier to implement.

but that affects all of the plugin options.

If I'm understanding correctly, it'd only be an issue if a Chart relied on the defaults and accidently overrode them using the root option scope. In my opinion though, this should be the ideal behaviour for any chart options. I think it should be fine to implement. What do you think?

@J4L0
Copy link

J4L0 commented Nov 24, 2022

Hello!

I am a little late to the party maybe, but I am suffering from the same / similar problem, using version 3.9.1.

The same default value Chart.defaults.font should, according to the documentation also be applied to the font in Points Label Options, but when I for ex. set:

Chart.defaults.font.size = '16'; and then don't define any value for pointsLabels.font.size, the default should be applied to the font size, which is not case by now.

The toFont function is also being used in the caller function, but for the reason that the last commit in helpers.options.js was in 2021, this issue has not been resolved yet I guess, is it?

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

No branches or pull requests

4 participants