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.

Sign up for GitHub

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 aspect ratio and add responsive unit tests #3356

Merged
merged 4 commits into from
Sep 24, 2016
Merged

Fix aspect ratio and add responsive unit tests #3356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8ec7ce2
Gulp command switch to run specific test files
simonbrunel Sep 23, 2016
16bcd6a
Fix initial aspect ratio when not responsive
simonbrunel Sep 23, 2016
d610987
Fix radar default aspect ratio and samples
simonbrunel Sep 23, 2016
7d65bd3
Initial core.controller.js unit tests
simonbrunel Sep 23, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions gulpfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ var browserify = require('browserify');
var source = require('vinyl-source-stream');
var merge = require('merge-stream');
var collapse = require('bundle-collapser/plugin');
var argv = require('yargs').argv
var package = require('./package.json');

var srcDir = './src/';
Expand All @@ -33,11 +34,10 @@ var header = "/*!\n" +
" */\n";

var preTestFiles = [
'./node_modules/moment/min/moment.min.js',
'./node_modules/moment/min/moment.min.js'
];

var testFiles = [
'./test/mockContext.js',
'./test/*.js',

// Disable tests which need to be rewritten based on changes introduced by
Expand Down Expand Up @@ -157,10 +157,13 @@ function validHTMLTask() {
}

function startTest() {
var files = ['./src/**/*.js'];
Array.prototype.unshift.apply(files, preTestFiles);
Array.prototype.push.apply(files, testFiles);
return files;
return [].concat(preTestFiles).concat([
'./src/**/*.js',
'./test/mockContext.js'
]).concat(
argv.inputs?
argv.inputs.split(';'):
testFiles);
}

function unittestTask() {
Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@
"karma-jasmine": "^0.3.6",
"karma-jasmine-html-reporter": "^0.1.8",
"merge-stream": "^1.0.0",
"vinyl-source-stream": "^1.1.0"
"vinyl-source-stream": "^1.1.0",
"yargs": "^5.0.0"
},
"spm": {
"main": "Chart.js"
Expand Down
2 changes: 1 addition & 1 deletion samples/doughnut.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
</head>

<body>
<div id="canvas-holder" style="width:75%">
<div id="canvas-holder" style="width:40%">
<canvas id="chart-area" />
</div>
<button id="randomizeData">Randomize Data</button>
Expand Down
2 changes: 1 addition & 1 deletion samples/polar-area.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
</head>

<body>
<div id="canvas-holder" style="width:75%">
<div id="canvas-holder" style="width:40%">
<canvas id="chart-area"></canvas>
</div>
<button id="randomizeData">Randomize Data</button>
Expand Down
2 changes: 1 addition & 1 deletion samples/radar-skip-points.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
</head>

<body>
<div style="width:75%">
<div style="width:40%">
<canvas id="canvas"></canvas>
</div>
<button id="randomizeData">Randomize Data</button>
Expand Down
2 changes: 1 addition & 1 deletion samples/radar.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
</head>

<body>
<div style="width:75%">
<div style="width:40%">
<canvas id="canvas"></canvas>
</div>
<button id="randomizeData">Randomize Data</button>
Expand Down
1 change: 0 additions & 1 deletion src/charts/Chart.Radar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
module.exports = function(Chart) {

Chart.Radar = function(context, config) {
config.options = Chart.helpers.configMerge({aspectRatio: 1}, config.options);
config.type = 'radar';

return new Chart(context, config);
Expand Down
1 change: 1 addition & 0 deletions src/controllers/controller.radar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ module.exports = function(Chart) {
var helpers = Chart.helpers;

Chart.defaults.radar = {
aspectRatio: 1,
scale: {
type: 'radialLinear'
},
Expand Down
198 changes: 168 additions & 30 deletions src/core/core.controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
module.exports = function(Chart) {

var helpers = Chart.helpers;

// Create a dictionary of chart types, to allow for extension of existing types
Chart.types = {};

Expand All @@ -13,40 +14,176 @@ module.exports = function(Chart) {
// Controllers available for dataset visualization eg. bar, line, slice, etc.
Chart.controllers = {};

/**
* The "used" size is the final value of a dimension property after all calculations have
* been performed. This method uses the computed style of `element` but returns undefined
* if the computed style is not expressed in pixels. That can happen in some cases where
* `element` has a size relative to its parent and this last one is not yet displayed,
* for example because of `display: none` on a parent node.
* TODO(SB) Move this method in the upcoming core.platform class.
* @see https://developer.mozilla.org/en-US/docs/Web/CSS/used_value
* @returns {Number} Size in pixels or undefined if unknown.
*/
function readUsedSize(element, property) {
var value = helpers.getStyle(element, property);
var matches = value && value.match(/(\d+)px/);
return matches? Number(matches[1]) : undefined;
}

/**
* Initializes the canvas style and render size without modifying the canvas display size,
* since responsiveness is handled by the controller.resize() method. The config is used
* to determine the aspect ratio to apply in case no explicit height has been specified.
* TODO(SB) Move this method in the upcoming core.platform class.
*/
function initCanvas(canvas, config) {
var style = canvas.style;

// NOTE(SB) canvas.getAttribute('width') !== canvas.width: in the first case it
// returns null or '' if no explicit value has been set to the canvas attribute.
var renderHeight = canvas.getAttribute('height');
var renderWidth = canvas.getAttribute('width');

// Chart.js modifies some canvas values that we want to restore on destroy
canvas._chartjs = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this

initial: {
height: renderHeight,
width: renderWidth,
style: {
display: style.display,
height: style.height,
width: style.width
}
}
};

// Force canvas to display as block to avoid extra space caused by inline
// elements, which would interfere with the responsive resize process.
// https://github.com/chartjs/Chart.js/issues/2538
style.display = style.display || 'block';

if (renderWidth === null || renderWidth === '') {
var displayWidth = readUsedSize(canvas, 'width');
if (displayWidth !== undefined) {
canvas.width = displayWidth;
}
}

if (renderHeight === null || renderHeight === '') {
if (canvas.style.height === '') {
// If no explicit render height and style height, let's apply the aspect ratio,
// which one can be specified by the user but also by charts as default option
// (i.e. options.aspectRatio). If not specified, use canvas aspect ratio of 2.
canvas.height = canvas.width / (config.options.aspectRatio || 2);
} else {
var displayHeight = readUsedSize(canvas, 'height');
if (displayWidth !== undefined) {
canvas.height = displayHeight;
}
}
}

return canvas;
}

/**
* Restores the canvas initial state, such as render/display sizes and style.
* TODO(SB) Move this method in the upcoming core.platform class.
*/
function releaseCanvas(canvas) {
if (!canvas._chartjs) {
return;
}

var initial = canvas._chartjs.initial;
['height', 'width'].forEach(function(prop) {
var value = initial[prop];
if (value === undefined || value === null) {
canvas.removeAttribute(prop);
} else {
canvas.setAttribute(prop, value);
}
});

helpers.each(initial.style || {}, function(value, key) {
canvas.style[key] = value;
});

delete canvas._chartjs;
}

/**
* Initializes the given config with global and chart default values.
*/
function initConfig(config) {
config = config || {};
return helpers.configMerge({
options: helpers.configMerge(
Chart.defaults.global,
Chart.defaults[config.type],
config.options || {}),
data: {
datasets: [],
labels: []
}
}, config);
}

/**
* @class Chart.Controller
* The main controller of a chart.
*/
Chart.Controller = function(instance) {
Chart.Controller = function(context, config, instance) {
var me = this;
var canvas;

this.chart = instance;
this.config = instance.config;
this.options = this.config.options = helpers.configMerge(Chart.defaults.global, Chart.defaults[this.config.type], this.config.options || {});
this.id = helpers.uid();
config = initConfig(config);
canvas = initCanvas(context.canvas, config);

Object.defineProperty(this, 'data', {
instance.ctx = context;
instance.canvas = canvas;
instance.config = config;
instance.width = canvas.width;
instance.height = canvas.height;
instance.aspectRatio = canvas.width / canvas.height;

helpers.retinaScale(instance);

me.id = helpers.uid();
me.chart = instance;
me.config = instance.config;
me.options = me.config.options;

Object.defineProperty(me, 'data', {
get: function() {
return this.config.data;
return me.config.data;
}
});

// Always bind this so that if the responsive state changes we still work
helpers.addResizeListener(canvas.parentNode, function() {
if (me.config.options.responsive) {
me.resize();
}
});

// Add the chart instance to the global namespace
Chart.instances[this.id] = this;
Chart.instances[me.id] = me;

if (this.options.responsive) {
if (me.options.responsive) {
// Silent resize before chart draws
this.resize(true);
me.resize(true);
}

this.initialize();
me.initialize();

return this;
return me;
};

helpers.extend(Chart.Controller.prototype, /** @lends Chart.Controller */ {

initialize: function() {
var me = this;

// Before init plugin notification
Chart.plugins.notify('beforeInit', [me]);

Expand Down Expand Up @@ -82,22 +219,27 @@ module.exports = function(Chart) {
resize: function(silent) {
var me = this;
var chart = me.chart;
var options = me.options;
var canvas = chart.canvas;
var newWidth = helpers.getMaximumWidth(canvas);
var aspectRatio = chart.aspectRatio;
var newHeight = (me.options.maintainAspectRatio && isNaN(aspectRatio) === false && isFinite(aspectRatio) && aspectRatio !== 0) ? newWidth / aspectRatio : helpers.getMaximumHeight(canvas);
var aspectRatio = (options.maintainAspectRatio && chart.aspectRatio) || null;

var sizeChanged = chart.width !== newWidth || chart.height !== newHeight;
// the canvas render width and height will be casted to integers so make sure that
// the canvas display style uses the same integer values to avoid blurring effect.
var newWidth = Math.floor(helpers.getMaximumWidth(canvas));
var newHeight = Math.floor(aspectRatio? newWidth / aspectRatio : helpers.getMaximumHeight(canvas));

if (!sizeChanged) {
return me;
if (chart.width === newWidth && chart.height === newHeight) {
return;
}

canvas.width = chart.width = newWidth;
canvas.height = chart.height = newHeight;

helpers.retinaScale(chart);

canvas.style.width = newWidth + 'px';
canvas.style.height = newHeight + 'px';

// Notify any plugins about the resize
var newSize = {width: newWidth, height: newHeight};
Chart.plugins.notify('resize', [me, newSize]);
Expand All @@ -111,8 +253,6 @@ module.exports = function(Chart) {
me.stop();
me.update(me.options.responsiveAnimationDuration);
}

return me;
},

ensureScalesHaveIDs: function() {
Expand Down Expand Up @@ -539,25 +679,23 @@ module.exports = function(Chart) {

destroy: function() {
var me = this;
var canvas = me.chart.canvas;

me.stop();
me.clear();

helpers.unbindEvents(me, me.events);
helpers.removeResizeListener(me.chart.canvas.parentNode);

// Reset canvas height/width attributes
var canvas = me.chart.canvas;
canvas.width = me.chart.width;
canvas.height = me.chart.height;
if (canvas) {
helpers.removeResizeListener(canvas.parentNode);
releaseCanvas(canvas);
}

// if we scaled the canvas in response to a devicePixelRatio !== 1, we need to undo that transform here
if (me.chart.originalDevicePixelRatio !== undefined) {
me.chart.ctx.scale(1 / me.chart.originalDevicePixelRatio, 1 / me.chart.originalDevicePixelRatio);
}

// Reset to the old style since it may have been changed by the device pixel ratio changes
canvas.style.width = me.chart.originalCanvasStyleWidth;
canvas.style.height = me.chart.originalCanvasStyleHeight;

Chart.plugins.notify('destroy', [me]);

delete Chart.instances[me.id];
Expand Down
3 changes: 0 additions & 3 deletions src/core/core.helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -859,9 +859,6 @@ module.exports = function(Chart) {
// when destroy is called
chart.originalDevicePixelRatio = chart.originalDevicePixelRatio || pixelRatio;
}

canvas.style.width = width + 'px';
canvas.style.height = height + 'px';
};
// -- Canvas methods
helpers.clear = function(chart) {
Expand Down
Loading