From: Dan Onoshko <danon0404@gmail.com>
Date: Fri, 2 Sep 2022 12:03:41 +0000 (+0400)
Subject: fix: respect aspect ratio with container height (#10646)
X-Git-Tag: v4.0.0~31
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0c51ecd451e577e82b0a2ada5483ec471b78a2de;p=thirdparty%2FChart.js.git

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 c2700528b..e34483ac2 100644
--- 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 215e8f2f3..5ab377839 100644
--- 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 a75267b8d..d1b2e4246 100644
--- 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 24dbe81b6..dfe41f1c3 100644
--- 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 b3b05a5ca..a2ba3ac62 100644
--- 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,
       });
     });
   });