Fix aspect ratio and add responsive unit tests #3356
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add the --inputs command switch to the unittest and unittestWatch tasks, to be able to run unit tests from the specified files only (e.g. gulp unittest --inputs=test/core.element.tests.js;test/core.helpers.tests.js).
When responsive is false and no canvas height explicitly set, the aspectRatio option wasn't applied because of the canvas default height. Prevent the retinaScale method to change the canvas display size since this method is called for none responsive charts, but instead make the resize() responsible of these changes. Also, as discussed some time ago, moved most of the core.js logic into core.controller.js. Clean up the destroy process and make sure that initial canvas values are properly saved and restored.
Now that the aspect ratio is correctly handled, fix samples for charts with aspect ratio of 1 which was vertically too large. Also fix the default aspect ratio for radar charts which wasn't applied when creating a chart directly using new Chart(ctx, { type: 'radar' }).
simonbrunel
force-pushed
the
aspect-ratio
branch
4 times, most recently
from
4728bd5 to
5c8d98b
Compare
simonbrunel
force-pushed
the
aspect-ratio
branch
from
5c8d98b to
7d65bd3
Compare
|
Finally stabilized unit tests (was actually failing because of #3152) ... so ready for @chartjs/maintainers review :) |
|
Looks good to me
|
etimberg
approved these changes
Sep 24, 2016
| var renderWidth = canvas.getAttribute('width'); | ||
|
|
||
| // Chart.js modifies some canvas values that we want to restore on destroy | ||
| canvas._chartjs = { |
exwm
pushed a commit
to exwm/Chart.js
that referenced
this pull request
Apr 30, 2021
Fix aspect ratio and add responsive unit tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Should have been only unit tests but then I realized that aspect ratio for none responsive chart was totally broken. So I decided to clean up the whole canvas initialization process and I hope to not have broken too many things :)
This PR includes:
aspectRatiofor radar chart and associated samplescore.controller.jstest/core.controller.tests.jscurrently testing AR and responsiveness (see screenshot below)gulp unittest --inputs=test/core.controller.tests.js)More details in commit messages