Merge pull request #45 from whythecode/fix/41-resize-must-be-passed-a…

…-displayed-plot-div-element fixes #41: added missing plotly instance cleanup
plotly · Feb 19, 2019 · 3f6b46d · 3f6b46d
2 parents 67d8355 + 2c18d0e
commit 3f6b46d
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 6 deletions.
diff --git a/src/app/shared/plot/plot.component.spec.ts b/src/app/shared/plot/plot.component.spec.ts
@@ -8,25 +8,20 @@ import * as PlotlyJS from 'plotly.js/dist/plotly';
 
 PlotlyService.setPlotly(PlotlyJS);
 
-
 describe('PlotComponent', () => {
     let component: PlotComponent;
     let fixture: ComponentFixture<PlotComponent>;
-    let plotlySpy: jasmine.SpyObj<PlotlyService>;
     let windowSpy: jasmine.SpyObj<Window>;
 
     beforeEach(async(() => {
-        const pSpy = jasmine.createSpyObj('PlotlyService', ['newPlot', 'plot']);
         windowSpy = jasmine.createSpyObj('Window', ['addEventListener', 'removeEventListener']);
 
         TestBed.configureTestingModule({
             declarations: [PlotComponent],
             providers: [
-                { provide: PlotlyService, useValue: pSpy },
+                PlotlyService,
             ],
         }).compileComponents();
-
-        plotlySpy = TestBed.get(PlotlyService);
     }));
 
     beforeEach(() => {
@@ -160,4 +155,61 @@ describe('PlotComponent', () => {
         component.updateWindowResizeHandler();
         expect(component.resizeHandler).toBeUndefined();
     });
+
+    it('should clear all added window events on destroy', async (done) => {
+
+        spyOn(component, 'ngOnDestroy').and.callThrough();
+
+        const windowListenerCount = (<any>window).eventListeners().length;
+
+        // make component responsive via both the lib and the component (at least 2 window events are added)
+        component.layout = { title: 'responsive', autosize: true };
+        component.config = { responsive: true };
+        component.useResizeHandler = true;
+
+        await component.createPlot();
+        await fixture.whenStable();
+
+        expect(component.ngOnDestroy).not.toHaveBeenCalled();
+
+        fixture.destroy();
+        await fixture.whenStable();
+
+        expect(component.ngOnDestroy).toHaveBeenCalled();
+
+        // amount of listeners should be the same as before initializing the component
+        expect((<any>window).eventListeners().length).toEqual(windowListenerCount);
+
+        done();
+    });
+
+    it('should not cause errors if window is resized after a responsive chart is destroyed', async (done) => {
+
+        // make component responsive via both the lib and the component
+        component.layout = { title: 'responsive', autosize: true };
+        component.config = { responsive: true };
+        component.useResizeHandler = true;
+
+        await component.createPlot();
+        await fixture.whenStable();
+
+        spyOn(PlotlyJS.Plots, 'resize').and.callThrough();
+
+        window.dispatchEvent(new Event('resize'));
+        await fixture.whenStable();
+
+        // responsive:true and useResizeHandler:true both cause .resize() to be called
+        expect(PlotlyJS.Plots.resize).toHaveBeenCalledTimes(2);
+        PlotlyJS.Plots.resize.calls.reset();
+
+        fixture.destroy();
+        await fixture.whenStable();
+
+        window.dispatchEvent(new Event('resize'));
+        await fixture.whenStable();
+
+        expect(PlotlyJS.Plots.resize).not.toHaveBeenCalled();
+
+        done();
+    });
 });
diff --git a/src/app/shared/plotly.service.ts b/src/app/shared/plotly.service.ts
@@ -25,6 +25,7 @@ export class PlotlyService {
         const index = PlotlyService.instances.indexOf(div);
         if (index >= 0) {
             PlotlyService.instances.splice(index, 1);
+            PlotlyService._plotly.purge(div);
         }
     }