From d48b1baa3d4dd337ac74de8260daeefa526af5ba Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 25 Sep 2018 16:13:22 +0200 Subject: [PATCH] feat(overlay): add option to automatically dispose on navigation (#12592) Adds the opt-in `disposeOnNavigation` option which will dispose of an overlay when the user goes back/forward in history. This is something that we had in `MatDialog` and `MatBottomSheet` already, but it's a common-enough case, especially for global overlays, that it makes sense to have it in the CDK. Fixes #12544. --- src/cdk/overlay/overlay-config.ts | 7 ++++ src/cdk/overlay/overlay-ref.ts | 18 ++++++++-- src/cdk/overlay/overlay.spec.ts | 40 +++++++++++++++------ src/cdk/overlay/overlay.ts | 9 +++-- src/lib/bottom-sheet/bottom-sheet-config.ts | 6 +++- src/lib/bottom-sheet/bottom-sheet-ref.ts | 17 ++------- src/lib/bottom-sheet/bottom-sheet.ts | 1 + src/lib/dialog/dialog-config.ts | 6 +++- src/lib/dialog/dialog-ref.ts | 20 ++--------- src/lib/dialog/dialog.ts | 3 +- 10 files changed, 78 insertions(+), 49 deletions(-) diff --git a/src/cdk/overlay/overlay-config.ts b/src/cdk/overlay/overlay-config.ts index 133000610ec..aa19e1958c2 100644 --- a/src/cdk/overlay/overlay-config.ts +++ b/src/cdk/overlay/overlay-config.ts @@ -53,6 +53,13 @@ export class OverlayConfig { */ direction?: Direction | Directionality; + /** + * Whether the overlay should be disposed of when the user goes backwards/forwards in history. + * Note that this usually doesn't include clicking on links (unless the user is using + * the `HashLocationStrategy`). + */ + disposeOnNavigation?: boolean = false; + constructor(config?: OverlayConfig) { if (config) { Object.keys(config) diff --git a/src/cdk/overlay/overlay-ref.ts b/src/cdk/overlay/overlay-ref.ts index 009917c9dc9..ebeb9a535d8 100644 --- a/src/cdk/overlay/overlay-ref.ts +++ b/src/cdk/overlay/overlay-ref.ts @@ -9,7 +9,8 @@ import {Direction, Directionality} from '@angular/cdk/bidi'; import {ComponentPortal, Portal, PortalOutlet, TemplatePortal} from '@angular/cdk/portal'; import {ComponentRef, EmbeddedViewRef, NgZone} from '@angular/core'; -import {Observable, Subject, merge} from 'rxjs'; +import {Location} from '@angular/common'; +import {Observable, Subject, merge, SubscriptionLike, Subscription} from 'rxjs'; import {take, takeUntil} from 'rxjs/operators'; import {OverlayKeyboardDispatcher} from './keyboard/overlay-keyboard-dispatcher'; import {OverlayConfig} from './overlay-config'; @@ -33,6 +34,7 @@ export class OverlayRef implements PortalOutlet, OverlayReference { private _attachments = new Subject(); private _detachments = new Subject(); private _positionStrategy: PositionStrategy | undefined; + private _locationChanges: SubscriptionLike = Subscription.EMPTY; /** * Reference to the parent of the `_host` at the time it was detached. Used to restore @@ -63,7 +65,9 @@ export class OverlayRef implements PortalOutlet, OverlayReference { private _config: ImmutableObject, private _ngZone: NgZone, private _keyboardDispatcher: OverlayKeyboardDispatcher, - private _document: Document) { + private _document: Document, + // @breaking-change 8.0.0 `_location` parameter to be made required. + private _location?: Location) { if (_config.scrollStrategy) { _config.scrollStrategy.attach(this); @@ -152,6 +156,12 @@ export class OverlayRef implements PortalOutlet, OverlayReference { // Track this overlay by the keyboard dispatcher this._keyboardDispatcher.add(this); + // @breaking-change 8.0.0 remove the null check for `_location` + // once the constructor parameter is made required. + if (this._config.disposeOnNavigation && this._location) { + this._locationChanges = this._location.subscribe(() => this.dispose()); + } + return attachResult; } @@ -195,6 +205,9 @@ export class OverlayRef implements PortalOutlet, OverlayReference { // rendered, even though it's transparent and unclickable which is why we remove it. this._detachContentWhenStable(); + // Stop listening for location changes. + this._locationChanges.unsubscribe(); + return detachmentResult; } @@ -211,6 +224,7 @@ export class OverlayRef implements PortalOutlet, OverlayReference { } this.detachBackdrop(); + this._locationChanges.unsubscribe(); this._keyboardDispatcher.remove(this); this._portalOutlet.dispose(); this._attachments.complete(); diff --git a/src/cdk/overlay/overlay.spec.ts b/src/cdk/overlay/overlay.spec.ts index 0e42e4cd861..71a968b2d8f 100644 --- a/src/cdk/overlay/overlay.spec.ts +++ b/src/cdk/overlay/overlay.spec.ts @@ -17,6 +17,8 @@ import { TemplatePortal, CdkPortal } from '@angular/cdk/portal'; +import {Location} from '@angular/common'; +import {SpyLocation} from '@angular/common/testing'; import { Overlay, OverlayContainer, @@ -38,6 +40,7 @@ describe('Overlay', () => { let viewContainerFixture: ComponentFixture; let dir: Direction; let zone: MockNgZone; + let mockLocation: SpyLocation; beforeEach(async(() => { dir = 'ltr'; @@ -56,21 +59,27 @@ describe('Overlay', () => { provide: NgZone, useFactory: () => zone = new MockNgZone() }, + { + provide: Location, + useClass: SpyLocation + }, ], }).compileComponents(); })); - beforeEach(inject([Overlay, OverlayContainer], (o: Overlay, oc: OverlayContainer) => { - overlay = o; - overlayContainer = oc; - overlayContainerElement = oc.getContainerElement(); + beforeEach(inject([Overlay, OverlayContainer, Location], + (o: Overlay, oc: OverlayContainer, l: Location) => { + overlay = o; + overlayContainer = oc; + overlayContainerElement = oc.getContainerElement(); - let fixture = TestBed.createComponent(TestComponentWithTemplatePortals); - fixture.detectChanges(); - templatePortal = fixture.componentInstance.templatePortal; - componentPortal = new ComponentPortal(PizzaMsg, fixture.componentInstance.viewContainerRef); - viewContainerFixture = fixture; - })); + const fixture = TestBed.createComponent(TestComponentWithTemplatePortals); + fixture.detectChanges(); + templatePortal = fixture.componentInstance.templatePortal; + componentPortal = new ComponentPortal(PizzaMsg, fixture.componentInstance.viewContainerRef); + viewContainerFixture = fixture; + mockLocation = l as SpyLocation; + })); afterEach(() => { overlayContainer.ngOnDestroy(); @@ -378,6 +387,17 @@ describe('Overlay', () => { .toBeTruthy('Expected host element to be back in the DOM.'); }); + it('should be able to dispose an overlay on navigation', () => { + const overlayRef = overlay.create({disposeOnNavigation: true}); + overlayRef.attach(componentPortal); + + expect(overlayContainerElement.textContent).toContain('Pizza'); + + mockLocation.simulateUrlPop(''); + expect(overlayContainerElement.childNodes.length).toBe(0); + expect(overlayContainerElement.textContent).toBe(''); + }); + describe('positioning', () => { let config: OverlayConfig; diff --git a/src/cdk/overlay/overlay.ts b/src/cdk/overlay/overlay.ts index ec3e882ecec..04a6c26eaf0 100644 --- a/src/cdk/overlay/overlay.ts +++ b/src/cdk/overlay/overlay.ts @@ -8,7 +8,7 @@ import {Directionality} from '@angular/cdk/bidi'; import {DomPortalOutlet} from '@angular/cdk/portal'; -import {DOCUMENT} from '@angular/common'; +import {DOCUMENT, Location} from '@angular/common'; import { ApplicationRef, ComponentFactoryResolver, @@ -16,6 +16,7 @@ import { Injectable, Injector, NgZone, + Optional, } from '@angular/core'; import {OverlayKeyboardDispatcher} from './keyboard/overlay-keyboard-dispatcher'; import {OverlayConfig} from './overlay-config'; @@ -53,7 +54,9 @@ export class Overlay { private _injector: Injector, private _ngZone: NgZone, @Inject(DOCUMENT) private _document: any, - private _directionality: Directionality) { } + private _directionality: Directionality, + // @breaking-change 8.0.0 `_location` parameter to be made required. + @Optional() private _location?: Location) { } /** * Creates an overlay. @@ -69,7 +72,7 @@ export class Overlay { overlayConfig.direction = overlayConfig.direction || this._directionality.value; return new OverlayRef(portalOutlet, host, pane, overlayConfig, this._ngZone, - this._keyboardDispatcher, this._document); + this._keyboardDispatcher, this._document, this._location); } /** diff --git a/src/lib/bottom-sheet/bottom-sheet-config.ts b/src/lib/bottom-sheet/bottom-sheet-config.ts index 34d73ae003b..3bb9d9ea2cb 100644 --- a/src/lib/bottom-sheet/bottom-sheet-config.ts +++ b/src/lib/bottom-sheet/bottom-sheet-config.ts @@ -40,7 +40,11 @@ export class MatBottomSheetConfig { /** Aria label to assign to the bottom sheet element. */ ariaLabel?: string | null = null; - /** Whether the bottom sheet should close when the user goes backwards/forwards in history. */ + /** + * Whether the bottom sheet should close when the user goes backwards/forwards in history. + * Note that this usually doesn't include clicking on links (unless the user is using + * the `HashLocationStrategy`). + */ closeOnNavigation?: boolean = true; /** Whether the bottom sheet should focus the first focusable element on open. */ diff --git a/src/lib/bottom-sheet/bottom-sheet-ref.ts b/src/lib/bottom-sheet/bottom-sheet-ref.ts index 7e718394aeb..626f94a0f91 100644 --- a/src/lib/bottom-sheet/bottom-sheet-ref.ts +++ b/src/lib/bottom-sheet/bottom-sheet-ref.ts @@ -9,7 +9,7 @@ import {Location} from '@angular/common'; import {ESCAPE} from '@angular/cdk/keycodes'; import {OverlayRef} from '@angular/cdk/overlay'; -import {merge, Observable, Subject, SubscriptionLike, Subscription} from 'rxjs'; +import {merge, Observable, Subject} from 'rxjs'; import {filter, take} from 'rxjs/operators'; import {MatBottomSheetContainer} from './bottom-sheet-container'; @@ -36,13 +36,11 @@ export class MatBottomSheetRef { /** Result to be passed down to the `afterDismissed` stream. */ private _result: R | undefined; - /** Subscription to changes in the user's location. */ - private _locationChanges: SubscriptionLike = Subscription.EMPTY; - constructor( containerInstance: MatBottomSheetContainer, private _overlayRef: OverlayRef, - location?: Location) { + // @breaking-change 8.0.0 `_location` parameter to be removed. + _location?: Location) { this.containerInstance = containerInstance; // Emit when opening animation completes @@ -61,7 +59,6 @@ export class MatBottomSheetRef { take(1) ) .subscribe(() => { - this._locationChanges.unsubscribe(); this._overlayRef.dispose(); this._afterDismissed.next(this._result); this._afterDismissed.complete(); @@ -73,14 +70,6 @@ export class MatBottomSheetRef { _overlayRef.keydownEvents().pipe(filter(event => event.keyCode === ESCAPE)) ).subscribe(() => this.dismiss()); } - - if (location) { - this._locationChanges = location.subscribe(() => { - if (containerInstance.bottomSheetConfig.closeOnNavigation) { - this.dismiss(); - } - }); - } } /** diff --git a/src/lib/bottom-sheet/bottom-sheet.ts b/src/lib/bottom-sheet/bottom-sheet.ts index 8c6fc9f354f..9222711e830 100644 --- a/src/lib/bottom-sheet/bottom-sheet.ts +++ b/src/lib/bottom-sheet/bottom-sheet.ts @@ -127,6 +127,7 @@ export class MatBottomSheet { const overlayConfig = new OverlayConfig({ direction: config.direction, hasBackdrop: config.hasBackdrop, + disposeOnNavigation: config.closeOnNavigation, maxWidth: '100%', scrollStrategy: this._overlay.scrollStrategies.block(), positionStrategy: this._overlay.position() diff --git a/src/lib/dialog/dialog-config.ts b/src/lib/dialog/dialog-config.ts index 770a26a7d5b..add3efbe906 100644 --- a/src/lib/dialog/dialog-config.ts +++ b/src/lib/dialog/dialog-config.ts @@ -104,7 +104,11 @@ export class MatDialogConfig { /** Scroll strategy to be used for the dialog. */ scrollStrategy?: ScrollStrategy; - /** Whether the dialog should close when the user goes backwards/forwards in history. */ + /** + * Whether the dialog should close when the user goes backwards/forwards in history. + * Note that this usually doesn't include clicking on links (unless the user is using + * the `HashLocationStrategy`). + */ closeOnNavigation?: boolean = true; // TODO(jelbourn): add configuration for lifecycle hooks, ARIA labelling. diff --git a/src/lib/dialog/dialog-ref.ts b/src/lib/dialog/dialog-ref.ts index d647a5fde5b..98af86092b9 100644 --- a/src/lib/dialog/dialog-ref.ts +++ b/src/lib/dialog/dialog-ref.ts @@ -9,7 +9,7 @@ import {ESCAPE} from '@angular/cdk/keycodes'; import {GlobalPositionStrategy, OverlayRef} from '@angular/cdk/overlay'; import {Location} from '@angular/common'; -import {Observable, Subject, Subscription, SubscriptionLike} from 'rxjs'; +import {Observable, Subject} from 'rxjs'; import {filter, take} from 'rxjs/operators'; import {DialogPosition} from './dialog-config'; import {MatDialogContainer} from './dialog-container'; @@ -42,13 +42,11 @@ export class MatDialogRef { /** Result to be passed to afterClosed. */ private _result: R | undefined; - /** Subscription to changes in the user's location. */ - private _locationChanges: SubscriptionLike = Subscription.EMPTY; - constructor( private _overlayRef: OverlayRef, public _containerInstance: MatDialogContainer, - location?: Location, + // @breaking-change 8.0.0 `_location` parameter to be removed. + _location?: Location, readonly id: string = `mat-dialog-${uniqueId++}`) { // Pass the id along to the container. @@ -73,7 +71,6 @@ export class MatDialogRef { _overlayRef.detachments().subscribe(() => { this._beforeClosed.next(this._result); this._beforeClosed.complete(); - this._locationChanges.unsubscribe(); this._afterClosed.next(this._result); this._afterClosed.complete(); this.componentInstance = null!; @@ -83,17 +80,6 @@ export class MatDialogRef { _overlayRef.keydownEvents() .pipe(filter(event => event.keyCode === ESCAPE && !this.disableClose)) .subscribe(() => this.close()); - - if (location) { - // Close the dialog when the user goes forwards/backwards in history or when the location - // hash changes. Note that this usually doesn't include clicking on links (unless the user - // is using the `HashLocationStrategy`). - this._locationChanges = location.subscribe(() => { - if (this._containerInstance._config.closeOnNavigation) { - this.close(); - } - }); - } } /** diff --git a/src/lib/dialog/dialog.ts b/src/lib/dialog/dialog.ts index 0b35506df9a..61942d5a965 100644 --- a/src/lib/dialog/dialog.ts +++ b/src/lib/dialog/dialog.ts @@ -195,7 +195,8 @@ export class MatDialog implements OnDestroy { minWidth: dialogConfig.minWidth, minHeight: dialogConfig.minHeight, maxWidth: dialogConfig.maxWidth, - maxHeight: dialogConfig.maxHeight + maxHeight: dialogConfig.maxHeight, + disposeOnNavigation: dialogConfig.closeOnNavigation }); if (dialogConfig.backdropClass) {