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

Legend centering and label click zones broken with non-standard font #11384

Closed
bolau opened this issue Jul 2, 2023 · 8 comments
Closed

Legend centering and label click zones broken with non-standard font #11384

bolau opened this issue Jul 2, 2023 · 8 comments

Comments

@bolau
Copy link

bolau commented Jul 2, 2023

Expected behavior

Even with a non-standard font, the legend should be centered properly, and clicking on a label should disable the correct dataset.

Current behavior

The legend seems to be positioned based on the wrong font width. Clicking on the right half of the second last label hides the last dataset instead of the second last. After that, the legend is re-centered, now correctly, and the click regions are correct.

Reproducible sample

https://www.borislau.de/chartjs_bug.html

Optional extra steps/info to reproduce

  • use a font that's wider than the regular by using Chart.defaults.font.family = 'Permanent Marker';
  • use 100% width/height
  • make many and wide labels, so that the error gets obvious

Possible solution

Didn't find a workaround.
Even waiting for document.fonts.ready doesn't help:

window.onload = function() {
    document.fonts.ready.then(function() {
        var ctx = document.getElementById('chartcanvas').getContext('2d');
        window.chart = new Chart(ctx, config);
    });
};

Context

I want to make a beautiful chart with the Bitstream Vera font, and I want to be able to hide and show datasets by clicking on the entries in the legend.

chart.js version

v4.3.0

Browser name and version

Firefox, Chrome, Safari

Link to your project

https://www.borislau.de/chartjs_bug.html

@bolau bolau added the type: bug label Jul 2, 2023
@bolau
Copy link
Author

bolau commented Jul 2, 2023

Note, that this applies to axis label positioning as well (see the y labels in my example).

Note that it doesn't make a difference, if I load the font via CSS font-face or JavaScript's FontFace API as suggested here: https://www.chartjs.org/docs/latest/general/fonts.html

Am I missing something, or is this an actual bug?
If you know a workaround, I'd be happy to learn about it!

@etimberg
Copy link
Member

etimberg commented Jul 2, 2023

In terms of how we actually do the measuring, we're relying on measureText from the canvas. I don't think we ever just assume that the width is a constant per character, so I'm not sure where this is coming from.

If you hadn't already tried to wait for the fonts to be ready, that would have been my first suggestion. The code for us to investigate will be

export function _measureText(
ctx: CanvasRenderingContext2D,
data: Record<string, number>,
gc: string[],
longest: number,
string: string
) {
let textWidth = data[string];
if (!textWidth) {
textWidth = data[string] = ctx.measureText(string).width;
gc.push(string);
}
if (textWidth > longest) {
longest = textWidth;
}
return longest;
}
.

What's curious is that when the Y axis maximum is < 5000, the labels are fine but they're wrong otherwise. It almost seems like in some cases we're sending the wrong labels to be measured

@bolau
Copy link
Author

bolau commented Jul 2, 2023

Thanks for the feedback! That helped me to further hunt down the issue.

At least in Firefox and Safari (but not in Chrome), the problem disappears if I assign the font to the canvas element via CSS, instead of just doing Chart.defaults.font.family = 'Vera';:
https://www.borislau.de/chartjs_bug_fixed.html

It seems that the measuring uses the default font. ctx.font = '12px Vera' also doesn't help, by the way.

@stockiNail
Copy link
Contributor

Forgive me if I'm jumping in the thread (I was curious). I had a quick look to the code.

@etimberg the _measureText function of the helpers is not used in the legend plugin where measureText of the context 2d is always used directly.

I have anyway found another weird behavior using the code of @bolau.
Locally and reducing the size (width and height), the legend items are not all showed and the hit boxes are of course wrong.
It could be related to root cause of this issue.

It seems something related to the max width acceptable on the legend, It seems (but not sure yet, I'm still having a look), that there is a different control between update (to calculate the hitboxes and location) and draw phases. It could be also related to PR #11352 which solved for vertical legend.

@stockiNail
Copy link
Contributor

@bolau @etimberg after a check, the code sound being correct.

But I see that the font is not loaded when chart is invoked.
Having a look to MDN, https://developer.mozilla.org/en-US/docs/Web/API/FontFace/load, @bolau I think you could use load method (and its Promise) of FontFace instead of document.fonts.ready promise:

const fontface = new FontFace("Vera", "url(Vera.ttf)")
document.fonts.add(fontface);

const config = {
    type: 'line',
    },
	.....		
}

window.onload = function() {
  fontface.load().then(function() {
    Chart.defaults.font.family = 'Vera';
    var ctx = document.getElementById('chartcanvas').getContext('2d');
     window.chart = new Chart(ctx, config);
  });
};

I have tested locally and sounds working.

@bolau
Copy link
Author

bolau commented Jul 3, 2023

@stockiNail very nice, indeed! This works! Thank you so much for your help!

Should we close this issue?

@stockiNail
Copy link
Contributor

Thank you so much for your help!

UR welcome

Should we close this issue?

Yes, I think so.

@etimberg
Copy link
Member

etimberg commented Jul 3, 2023

Thanks @stockiNail, good find on the font loading

@etimberg etimberg closed this as completed Jul 3, 2023
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

3 participants