fix: Chart shrinking uncontrollably on browser zoom (#11089)

[gbaron]

My comment on the original issue contains the full analysis (scenario, root cause, how to reproduce):
#11089 (comment)

The functions retinaScale and getMaximumSize are full of minor details and multiple flows.
Hence, this area in the code is quite sensitive IMHO.
I'm making an attempt to fix the issue with minimum side effects.

Fixes #11089.

[kurkle]

Wondering if round1 would be enough to fix the issue..

[kurkle]

Or if removing the width rounding too would sctually fix it :)

[lincolnthree]

Really need this fix!

[Kebechet]

This fix is really needed ! My chart on Android browser shrinks all the time.

EDIT:
I have tested this fix on Android, iOS and Windows and it works correctly.
I have Chart.js built with this fix here

[garybutton]

Bumping this one, also really in need of the fix! 🙏

[mudassirzr]

Badly waiting for this fix, can we get this one merged and released soon please?

[fly74]

What is blocking the thing since February?

[etimberg]

@fly74 this hasn't been merged because this is very risky code to touch and this is an issue that the bulk of our users are not experiencing. While this fix solves this specific issue, there are no guarantees that it doesn't break other user's charts or introduces the same problem in other ways. @kurkle already commented that round1 instead of round should likely be used here since it will round to a single decimal place and I think that change would align better with what we've done elsewhere.

[fly74]

@etimberg I know that the use of HiDPI Monitors is not common for everybody but 4k with a scale is the future. This is resulting that the browser is choosing a zoom value which is sometimes like 116% or other what is resulting is this rounding issue. Why is it dangerous to round?

[etimberg]

@fly74 the risk is that we run into a scenario like this hence why we're reluctant to make a ton of changes to this code

1. Round the width
2. Height changes slightly
3. Scrollbar removed from container
4. Width increases so height is increased
5. Scrollbar comes back
6. Infinite loop

If we merge this, we're rounding the height. From what I recall when we last ran into this, rounding to the nearest integer was much more likely to cause the scrollbar problem hence rounding to a single decimal place where our testing showed that it was less likely to occur.

[lincolnthree]

@fly74 the risk is that we run into a scenario like this hence why we're reluctant to make a ton of changes to this code

A thought. Are you open to adding a configuration parameter or allowing a custom/passthrough rounding function to modify this behavior?

That way individual users could extend the behavior to meet their specific requirements? As it stands right now, we're in this situation you described with the existing functionality. It's just the chart that's freaking out, not the scrollbars (we have them disabled via CSS.)

[fly74]

@lincolnthree that would really a perfect solution for everybody who are waiting to get it work. Let's try it with a config parameter. Great idea 👌!

[etimberg]

@lincolnthree I would be open to having a config option for this + something in the docs discussing this problem and potential solutions.

@LeeLenaleee @kurkle opinions?

[gbaron]

@etimberg with regards to config option, what did you have in mind?

[etimberg]

@gbaron could be something like roundContainerHeightDuringResize which defaults to false. I'd probably put that under the layout options https://www.chartjs.org/docs/latest/configuration/layout.html