-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
fix/4397 #4406
fix/4397 #4406
Conversation
Elements were resizing incorrectly if they were regenerated while collapsed. Added a check to avoid this issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left two minor comments. It'd also be nice to have a test for this that tries to create a chart inside of a collapsed div so that any errors here are prevented in the future
src/core/core.controller.js
Outdated
var newHeight = 0; | ||
|
||
// Avoid issues with the canvas having negative maximum width and/or height due to element being collapsed | ||
if (Math.floor(helpers.getMaximumWidth(canvas)) >= 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would change just just slightly to cache the return of helpers.getMaximumWidth(canvas)
into a local variable. This would improve minification and performance. The max width function reads some styles off the canvas element which can be slow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, and could be simply:
var newWidth = Math.max(0, Math.floor(helpers.getMaximumWidth(canvas)));
...
src/core/core.controller.js
Outdated
if (Math.floor(helpers.getMaximumWidth(canvas)) >= 0) { | ||
newWidth = Math.floor(helpers.getMaximumWidth(canvas)); | ||
} | ||
if (helpers.getMaximumHeight(canvas) >= 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment here about the return of helpers.getMaximumHeight
Elements were resizing incorrectly if they were resized while collapsed. Added a check to avoid this issue.