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

Canvas height is always "width / 2" #75

Closed
technyon opened this issue Apr 19, 2024 · 6 comments
Closed

Canvas height is always "width / 2" #75

technyon opened this issue Apr 19, 2024 · 6 comments

Comments

@technyon
Copy link

technyon commented Apr 19, 2024

Describe the bug
For line charts (and maybe others, I only tried this one), if setting width and height, the height parameter is ignored and is always "width / 2". This issue probably is related to #15

To Reproduce
Steps to reproduce the behavior:

  • Go to LineSimple.Razor and set width and height, e. g.
    <Chart Config="_config1" @ref="_chart1" Width="500px" Height="400px"></Chart>
  • Run the demo and navigate to the "Line Simple" demo
  • Open the browsers developer tools
  • Using the element picker, check the canvas element
  • See width and height: height: 250px; width: 500p

Expected behavior

  • height parameter is as defined, in this example 400px

Desktop (please complete the following information):

  • OS: Windows
  • Browser Chrome, Edge
  • Version: Chrome: 121.0.6167.86

Additional context
As far as I can see, the _resize method in chart.js (from line 6855) overrides the width and height:

  _resize(width, height) {
    const options = this.options;
    const canvas = this.canvas;
    const aspectRatio = options.maintainAspectRatio && this.aspectRatio;
    const newSize = this.platform.getMaximumSize(canvas, width, height, aspectRatio);
    const newRatio = options.devicePixelRatio || this.platform.getDevicePixelRatio();
    const mode = this.width ? 'resize' : 'attach';
    this.width = newSize.width;
    this.height = newSize.height;
    this._aspectRatio = this.aspectRatio;
    if (!retinaScale(this, newRatio, true)) {
      return;
    

If I change the lines setting the width to this, it works as expected:

    this.width = width;
    this.height = height;

So, "this.platform.getMaximumSize(canvas, width, height, aspectRatio);" returns a smaller maximum size than requested. Although I have an idea how this happens, I'm not sure what the best fix for this is. There probably is a reason why getMaximumSize returns a smaller size.

As a side note, I've noticed that the canvas element can be removed from Chart.razor:

<canvas id="@Config.CanvasId" style="@(Height != null ? $"height:{Height}" : "") @(Width != null ? $"width:{Width};" : "")"></canvas>

I can remove this line, and everything still works as usual. It seems the canvas is created later in javascript.

@technyon
Copy link
Author

P.S.:

If setting width and height as described above, a check for undefined at the beginning of _resize has to be added:

      if (width === undefined) return;
      if (height === undefined) return;

@technyon
Copy link
Author

technyon commented Apr 19, 2024

getMaximumSize returns height = width / 2 because of the supplied aspect ratio of 2. As a workaround, I've disabled maintainAspectRatio in my code:

config.Options.MaintainAspectRatio = false;

This behavior is highly confusing when starting with this library. As a new user, I didn't know about this option and it took me quite a while to figure this out, only by digging into the javascript code. Maybe the default behavior should be changed to MaintainAspectRatio = false.

@erossini
Copy link
Owner

Thank you so much for your comment. I saw the issue with the line graph (only this one). I didn't want to change the chart.js script to avoid issues if I have to update the version of the script.

I'm using the default values from the ChartJs documentation.

Have you found the right configuration for the line chart? Is it now displayed using the size you want?

@technyon
Copy link
Author

If I MaintainAspectRatio to false it works as I need it.

@technyon
Copy link
Author

Overriding the defaults from C# should work I guess, no need to change chart.js

Do you prefer to close the issue for now, or keep it open?

@erossini
Copy link
Owner

Cool! Thank you for your update. We can close it then.

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

No branches or pull requests

2 participants