Fix inconsistent aspect ratio

[jtblin]

Please consider the following before submitting a pull request:

Guidelines for contributing: https://github.com/chartjs/Chart.js/blob/master/CONTRIBUTING.md

Example of changes on an interactive website such as the following:

• http://jsbin.com/
• http://jsfiddle.net/
• http://codepen.io/pen/
• Premade template: http://codepen.io/pen?template=JXVYzq

[jtblin]

This PR makes the aspect ratio consistent between all chart types.

[etimberg]

@simonbrunel thoughts? I'm ok with making this change, but we should probably add some documentation because otherwise users might complain about the change (we made this change very explicitly in response to a user filed issue about the pie/doughnut charts not using the entire canvas

[simonbrunel]

I'm not sure what users expect for these charts (square or rectangular default size) and can't find tickets about people complaining of the default aspect ratio (I might have missed it). It's a second breaking change that may certainly impact many projects. The first breaking change was a bug fix, this one looks more like a preference?

If everyone agree with it then I'm fine. Though, I'm curious why this change is needed in the lib instead of modifying defaults on the project side?

Chart.defaults.polarArea.aspectRatio = 2;
Chart.defaults.doughnut.aspectRatio = 2;
Chart.defaults.radar.aspectRatio = 2;

[simonbrunel]

This test needs to verify the same values, defaults need to be changed before and after the test:

it('should use default "chart" aspect ratio for render and display sizes', function() {
    var ratio = Chart.defaults.doughnut.aspectRatio;
    Chart.defaults.doughnut.aspectRatio = 1;

    var chart = acquireChart({
        type: 'doughnut',
        // ...

    expect(chart).toBeChartOfSize({
        // ...
    });

    Chart.defaults.doughnut.aspectRatio = ratio;
});

[jtblin]

@simonbrunel I've done the change you requested.

The first breaking change was a bug fix, this one looks more like a preference?

I disagree with this definition :) The bug fix (correcting handling aspect ratio) didn't need to introduce a breaking change (changing the aspect ratio of pie charts and family) and it now makes the charts look inconsistent. Here is how it looks like with the defaults:

As you can see, it looks really bad and each user of chartjs shouldn't have to play with aspect ratio, it should look great by default. We may want to document the aspect ratio prominently if not already done.

[simonbrunel]

You right, I should have changed the default aspect ratio to 2 for all charts when fixing that bug, so no breaking changes would have been introduced. I also agree with "... it should looks great by default.", but it seems that some users reported that these kind of charts doesn't look good with an aspect ratio of 2 (@etimberg do you remember tickets by any chance?).

Anyway, I'm good with these changes.

[etimberg]

I don't recall the ticket unfortunately. I'm fine with these changes as well

[jtblin]

Thanks guys! 👍