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

[BUG] Retina scaling no longer working in 2.4.0 (master branch) #3575

Closed
Driklyn opened this issue Nov 9, 2016 · 9 comments
Closed

[BUG] Retina scaling no longer working in 2.4.0 (master branch) #3575

Driklyn opened this issue Nov 9, 2016 · 9 comments
Milestone

Comments

@Driklyn
Copy link

Driklyn commented Nov 9, 2016

Testing out the latest using the master branch (since I needed a new feature) and now my charts are twice as big and blurry.

I believe it is related to this commit.

Expected Behavior

Using a width/height of 640x480, a canvas is created like so:

<canvas width="1280" height="960" style="display: block; width: 640px; height: 480px;"></canvas>

Current Behavior

With responsive set to false, the canvas is created as:

<canvas width="1280" height="960" style="display: block;"></canvas>

With responsive set to true, the canvas is created as:

<canvas width="2560" height="1920" style="display: block; width: 1280px; height: 960px;"></canvas>
@simonbrunel
Copy link
Member

Can you drop an example (HTML + JavaScript), here or in a jsFiddle?

@Driklyn
Copy link
Author

Driklyn commented Nov 9, 2016

Any example should do the trick (such as using the Getting Started example in the documentation). I would put together a JSFiddle but I don't think I can have it use 2.4.0 since it isn't released yet. If this is possible, let me know and I certainly will!

But here are a couple screenshots:

Using 400x400 with responsive set to false:

responsive-false

With responsive set to true:

responsive-true

Both charts appear as 800x800 and are twice as big as they should be.

@Driklyn
Copy link
Author

Driklyn commented Nov 9, 2016

Yep, the Getting Started example will work.

@simonbrunel
Copy link
Member

simonbrunel commented Nov 9, 2016

You right, it's broken since this commit change. I just didn't think about that retina case when only the canvas width/height are set. The proper way to size a chart (IMO) is to set the canvas style only: <canvas style="width: 640px; height: 480px"></canvas> in which case it should work as expected. Of course, the reported case needs to be fixed!

@simonbrunel simonbrunel added this to the Version 2.4 milestone Nov 9, 2016
@simonbrunel
Copy link
Member

However, do we agree that the responsive: true case is correct?

@simonbrunel simonbrunel mentioned this issue Nov 9, 2016
5 tasks
@Driklyn
Copy link
Author

Driklyn commented Nov 10, 2016

Ah, setting the width/height in style does indeed solve the issue! Thanks.

And yup, the responsive: true case is correct.

@simonbrunel
Copy link
Member

@Driklyn can you please pull latest master and confirm that it's fixed?

@Driklyn
Copy link
Author

Driklyn commented Nov 12, 2016

Yep, looks good now whether you're using <canvas width="640" height="480"></canvas> or <canvas style="width: 640px; height: 480px"></canvas>.

Thanks for the quick fix! 👍

@etimberg
Copy link
Member

thanks @Driklyn for verifying this

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