Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(dynamic overlay): recreate overlay when overlay container change #1913

Merged
merged 5 commits into from
Aug 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/framework/theme/components/cdk/adapter/adapter.module.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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 },
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -75,6 +75,12 @@ const scrollStrategies = {
reposition: () => (<unknown> repositionRes) as ScrollStrategy,
};

export class NbOverlayContainerMock {
getContainerElement() {
return { contains() { return true; } };
}
}

export class NbOverlayServiceMock {
_config: NbOverlayConfig;

Expand Down Expand Up @@ -114,6 +120,7 @@ describe('dynamic-overlay', () => {
NbDynamicOverlay,
{ provide: NbOverlayService, useClass: NbOverlayServiceMock },
{ provide: NgZone, useClass: MockNgZone },
{ provide: NbOverlayContainer, useClass: NbOverlayContainerMock },
],
});
overlayService = bed.get(NbOverlayService);
Expand Down Expand Up @@ -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');

Expand Down Expand Up @@ -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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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<NbRenderableContainer>,
Expand Down Expand Up @@ -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() {
Expand All @@ -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() {
Expand Down Expand Up @@ -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;
}
}
}