fix: do not reset configuration if chart is immediately re-attached (#…

…8117)
vaadin · Nov 8, 2024 · c673dc2 · c673dc2
1 parent 49f3691
commit c673dc2
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 8 deletions.
diff --git a/packages/charts/src/vaadin-chart.js b/packages/charts/src/vaadin-chart.js
@@ -1078,14 +1078,20 @@ class Chart extends ResizeMixin(ElementMixin(ThemableMixin(PolymerElement))) {
   disconnectedCallback() {
     super.disconnectedCallback();
 
-    if (this.configuration) {
-      this.configuration.destroy();
-      this.configuration = undefined;
-    }
+    queueMicrotask(() => {
+      if (this.isConnected) {
+        return;
+      }
 
-    if (this._childObserver) {
-      this._childObserver.disconnect();
-    }
+      if (this.configuration) {
+        this.configuration.destroy();
+        this.configuration = undefined;
+      }
+
+      if (this._childObserver) {
+        this._childObserver.disconnect();
+      }
+    });
   }
 
   /** @private */

diff --git a/packages/charts/test/reattach.test.js b/packages/charts/test/reattach.test.js
@@ -1,5 +1,5 @@
 import { expect } from '@vaadin/chai-plugins';
-import { fixtureSync, oneEvent } from '@vaadin/testing-helpers';
+import { fixtureSync, nextFrame, oneEvent } from '@vaadin/testing-helpers';
 import sinon from 'sinon';
 import '../vaadin-chart.js';
 
@@ -25,24 +25,42 @@ describe('reattach', () => {
 
     const spy = sinon.spy(chart.configuration, 'destroy');
     wrapper.removeChild(chart);
+    await nextFrame();
+
     expect(spy).to.be.calledOnce;
     expect(chart.configuration).to.be.undefined;
   });
 
   it('should re-create chart configuration when attached to a new parent', async () => {
     await oneEvent(chart, 'chart-load');
     wrapper.removeChild(chart);
+    await nextFrame();
 
     inner.appendChild(chart);
     await oneEvent(chart, 'chart-load');
     expect(chart.configuration.series.length).to.be.equal(chart.childElementCount);
   });
 
+  it('should not re-create chart configuration when moved to a new parent', async () => {
+    await oneEvent(chart, 'chart-load');
+
+    const configuration = chart.configuration;
+
+    const spy = sinon.spy(chart.configuration, 'destroy');
+    inner.appendChild(chart);
+    await nextFrame();
+
+    expect(spy).to.not.be.called;
+    expect(chart.configuration).to.be.equal(configuration);
+  });
+
   it('should apply title updated while detached after reattach', async () => {
     chart.title = 'Title';
     await oneEvent(chart, 'chart-load');
 
     wrapper.removeChild(chart);
+    await nextFrame();
+
     chart.updateConfiguration({ title: { text: 'New title' } });
 
     inner.appendChild(chart);
@@ -55,6 +73,8 @@ describe('reattach', () => {
     await oneEvent(chart, 'chart-load');
 
     wrapper.removeChild(chart);
+    await nextFrame();
+
     chart.updateConfiguration({ subtitle: { text: 'New subtitle' } });
 
     inner.appendChild(chart);
@@ -67,6 +87,8 @@ describe('reattach', () => {
     await oneEvent(chart, 'chart-load');
 
     wrapper.removeChild(chart);
+    await nextFrame();
+
     chart.updateConfiguration({ chart: { type: 'line' } });
 
     inner.appendChild(chart);
@@ -79,6 +101,8 @@ describe('reattach', () => {
     await oneEvent(chart, 'chart-load');
 
     wrapper.removeChild(chart);
+    await nextFrame();
+
     chart.updateConfiguration({ tooltip: { enabled: false } });
 
     inner.appendChild(chart);
@@ -91,6 +115,8 @@ describe('reattach', () => {
     await oneEvent(chart, 'chart-load');
 
     wrapper.removeChild(chart);
+    await nextFrame();
+
     chart.updateConfiguration({ legend: { enabled: true } });
 
     inner.appendChild(chart);
@@ -103,6 +129,8 @@ describe('reattach', () => {
     await oneEvent(chart, 'chart-load');
 
     wrapper.removeChild(chart);
+    await nextFrame();
+
     chart.updateConfiguration({ chart: { options3d: { enabled: false } } });
 
     inner.appendChild(chart);
@@ -115,6 +143,8 @@ describe('reattach', () => {
     await oneEvent(chart, 'chart-load');
 
     wrapper.removeChild(chart);
+    await nextFrame();
+
     chart.updateConfiguration({ chart: { polar: false } });
 
     inner.appendChild(chart);
@@ -128,6 +158,8 @@ describe('reattach', () => {
     await oneEvent(chart, 'chart-load');
 
     wrapper.removeChild(chart);
+    await nextFrame();
+
     chart.updateConfiguration({ lang: { noData: 'Nothing' } });
 
     inner.appendChild(chart);
@@ -147,6 +179,7 @@ describe('reattach', () => {
         series: { stacking: 'percent' },
       },
     });
+    await nextFrame();
 
     inner.appendChild(chart);
     await oneEvent(chart, 'chart-load');
@@ -158,6 +191,8 @@ describe('reattach', () => {
     await oneEvent(chart, 'chart-load');
 
     wrapper.removeChild(chart);
+    await nextFrame();
+
     chart.updateConfiguration({ xAxis: { categories: ['Jun', 'Jul', 'Aug'] } });
 
     inner.appendChild(chart);
@@ -173,6 +208,8 @@ describe('reattach', () => {
     await oneEvent(chart, 'chart-load');
 
     wrapper.removeChild(chart);
+    await nextFrame();
+
     chart.updateConfiguration({ xAxis: { min: 1 } });
 
     inner.appendChild(chart);
@@ -185,6 +222,8 @@ describe('reattach', () => {
     await oneEvent(chart, 'chart-load');
 
     wrapper.removeChild(chart);
+    await nextFrame();
+
     chart.updateConfiguration({ xAxis: { max: 3 } });
 
     inner.appendChild(chart);
@@ -197,6 +236,8 @@ describe('reattach', () => {
     await oneEvent(chart, 'chart-load');
 
     wrapper.removeChild(chart);
+    await nextFrame();
+
     chart.updateConfiguration({ chart: { inverted: false } });
 
     inner.appendChild(chart);