From 258b75bd0bbca25e376ed31b0fa53d37a63654b0 Mon Sep 17 00:00:00 2001 From: Sergey Andrievskiy Date: Wed, 21 Aug 2019 15:14:58 +0300 Subject: [PATCH] fix(dynamic overlay): recreate overlay when overlay container change (#1913) --- .../components/cdk/adapter/adapter.module.ts | 2 + .../overlay/dynamic/dynamic-overlay.spec.ts | 43 ++++++++++++++++++- .../cdk/overlay/dynamic/dynamic-overlay.ts | 32 ++++++++++---- 3 files changed, 67 insertions(+), 10 deletions(-) diff --git a/src/framework/theme/components/cdk/adapter/adapter.module.ts b/src/framework/theme/components/cdk/adapter/adapter.module.ts index ea4b808824..ffd41d14d4 100644 --- a/src/framework/theme/components/cdk/adapter/adapter.module.ts +++ b/src/framework/theme/components/cdk/adapter/adapter.module.ts @@ -1,6 +1,7 @@ import { ModuleWithProviders, NgModule } from '@angular/core'; import { OverlayContainer, ScrollDispatcher, ScrollStrategyOptions } from '@angular/cdk/overlay'; +import { NbOverlayContainer } from '../overlay/mapping'; import { NbOverlayContainerAdapter } from './overlay-container-adapter'; import { NbScrollDispatcherAdapter } from './scroll-dispatcher-adapter'; import { NbViewportRulerAdapter } from './viewport-ruler-adapter'; @@ -17,6 +18,7 @@ export class NbCdkAdapterModule { NbOverlayContainerAdapter, NbBlockScrollStrategyAdapter, { provide: OverlayContainer, useExisting: NbOverlayContainerAdapter }, + { provide: NbOverlayContainer, useExisting: NbOverlayContainerAdapter }, { provide: ScrollDispatcher, useClass: NbScrollDispatcherAdapter }, { provide: ScrollStrategyOptions, useClass: NbScrollStrategyOptions }, ], diff --git a/src/framework/theme/components/cdk/overlay/dynamic/dynamic-overlay.spec.ts b/src/framework/theme/components/cdk/overlay/dynamic/dynamic-overlay.spec.ts index 3225760db9..b0058a5889 100644 --- a/src/framework/theme/components/cdk/overlay/dynamic/dynamic-overlay.spec.ts +++ b/src/framework/theme/components/cdk/overlay/dynamic/dynamic-overlay.spec.ts @@ -6,7 +6,7 @@ import { ScrollStrategy } from '@angular/cdk/overlay'; import { NbDynamicOverlay } from './dynamic-overlay'; import { NbOverlayService } from '../overlay-service'; import { NbRenderableContainer } from '../overlay-container'; -import { NbComponentPortal, NbOverlayConfig } from '../mapping'; +import { NbComponentPortal, NbOverlayConfig, NbOverlayContainer } from '../mapping'; @Component({ template: '' }) export class NbDynamicOverlayMockComponent implements NbRenderableContainer { @@ -75,6 +75,12 @@ const scrollStrategies = { reposition: () => ( repositionRes) as ScrollStrategy, }; +export class NbOverlayContainerMock { + getContainerElement() { + return { contains() { return true; } }; + } +} + export class NbOverlayServiceMock { _config: NbOverlayConfig; @@ -114,6 +120,7 @@ describe('dynamic-overlay', () => { NbDynamicOverlay, { provide: NbOverlayService, useClass: NbOverlayServiceMock }, { provide: NgZone, useClass: MockNgZone }, + { provide: NbOverlayContainer, useClass: NbOverlayContainerMock }, ], }); overlayService = bed.get(NbOverlayService); @@ -199,7 +206,7 @@ describe('dynamic-overlay', () => { expect(createOverlaySpy).toHaveBeenCalledTimes(1); }); - it('should not attache to ref if already shown', () => { + it('should not attach to ref if already shown', () => { const attachSpy = spyOn(ref, 'attach').and.callThrough(); const hasAttacheSpy = spyOn(ref, 'hasAttached'); @@ -375,4 +382,36 @@ describe('dynamic-overlay', () => { expect(updatePositionSpy).toHaveBeenCalledTimes(1); }); + it(`should recreate overlay if it's host isn't child of overlay container`, () => { + dynamicOverlay.show(); + dynamicOverlay.hide(); + + const overlayContainer = TestBed.get(NbOverlayContainer); + const getContainerElementSpy = spyOn(overlayContainer, 'getContainerElement').and.returnValues( + { contains() { return false; } }, + { contains() { return true; } }, + ); + + dynamicOverlay.show(); + + expect(getContainerElementSpy).toHaveBeenCalledTimes(2); + }); + + it(`should dispose overlay ref when recreating overlay`, () => { + const disposeSpy = spyOn(ref, 'dispose').and.callThrough(); + + dynamicOverlay.show(); + dynamicOverlay.hide(); + + const overlayContainer = TestBed.get(NbOverlayContainer); + // return false once to force overlay ref recreation and then always return true + overlayContainer.getContainerElement = () => { + overlayContainer.getContainerElement = () => ({ contains: () => true }); + return { contains: () => false }; + }; + + dynamicOverlay.show(); + + expect(disposeSpy).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/framework/theme/components/cdk/overlay/dynamic/dynamic-overlay.ts b/src/framework/theme/components/cdk/overlay/dynamic/dynamic-overlay.ts index 4c4462431a..0b9ed44462 100644 --- a/src/framework/theme/components/cdk/overlay/dynamic/dynamic-overlay.ts +++ b/src/framework/theme/components/cdk/overlay/dynamic/dynamic-overlay.ts @@ -9,7 +9,7 @@ import { import { NbRenderableContainer } from '../overlay-container'; import { createContainer, NbOverlayContent, NbOverlayService, patch } from '../overlay-service'; -import { NbOverlayRef } from '../mapping'; +import { NbOverlayRef, NbOverlayContainer } from '../mapping'; export interface NbDynamicOverlayController { show(); @@ -36,9 +36,10 @@ export class NbDynamicOverlay { } constructor( - private overlay: NbOverlayService, - private componentFactoryResolver: ComponentFactoryResolver, - private zone: NgZone) { + protected overlay: NbOverlayService, + protected componentFactoryResolver: ComponentFactoryResolver, + protected zone: NgZone, + protected overlayContainer: NbOverlayContainer) { } create(componentType: Type, @@ -113,6 +114,12 @@ export class NbDynamicOverlay { } this.renderContainer(); + + if (!this.hasOverlayInContainer()) { + // Dispose overlay ref as it refers to the old overlay container and create new by calling `show` + this.disposeOverlayRef(); + return this.show(); + } } hide() { @@ -135,10 +142,7 @@ export class NbDynamicOverlay { dispose() { this.alive = false; this.hide(); - if (this.ref) { - this.ref.dispose(); - this.ref = null; - } + this.disposeOverlayRef(); } getContainer() { @@ -187,4 +191,16 @@ export class NbDynamicOverlay { this.ref && this.ref.updatePosition(); }); } + + protected hasOverlayInContainer(): boolean { + return this.overlayContainer.getContainerElement().contains(this.ref.hostElement); + } + + protected disposeOverlayRef() { + if (this.ref) { + this.ref.dispose(); + this.ref = null; + this.container = null; + } + } }