Skip to content

fix(overlay): make config immutable for existing refs #7376

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

Merged
merged 1 commit into from
Nov 28, 2017
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: 1 addition & 1 deletion src/cdk/overlay/overlay-directives.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
}

this._position.withDirection(this.dir);
this._overlayRef.getConfig().direction = this.dir;
this._overlayRef.setDirection(this.dir);
this._document.addEventListener('keydown', this._escapeListener);

if (!this._overlayRef.hasAttached()) {
Expand Down
85 changes: 57 additions & 28 deletions src/cdk/overlay/overlay-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,20 @@
* found in the LICENSE file at https://angular.io/license
*/

import {NgZone} from '@angular/core';
import {PortalOutlet, Portal} from '@angular/cdk/portal';
import {OverlayConfig} from './overlay-config';
import {OverlayKeyboardDispatcher} from './keyboard/overlay-keyboard-dispatcher';
import {Direction} from '@angular/cdk/bidi';
import {ComponentPortal, Portal, PortalOutlet, TemplatePortal} from '@angular/cdk/portal';
import {ComponentRef, EmbeddedViewRef, NgZone} from '@angular/core';
import {Observable} from 'rxjs/Observable';
import {Subject} from 'rxjs/Subject';
import {take} from 'rxjs/operators/take';
import {Subject} from 'rxjs/Subject';
import {OverlayKeyboardDispatcher} from './keyboard/overlay-keyboard-dispatcher';
import {OverlayConfig} from './overlay-config';


/** An object where all of its properties cannot be written. */
export type ImmutableObject<T> = {
readonly [P in keyof T]: T[P];
};

/**
* Reference to an overlay that has been created with the Overlay service.
Expand All @@ -31,7 +37,7 @@ export class OverlayRef implements PortalOutlet {
constructor(
private _portalOutlet: PortalOutlet,
private _pane: HTMLElement,
private _config: OverlayConfig,
private _config: ImmutableObject<OverlayConfig>,
private _ngZone: NgZone,
private _keyboardDispatcher: OverlayKeyboardDispatcher) {

Expand All @@ -45,8 +51,14 @@ export class OverlayRef implements PortalOutlet {
return this._pane;
}

attach<T>(portal: ComponentPortal<T>): ComponentRef<T>;
attach<T>(portal: TemplatePortal<T>): EmbeddedViewRef<T>;
attach(portal: any): any;

/**
* Attaches the overlay to a portal instance and adds the backdrop.
* Attaches content, given via a Portal, to the overlay.
* If the overlay is configured to have a backdrop, it will be created.
*
* @param portal Portal instance to which to attach the overlay.
* @returns The portal attachment result.
*/
Expand All @@ -59,8 +71,8 @@ export class OverlayRef implements PortalOutlet {

// Update the pane element with the given configuration.
this._updateStackingOrder();
this.updateSize();
this.updateDirection();
this._updateElementSize();
this._updateElementDirection();

if (this._config.scrollStrategy) {
this._config.scrollStrategy.enable();
Expand Down Expand Up @@ -133,9 +145,7 @@ export class OverlayRef implements PortalOutlet {
return detachmentResult;
}

/**
* Cleans up the overlay from the DOM.
*/
/** Cleans up the overlay from the DOM. */
dispose(): void {
const isAttached = this.hasAttached();

Expand All @@ -161,16 +171,12 @@ export class OverlayRef implements PortalOutlet {
this._detachments.complete();
}

/**
* Checks whether the overlay has been attached.
*/
/** Whether the overlay has attached content. */
hasAttached(): boolean {
return this._portalOutlet.hasAttached();
}

/**
* Gets an observable that emits when the backdrop has been clicked.
*/
/** Gets an observable that emits when the backdrop has been clicked. */
backdropClick(): Observable<void> {
return this._backdropClick.asObservable();
}
Expand All @@ -190,9 +196,7 @@ export class OverlayRef implements PortalOutlet {
return this._keydownEvents.asObservable();
}

/**
* Gets the current config of the overlay.
*/
/** Gets the the current overlay configuration, which is immutable. */
getConfig(): OverlayConfig {
return this._config;
}
Expand All @@ -204,13 +208,25 @@ export class OverlayRef implements PortalOutlet {
}
}

/** Update the size properties of the overlay. */
updateSize(sizeConfig: OverlaySizeConfig) {
this._config = {...this._config, ...sizeConfig};
this._updateElementSize();
}

/** Sets the LTR/RTL direction for the overlay. */
setDirection(dir: Direction) {
this._config = {...this._config, direction: dir};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why clone the config here instead of just setting the direction?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the config is immutable

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be enough to clone it on init and then work off the clone?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like having the config as immutable all the time because it reinforces the fact that something extra needs to be done to update it.

this._updateElementDirection();
}

/** Updates the text direction of the overlay panel. */
private updateDirection() {
private _updateElementDirection() {
this._pane.setAttribute('dir', this._config.direction!);
}

/** Updates the size of the overlay based on the overlay config. */
updateSize() {
/** Updates the size of the overlay element based on the overlay config. */
private _updateElementSize() {
if (this._config.width || this._config.width === 0) {
this._pane.style.width = formatCssUnit(this._config.width);
}
Expand Down Expand Up @@ -259,10 +275,12 @@ export class OverlayRef implements PortalOutlet {
this._backdropElement.addEventListener('click', () => this._backdropClick.next(null));

// Add class to fade-in the backdrop after one frame.
requestAnimationFrame(() => {
if (this._backdropElement) {
this._backdropElement.classList.add('cdk-overlay-backdrop-showing');
}
this._ngZone.runOutsideAngular(() => {
requestAnimationFrame(() => {
if (this._backdropElement) {
this._backdropElement.classList.add('cdk-overlay-backdrop-showing');
}
});
});
}

Expand Down Expand Up @@ -323,3 +341,14 @@ export class OverlayRef implements PortalOutlet {
function formatCssUnit(value: number | string) {
return typeof value === 'string' ? value as string : `${value}px`;
}


/** Size properties for an overlay. */
export interface OverlaySizeConfig {
width?: number | string;
height?: number | string;
minWidth?: number | string;
minHeight?: number | string;
maxWidth?: number | string;
maxHeight?: number | string;
}
4 changes: 4 additions & 0 deletions src/cdk/portal/portal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@ export abstract class BasePortalOutlet implements PortalOutlet {
return !!this._attachedPortal;
}

attach<T>(portal: ComponentPortal<T>): ComponentRef<T>;
attach<T>(portal: TemplatePortal<T>): EmbeddedViewRef<T>;
attach(portal: any): any;

/** Attaches a portal. */
attach(portal: Portal<any>): any {
if (!portal) {
Expand Down
3 changes: 1 addition & 2 deletions src/lib/autocomplete/autocomplete-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -458,8 +458,7 @@ export class MatAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
this._overlayRef = this._overlay.create(this._getOverlayConfig());
} else {
/** Update the panel width, in case the host width has changed */
this._overlayRef.getConfig().width = this._getHostWidth();
this._overlayRef.updateSize();
this._overlayRef.updateSize({width: this._getHostWidth()});
}

if (this._overlayRef && !this._overlayRef.hasAttached()) {
Expand Down