Skip to content

refactor(overlay): allow for object to be passed to the OverlayState constructor #6615

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 2 commits into from
Sep 1, 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
13 changes: 6 additions & 7 deletions src/cdk/overlay/overlay-directives.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,12 @@ export class ConnectedOverlayDirective implements OnDestroy, OnChanges {

/** Builds the overlay config based on the directive's inputs */
private _buildConfig(): OverlayState {
let overlayConfig = new OverlayState();
const positionStrategy = this._position = this._createPositionStrategy();
const overlayConfig = new OverlayState({
Copy link
Member

Choose a reason for hiding this comment

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

Since all of the options are optional now, could you just change most places to, e.g.,

const overlayConfig = {
  positionStrategy,
  scrollStrategy: this.scrollStrategy,
  hasBackdrop: this.hasBackdrop,
};

?

Copy link
Member Author

@crisbeto crisbeto Aug 24, 2017

Choose a reason for hiding this comment

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

That's what I did. The position in this particular case is different, because we want to save a reference to it before we pass it to the OverlayRef.

Copy link
Member

Choose a reason for hiding this comment

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

I meant omitting the new OverlayState( part and just having the object literal

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, gotcha. Instantiating the class lets us apply the default options for things like the direction, backdrop class and scroll strategy.

positionStrategy,
scrollStrategy: this.scrollStrategy,
hasBackdrop: this.hasBackdrop
});

if (this.width || this.width === 0) {
overlayConfig.width = this.width;
Expand All @@ -290,16 +295,10 @@ export class ConnectedOverlayDirective implements OnDestroy, OnChanges {
overlayConfig.minHeight = this.minHeight;
}

overlayConfig.hasBackdrop = this.hasBackdrop;

if (this.backdropClass) {
overlayConfig.backdropClass = this.backdropClass;
}

this._position = this._createPositionStrategy() as ConnectedPositionStrategy;
overlayConfig.positionStrategy = this._position;
overlayConfig.scrollStrategy = this.scrollStrategy;

return overlayConfig;
}

Expand Down
19 changes: 15 additions & 4 deletions src/cdk/overlay/overlay-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ export class OverlayRef implements PortalHost {
private _state: OverlayState,
private _ngZone: NgZone) {

_state.scrollStrategy.attach(this);
if (_state.scrollStrategy) {
_state.scrollStrategy.attach(this);
}
}

/** The overlay's HTML element */
Expand All @@ -54,7 +56,10 @@ export class OverlayRef implements PortalHost {
this.updateSize();
this.updateDirection();
this.updatePosition();
this._state.scrollStrategy.enable();

if (this._state.scrollStrategy) {
this._state.scrollStrategy.enable();
}

// Enable pointer events for the overlay pane element.
this._togglePointerEvents(true);
Expand Down Expand Up @@ -89,7 +94,10 @@ export class OverlayRef implements PortalHost {
// This is necessary because otherwise the pane element will cover the page and disable
// pointer events therefore. Depends on the position strategy and the applied pane boundaries.
this._togglePointerEvents(false);
this._state.scrollStrategy.disable();

if (this._state.scrollStrategy) {
this._state.scrollStrategy.disable();
}

let detachmentResult = this._portalHost.detach();

Expand All @@ -107,7 +115,10 @@ export class OverlayRef implements PortalHost {
this._state.positionStrategy.dispose();
}

this._state.scrollStrategy.disable();
if (this._state.scrollStrategy) {
this._state.scrollStrategy.disable();
}

this.detachBackdrop();
this._portalHost.dispose();
this._attachments.complete();
Expand Down
10 changes: 8 additions & 2 deletions src/cdk/overlay/overlay-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ import {NoopScrollStrategy} from './scroll/noop-scroll-strategy';
*/
export class OverlayState {
/** Strategy with which to position the overlay. */
positionStrategy: PositionStrategy;
positionStrategy?: PositionStrategy;

/** Strategy to be used when handling scroll events while the overlay is open. */
scrollStrategy: ScrollStrategy = new NoopScrollStrategy();
scrollStrategy?: ScrollStrategy = new NoopScrollStrategy();

/** Custom class to add to the overlay pane. */
panelClass?: string | string[] = '';
Expand Down Expand Up @@ -53,6 +53,12 @@ export class OverlayState {
/** The direction of the text in the overlay panel. */
direction?: Direction = 'ltr';

constructor(state?: OverlayState) {
if (state) {
Copy link
Member

Choose a reason for hiding this comment

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

Could you do this?

extendObject(this, state);

(from core/util, handles null / undefined)

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 didn't use extendObject, because it would introduce a dependency between Material and the CDK.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I forgot that was in material.

Object.keys(state).forEach(key => this[key] = state[key]);
}
}

// TODO(jelbourn): configuration still to add
// - focus trap
// - disable pointer events
Expand Down
18 changes: 6 additions & 12 deletions src/cdk/overlay/overlay.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,7 @@ describe('Overlay', () => {
});

it('should set the direction', () => {
const state = new OverlayState();
state.direction = 'rtl';
const state = new OverlayState({direction: 'rtl'});

overlay.create(state).attach(componentPortal);

Expand All @@ -153,10 +152,7 @@ describe('Overlay', () => {
});

it('should emit the attachment event after everything is added to the DOM', () => {
let state = new OverlayState();

state.hasBackdrop = true;

let state = new OverlayState({hasBackdrop: true});
let overlayRef = overlay.create(state);

overlayRef.attachments().subscribe(() => {
Expand Down Expand Up @@ -415,9 +411,8 @@ describe('Overlay', () => {

describe('panelClass', () => {
it('should apply a custom overlay pane class', () => {
const config = new OverlayState();
const config = new OverlayState({panelClass: 'custom-panel-class'});

config.panelClass = 'custom-panel-class';
overlay.create(config).attach(componentPortal);
viewContainerFixture.detectChanges();

Expand All @@ -426,9 +421,8 @@ describe('Overlay', () => {
});

it('should be able to apply multiple classes', () => {
const config = new OverlayState();
const config = new OverlayState({panelClass: ['custom-class-one', 'custom-class-two']});

config.panelClass = ['custom-class-one', 'custom-class-two'];
overlay.create(config).attach(componentPortal);
viewContainerFixture.detectChanges();

Expand All @@ -445,8 +439,8 @@ describe('Overlay', () => {
let overlayRef: OverlayRef;

beforeEach(() => {
config = new OverlayState();
fakeScrollStrategy = config.scrollStrategy = new FakeScrollStrategy();
fakeScrollStrategy = new FakeScrollStrategy();
config = new OverlayState({scrollStrategy: fakeScrollStrategy});
overlayRef = overlay.create(config);
});

Expand Down
3 changes: 1 addition & 2 deletions src/cdk/overlay/scroll/block-scroll-strategy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,8 @@ describe('BlockScrollStrategy', () => {
}));

beforeEach(inject([Overlay, ViewportRuler], (overlay: Overlay, viewportRuler: ViewportRuler) => {
let overlayState = new OverlayState();
let overlayState = new OverlayState({scrollStrategy: overlay.scrollStrategies.block()});

overlayState.scrollStrategy = overlay.scrollStrategies.block();
overlayRef = overlay.create(overlayState);
componentPortal = new ComponentPortal(FocacciaMsg);

Expand Down
3 changes: 1 addition & 2 deletions src/cdk/overlay/scroll/close-scroll-strategy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ describe('CloseScrollStrategy', () => {
}));

beforeEach(inject([Overlay], (overlay: Overlay) => {
let overlayState = new OverlayState();
overlayState.scrollStrategy = overlay.scrollStrategies.close();
let overlayState = new OverlayState({scrollStrategy: overlay.scrollStrategies.close()});
overlayRef = overlay.create(overlayState);
componentPortal = new ComponentPortal(MozarellaMsg);
}));
Expand Down
3 changes: 1 addition & 2 deletions src/cdk/overlay/scroll/reposition-scroll-strategy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ describe('RepositionScrollStrategy', () => {
}));

beforeEach(inject([Overlay], (overlay: Overlay) => {
let overlayState = new OverlayState();
overlayState.scrollStrategy = overlay.scrollStrategies.reposition();
let overlayState = new OverlayState({scrollStrategy: overlay.scrollStrategies.reposition()});
overlayRef = overlay.create(overlayState);
componentPortal = new ComponentPortal(PastaMsg);
}));
Expand Down
5 changes: 3 additions & 2 deletions src/cdk/overlay/scroll/scroll-strategy.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ scroll strategy, to the `OverlayState`. By default, all overlays will use the `n
doesn't do anything. The other available strategies are `reposition`, `block` and `close`:

```ts
let overlayState = new OverlayState();
let overlayState = new OverlayState({
scrollStrategy: overlay.scrollStrategies.block()
});

overlayState.scrollStrategy = overlay.scrollStrategies.block();
this._overlay.create(overlayState).attach(yourPortal);
```

Expand Down
21 changes: 8 additions & 13 deletions src/demo-app/overlay/overlay-demo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,9 @@ export class OverlayDemo {
{originX: 'start', originY: 'bottom'},
{overlayX: 'start', overlayY: 'top'} );

let config = new OverlayState();
config.positionStrategy = strategy;

let config = new OverlayState({positionStrategy: strategy});
let overlayRef = this.overlay.create(config);

overlayRef.attach(new ComponentPortal(SpagettiPanel, this.viewContainerRef));
}

Expand All @@ -88,22 +87,18 @@ export class OverlayDemo {
{originX: 'start', originY: 'bottom'},
{overlayX: 'end', overlayY: 'top'} );

let config = new OverlayState();
config.positionStrategy = strategy;

let config = new OverlayState({positionStrategy: strategy});
let overlayRef = this.overlay.create(config);

overlayRef.attach(this.tortelliniTemplate);
}

openPanelWithBackdrop() {
let config = new OverlayState();

config.positionStrategy = this.overlay.position()
.global()
.centerHorizontally();
config.hasBackdrop = true;
config.backdropClass = 'cdk-overlay-transparent-backdrop';
let config = new OverlayState({
hasBackdrop: true,
backdropClass: 'cdk-overlay-transparent-backdrop',
positionStrategy: this.overlay.position().global().centerHorizontally()
});

let overlayRef = this.overlay.create(config);
overlayRef.attach(this.templatePortals.first);
Expand Down
12 changes: 6 additions & 6 deletions src/lib/autocomplete/autocomplete-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -468,12 +468,12 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
}

private _getOverlayConfig(): OverlayState {
const overlayState = new OverlayState();
overlayState.positionStrategy = this._getOverlayPosition();
overlayState.width = this._getHostWidth();
overlayState.direction = this._dir ? this._dir.value : 'ltr';
overlayState.scrollStrategy = this._scrollStrategy();
return overlayState;
return new OverlayState({
positionStrategy: this._getOverlayPosition(),
scrollStrategy: this._scrollStrategy(),
width: this._getHostWidth(),
direction: this._dir ? this._dir.value : 'ltr'
});
}

private _getOverlayPosition(): PositionStrategy {
Expand Down
13 changes: 7 additions & 6 deletions src/lib/datepicker/datepicker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -322,12 +322,13 @@ export class MdDatepicker<D> implements OnDestroy {

/** Create the popup. */
private _createPopup(): void {
const overlayState = new OverlayState();
overlayState.positionStrategy = this._createPopupPositionStrategy();
overlayState.hasBackdrop = true;
overlayState.backdropClass = 'md-overlay-transparent-backdrop';
overlayState.direction = this._dir ? this._dir.value : 'ltr';
overlayState.scrollStrategy = this._scrollStrategy();
const overlayState = new OverlayState({
positionStrategy: this._createPopupPositionStrategy(),
hasBackdrop: true,
backdropClass: 'md-overlay-transparent-backdrop',
direction: this._dir ? this._dir.value : 'ltr',
scrollStrategy: this._scrollStrategy()
});

this._popupRef = this._overlay.create(overlayState);
}
Expand Down
18 changes: 10 additions & 8 deletions src/lib/dialog/dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,17 +186,19 @@ export class MdDialog {
* @returns The overlay configuration.
*/
private _getOverlayState(dialogConfig: MdDialogConfig): OverlayState {
const overlayState = new OverlayState();
overlayState.panelClass = dialogConfig.panelClass;
overlayState.hasBackdrop = dialogConfig.hasBackdrop;
overlayState.scrollStrategy = this._scrollStrategy();
overlayState.direction = dialogConfig.direction;
const state = new OverlayState({
positionStrategy: this._overlay.position().global(),
scrollStrategy: this._scrollStrategy(),
panelClass: dialogConfig.panelClass,
hasBackdrop: dialogConfig.hasBackdrop,
direction: dialogConfig.direction
});

if (dialogConfig.backdropClass) {
overlayState.backdropClass = dialogConfig.backdropClass;
state.backdropClass = dialogConfig.backdropClass;
}
overlayState.positionStrategy = this._overlay.position().global();

return overlayState;
return state;
}

/**
Expand Down
14 changes: 7 additions & 7 deletions src/lib/menu/menu-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -322,13 +322,13 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
* @returns OverlayState
*/
private _getOverlayConfig(): OverlayState {
const overlayState = new OverlayState();
overlayState.positionStrategy = this._getPosition();
overlayState.hasBackdrop = !this.triggersSubmenu();
overlayState.backdropClass = 'cdk-overlay-transparent-backdrop';
overlayState.direction = this.dir;
overlayState.scrollStrategy = this._scrollStrategy();
return overlayState;
return new OverlayState({
positionStrategy: this._getPosition(),
hasBackdrop: !this.triggersSubmenu(),
backdropClass: 'cdk-overlay-transparent-backdrop',
direction: this.dir,
scrollStrategy: this._scrollStrategy()
});
}

/**
Expand Down
8 changes: 5 additions & 3 deletions src/lib/snack-bar/snack-bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,11 @@ export class MdSnackBar {
* @param config The user-specified snack bar config.
*/
private _createOverlay(config: MdSnackBarConfig): OverlayRef {
const state = new OverlayState();
state.direction = config.direction;
state.positionStrategy = this._overlay.position().global().centerHorizontally().bottom('0');
const state = new OverlayState({
direction: config.direction,
positionStrategy: this._overlay.position().global().centerHorizontally().bottom('0')
});

return this._overlay.create(state);
}

Expand Down
18 changes: 9 additions & 9 deletions src/lib/tooltip/tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,13 +279,13 @@ export class MdTooltip implements OnDestroy {

/** Create the overlay config and position strategy */
private _createOverlay(): OverlayRef {
let origin = this._getOrigin();
let position = this._getOverlayPosition();
const origin = this._getOrigin();
const position = this._getOverlayPosition();

// Create connected position strategy that listens for scroll events to reposition.
// After position changes occur and the overlay is clipped by a parent scrollable then
// close the tooltip.
let strategy = this._overlay.position().connectedTo(this._elementRef, origin, position);
const strategy = this._overlay.position().connectedTo(this._elementRef, origin, position);
strategy.withScrollableContainers(this._scrollDispatcher.getScrollContainers(this._elementRef));
strategy.onPositionChange.subscribe(change => {
if (change.scrollableViewProperties.isOverlayClipped &&
Expand All @@ -294,12 +294,12 @@ export class MdTooltip implements OnDestroy {
}
});

let config = new OverlayState();

config.direction = this._dir ? this._dir.value : 'ltr';
config.positionStrategy = strategy;
config.panelClass = TOOLTIP_PANEL_CLASS;
config.scrollStrategy = this._scrollStrategy();
const config = new OverlayState({
direction: this._dir ? this._dir.value : 'ltr',
positionStrategy: strategy,
panelClass: TOOLTIP_PANEL_CLASS,
scrollStrategy: this._scrollStrategy()
});

this._overlayRef = this._overlay.create(config);

Expand Down