fix: respect aspect ratio with container height (#10646)

* fix: respect aspect ratio with container height

* docs: add info into migration guide

diff --git a/docs/migration/v4-migration.md b/docs/migration/v4-migration.md
index c2700528bdb2f6f46b6df4310ee960052c82775e..e34483ac2a9121de7a7227649eb2b5d724dbf801 100644 (file)
--- a/docs/migration/v4-migration.md
+++ b/docs/migration/v4-migration.md
@@ -26,6 +26,7 @@ A number of changes were made to the configuration options passed to the `Chart`
 * The z index for the border of a scale is now configurable instead of being 1 higher as the grid z index.
 * Linear scales now add and subtracts `5%` of the max value to the range if the min and max are the same instead of `1`.
 * If the tooltip callback returns `undefined`, then the default callback will be used.
+* `maintainAspectRatio` respects container height.
 
 #### Type changes
 * The order of the `ChartMeta` parameters have been changed from `<Element, DatasetElement, Type>` to `<Type, Element, DatasetElement>`.

diff --git a/src/helpers/helpers.dom.js b/src/helpers/helpers.dom.js
index 215e8f2f31838fa084dcfcf77357c4452d8895a1..5ab3778394277d1cbb090ab02a5384ac6b666e26 100644 (file)
--- a/src/helpers/helpers.dom.js
+++ b/src/helpers/helpers.dom.js
@@ -174,6 +174,12 @@ export function getMaximumSize(canvas, bbWidth, bbHeight, aspectRatio) {
     // If the canvas has width, but no height, default to aspectRatio of 2 (canvas default)
     height = round1(width / 2);
   }
+
+  if (aspectRatio && height > containerSize.height) {
+    height = containerSize.height;
+    width = round1(Math.floor(height * aspectRatio));
+  }
+
   return {
     width,
     height

diff --git a/test/specs/core.controller.tests.js b/test/specs/core.controller.tests.js
index a75267b8d747be03898ba07e907aeb597a489b2b..d1b2e424603bd28899b1936394aec615e2b5b456 100644 (file)
--- a/test/specs/core.controller.tests.js
+++ b/test/specs/core.controller.tests.js
@@ -1254,6 +1254,7 @@ describe('Chart', function() {
           done();
         });
         canvas.parentNode.style.width = '455px';
+        canvas.parentNode.style.height = '455px';
       });
     });
 
@@ -1341,7 +1342,7 @@ describe('Chart', function() {
       wrapper.style.width = '450px';
     });
 
-    it('should not resize the canvas when parent height changes', function(done) {
+    it('should maintain aspect ratio when parent height changes', function(done) {
       var chart = acquireChart({
         options: {
           responsive: true,
@@ -1370,8 +1371,8 @@ describe('Chart', function() {
 
         waitForResize(chart, function() {
           expect(chart).toBeChartOfSize({
-            dw: 320, dh: 160,
-            rw: 320, rh: 160,
+            dw: 300, dh: 150,
+            rw: 300, rh: 150,
           });
 
           done();
@@ -1857,6 +1858,7 @@ describe('Chart', function() {
         done();
       });
       chart.canvas.parentNode.style.width = '400px';
+      chart.canvas.parentNode.style.height = '400px';
     });
 
     it ('should notify initially disabled plugin in correct order', function() {

diff --git a/test/specs/helpers.dom.tests.js b/test/specs/helpers.dom.tests.js
index 24dbe81b68aa959bd05bf27507bb208672159d86..dfe41f1c3ef3860b259eee8ca72526b54cd6dda6 100644 (file)
--- a/test/specs/helpers.dom.tests.js
+++ b/test/specs/helpers.dom.tests.js
@@ -453,4 +453,38 @@ describe('DOM helpers tests', function() {
       expect(nativePosition).toEqual({x: chartPosition.x, y: chartPosition.y});
     });
   });
+
+  it('should respect aspect ratio and container width', () => {
+    const container = document.createElement('div');
+    container.style.width = '200px';
+    container.style.height = '500px';
+
+    document.body.appendChild(container);
+
+    const target = document.createElement('div');
+    target.style.width = '500px';
+    target.style.height = '500px';
+    container.appendChild(target);
+
+    expect(helpers.getMaximumSize(target, 200, 500, 1)).toEqual(jasmine.objectContaining({width: 200, height: 200}));
+
+    document.body.removeChild(container);
+  });
+
+  it('should respect aspect ratio and container height', () => {
+    const container = document.createElement('div');
+    container.style.width = '500px';
+    container.style.height = '200px';
+
+    document.body.appendChild(container);
+
+    const target = document.createElement('div');
+    target.style.width = '500px';
+    target.style.height = '500px';
+    container.appendChild(target);
+
+    expect(helpers.getMaximumSize(target, 500, 200, 1)).toEqual(jasmine.objectContaining({width: 200, height: 200}));
+
+    document.body.removeChild(container);
+  });
 });

diff --git a/test/specs/platform.dom.tests.js b/test/specs/platform.dom.tests.js
index b3b05a5cacf4f32f26c1d60cbf6251826e57ef4d..a2ba3ac62f62e9b4b0319f087a896fd911be5980 100644 (file)
--- a/test/specs/platform.dom.tests.js
+++ b/test/specs/platform.dom.tests.js
@@ -271,7 +271,7 @@ describe('Platform.dom', function() {
   });
 
   describe('config.options.responsive: true (maintainAspectRatio: true)', function() {
-    it('should fill parent width and use aspect ratio to calculate height', function() {
+    it('should fit parent using aspect ratio to calculate size', function() {
       var chart = acquireChart({
         options: {
           responsive: true,
@@ -287,8 +287,8 @@ describe('Platform.dom', function() {
       });
 
       expect(chart).toBeChartOfSize({
-        dw: 300, dh: 490,
-        rw: 300, rh: 490,
+        dw: 214, dh: 350,
+        rw: 214, rh: 350,
       });
     });
   });