refactor!: prevent vaadin-chart from expanding parent containers (#10366)

Author: tomivirkki

packages/charts/src/vaadin-chart-mixin.js

@@ -795,10 +795,6 @@ export const ChartMixin = (superClass) =>
         // Detect if the chart had already been initialized. This might happen in
         // environments where the chart is lazily attached (e.g Grid).
         if (this.configuration) {
-          const { height } = this.$.wrapper.style;
-          this.$.wrapper.style.height = '';
-          this.configuration.setSize(null, null, false);
-          this.$.wrapper.style.height = height;
           return;
         }
 
@@ -829,6 +825,16 @@ export const ChartMixin = (superClass) =>
       const { height, width } = contentRect;
       const { chartHeight, chartWidth } = this.configuration;
 
+      this.$.wrapper.style.minHeight = '';
+      // Use 1px as the threshold to align with Highcharts
+      if (this.$.wrapper.offsetHeight <= 1) {
+        this.$.wrapper.style.minHeight = `${chartHeight}px`;
+      }
+      this.$.wrapper.style.minWidth = '';
+      if (this.$.wrapper.offsetWidth <= 1) {
+        this.$.wrapper.style.minWidth = `${chartWidth}px`;
+      }
+
       if (height !== chartHeight || width !== chartWidth) {
         this.__reflow();
       }

packages/charts/src/vaadin-chart.js

@@ -178,8 +178,8 @@ class Chart extends ChartMixin(ThemableMixin(ElementMixin(PolylitMixin(LumoInjec
   /** @protected */
   render() {
     return html`
-      <div id="wrapper" style="height: 100%; width: 100%;">
-        <div id="chart" style="height: 100%; width: 100%;"></div>
+      <div id="wrapper" style="height: 100%; width: 100%; position: relative;">
+        <div id="chart" style="height: 100%; width: 100%; position: absolute;"></div>
       </div>
       <slot id="slot"></slot>
     `;

packages/charts/test/chart-element.test.js

@@ -1,5 +1,5 @@
 import { expect } from '@vaadin/chai-plugins';
-import { aTimeout, fixtureSync, nextFrame, nextRender, oneEvent } from '@vaadin/testing-helpers';
+import { aTimeout, fixtureSync, nextFrame, nextRender, nextResize, oneEvent } from '@vaadin/testing-helpers';
 import sinon from 'sinon';
 import './chart-not-animated-styles.js';
 import '../src/vaadin-chart.js';
@@ -344,6 +344,47 @@ describe('vaadin-chart', () => {
       expect(rect.width).to.be.equal(300);
       expect(chart.configuration.chartWidth).to.be.equal(300);
     });
+
+    it('should use intrinsic width when container provides no width', async () => {
+      chart = fixtureSync(`
+        <div style="display: inline-flex; height: 300px">
+          <vaadin-chart></vaadin-chart>
+        </div>`).querySelector('vaadin-chart');
+
+      await nextResize(chart);
+
+      expect(chart.offsetHeight).to.be.equal(300);
+      expect(chart.offsetWidth).to.be.greaterThan(0);
+      expect(chart.configuration.chartHeight).to.be.equal(chart.offsetHeight);
+    });
+
+    it('should resize from intrinsic width to explicit width', async () => {
+      chart = fixtureSync(`
+        <div style="display: inline-flex; height: 300px">
+          <vaadin-chart></vaadin-chart>
+        </div>`).querySelector('vaadin-chart');
+
+      await nextResize(chart);
+
+      chart.style.width = '100px';
+      await nextResize(chart);
+
+      expect(chart.offsetHeight).to.be.equal(300);
+      expect(chart.offsetWidth).to.be.equal(100);
+      expect(chart.configuration.chartWidth).to.be.equal(chart.offsetWidth);
+    });
+
+    it('should collapse to zero width when container width is zero', async () => {
+      chart = fixtureSync(`
+        <div style="display: inline-flex; height: 300px; width: 0px">
+          <vaadin-chart></vaadin-chart>
+        </div>`).querySelector('vaadin-chart');
+
+      await nextResize(chart);
+
+      expect(chart.offsetHeight).to.be.equal(300);
+      expect(chart.offsetWidth).to.be.equal(0);
+    });
   });
 
   describe('height', () => {
@@ -367,6 +408,66 @@ describe('vaadin-chart', () => {
       expect(rect.height).to.be.equal(300);
       expect(chart.configuration.chartHeight).to.be.equal(300);
     });
+
+    it('should not cause container height expansion in flex layout', async () => {
+      const chartMinHeight = 300;
+      const siblingHeight = 5;
+
+      // See https://github.com/vaadin/web-components/issues/10316
+      chart = fixtureSync(`
+        <div style="display: flex">
+          <div>
+              <vaadin-chart style="height: 100%; min-height: ${chartMinHeight}px;"></vaadin-chart>
+              <div style="height: ${siblingHeight}px"></div>
+          </div>
+        </div>`).querySelector('vaadin-chart');
+
+      await nextResize(chart);
+
+      expect(chart.offsetHeight).to.be.equal(chartMinHeight + siblingHeight);
+      expect(chart.configuration.chartHeight).to.be.equal(chart.offsetHeight);
+    });
+
+    it('should use intrinsic height when container provides no height', async () => {
+      chart = fixtureSync(`
+        <div style="display: flex; width: 300px">
+          <vaadin-chart></vaadin-chart>
+        </div>`).querySelector('vaadin-chart');
+
+      await nextResize(chart);
+
+      expect(chart.offsetWidth).to.be.equal(300);
+      expect(chart.offsetHeight).to.be.greaterThan(0);
+      expect(chart.configuration.chartHeight).to.be.equal(chart.offsetHeight);
+    });
+
+    it('should resize from intrinsic height to explicit height', async () => {
+      chart = fixtureSync(`
+        <div style="display: flex; width: 300px">
+          <vaadin-chart></vaadin-chart>
+        </div>`).querySelector('vaadin-chart');
+
+      await nextResize(chart);
+
+      chart.style.height = '100px';
+      await nextResize(chart);
+
+      expect(chart.offsetWidth).to.be.equal(300);
+      expect(chart.offsetHeight).to.be.equal(100);
+      expect(chart.configuration.chartHeight).to.be.equal(chart.offsetHeight);
+    });
+
+    it('should collapse to zero height when container height is zero', async () => {
+      chart = fixtureSync(`
+        <div style="display: flex; width: 300px; height: 0px">
+          <vaadin-chart></vaadin-chart>
+        </div>`).querySelector('vaadin-chart');
+
+      await nextResize(chart);
+
+      expect(chart.offsetWidth).to.be.equal(300);
+      expect(chart.offsetHeight).to.be.equal(0);
+    });
   });
 
   describe('resize', () => {

packages/charts/test/reattach.test.js

@@ -1,5 +1,5 @@
 import { expect } from '@vaadin/chai-plugins';
-import { fixtureSync, nextFrame, oneEvent } from '@vaadin/testing-helpers';
+import { fixtureSync, nextFrame, nextResize, oneEvent } from '@vaadin/testing-helpers';
 import sinon from 'sinon';
 import './chart-not-animated-styles.js';
 import '../src/vaadin-chart.js';
@@ -262,18 +262,19 @@ describe('reattach', () => {
 
   it('should restore default height when moving from different container with defined height', async () => {
     await oneEvent(chart, 'chart-load');
+    await nextResize(chart);
 
     const initialHeight = getComputedStyle(chart).height;
 
     inner.style.height = '700px';
     inner.appendChild(chart);
-    await nextFrame();
+    await nextResize(chart);
 
     expect(getComputedStyle(chart).height).to.be.equal(inner.style.height);
 
     // Move back to first parent
     wrapper.appendChild(chart);
-    await nextFrame();
+    await nextResize(chart);
 
     expect(getComputedStyle(chart).height).to.be.equal(initialHeight);
   });

packages/charts/test/styling.test.js

@@ -1,5 +1,5 @@
 import { expect } from '@vaadin/chai-plugins';
-import { fixtureSync, oneEvent } from '@vaadin/testing-helpers';
+import { fixtureSync, nextResize, oneEvent } from '@vaadin/testing-helpers';
 import './theme-styles.js';
 import '../src/vaadin-chart.js';
 
@@ -14,6 +14,7 @@ describe('vaadin-chart styling', () => {
         </vaadin-chart>
       `);
       await oneEvent(chart, 'chart-load');
+      await nextResize(chart);
       chartContainer = chart.$.chart;
     });