Skip to content

Commit

Permalink
refactor: dont return a promise when opening a dialog (#383)
Browse files Browse the repository at this point in the history
BREAKING CHANGE:
Calling `modal.open()` returned a Promise of `DialogRef`. The async operation is not required and exists due to legacy angular implementation of dynamic components.
`modal.open()` now returns the `DialogRef` instance and NOT the `Promise`.

You need to refactor your codebase to accomodate this change, this is a big change
and it is required to remove the complexity working with `DialogRef`.

Plugin authoers: `Maybe` type does not exists anymore.
If you retuned the dialog instance from your `Modal` implementations there is not much to refactor other then types here and there.

If you return `Promise` of `DialogRef` you will have to refactor your code.
  • Loading branch information
shlomiassaf committed Aug 13, 2017
1 parent 3a1eb76 commit 21f54ee
Show file tree
Hide file tree
Showing 20 changed files with 54 additions and 70 deletions.
21 changes: 13 additions & 8 deletions README.md
Expand Up @@ -67,7 +67,7 @@ export class AppComponent {
constructor(public modal: Modal) { }

onClick() {
this.modal.alert()
const dialogRef = this.modal.alert()
.size('lg')
.showClose(true)
.title('A simple Alert style modal window')
Expand All @@ -82,22 +82,27 @@ export class AppComponent {
<li>Close wth button click</li>
<li>HTML content</li>
</ul>`)
.open()
.then( dialogRef => {
dialogRef.result.then( result => alert(`The result is: ${result}`);
});
.open();

dialogRef.result
.then( result => alert(`The result is: ${result}`) );
}
}
```

We are using the `alert()` method, one of 3 (prompt, confirm)) fluent-api methods we call `drop-ins`
If you are using **ngx-modialog** version 3.X.X or below, `open()` returned a promise so replace the last 2 lines with:
```typescript
dialogRef
.then( dialogRef => {
dialogRef.result.then( result => alert(`The result is: ${result}`);
});
```
The object returned from calling the `open()` method is a `Promise` to a {@link DialogRef} instance, `DialogRef` is how you can control an open modal instance.
We are using the `alert()` method, one of 3 (prompt, confirm)) fluent-api methods we call `drop-ins`
We then use the `result` property to wait for the modal closing event.
**Notes:**
- The `Promise` returned from `open()` is not required and exists due to legacy angular implementation when creating components dynamically was an async operation.
- Fluent API methods (drop-ins) are pre-configured (presets) methods that allow easy configuration and execution, you can create custom presets - see the demo application.
- For more control use the `open()` method, which is used by all drop in's internally.
- We import the `Modal` service from the plugin and not from the root library.
Expand Down
2 changes: 1 addition & 1 deletion package.json
@@ -1,7 +1,7 @@
{
"name": "ngx-modialog",
"description": "Modal / Dialog for Angular",
"version": "4.0.0-beta.0",
"version": "4.0.0-beta.1",
"repository": {
"type": "git",
"url": "https://github.com/shlomiassaf/ngx-modialog.git"
Expand Down
9 changes: 4 additions & 5 deletions src/demo/app/demo-head/deam-head.ts
Expand Up @@ -10,7 +10,7 @@ import { DialogRef } from 'ngx-modialog';

export interface ModalCommandDescriptor {
text: string;
factory: () => Promise<DialogRef<any>>;
factory: () => DialogRef<any>;
}
export interface DropInClickEvent {
event: Event;
Expand Down Expand Up @@ -51,11 +51,10 @@ export class DemoHead {
this.processDialog(btn.factory());
}

private processDialog(dialog: Promise<DialogRef<any>>) {
dialog.then((resultPromise) => {
return resultPromise.result.then((result) => {
private processDialog(dialog: DialogRef<any>) {
return dialog.result.then((result) => {
this.result = result;
}, () => this.result = 'Rejected!');
});

}
}
17 changes: 9 additions & 8 deletions src/demo/app/home/home.ts
Expand Up @@ -23,13 +23,14 @@ export class Home {
.templateRef(this.myTemplate)
.inElement(true)
.open('home-overlay-container')
.then(d => d.result)
.catch((e) => {
console.log('This message should appear if you navigate away from the home page.');
console.log('If a modal is opened in a view container within a component that is the page or' +
'part of the page, navigation will destroy the page thus destroy the modal. To prevent ' +
'memory leaks and unexpected behavior a "DialogBailOutError" error is thrown.');
console.log(e);
});
.result
.then(d => d.result)
.catch((e) => {
console.log('This message should appear if you navigate away from the home page.');
console.log('If a modal is opened in a view container within a component that is the page or' +
'part of the page, navigation will destroy the page thus destroy the modal. To prevent ' +
'memory leaks and unexpected behavior a "DialogBailOutError" error is thrown.');
console.log(e);
});
}
}
8 changes: 2 additions & 6 deletions src/demo/app/home/in-app-plugin/modal.ts
@@ -1,10 +1,6 @@
import {
Injectable,
ResolvedReflectiveProvider as RRP
} from '@angular/core';
import { Injectable } from '@angular/core';

import {
Maybe,
Overlay,
DialogRef,
ContainerContent,
Expand All @@ -24,7 +20,7 @@ export class Modal extends Modal_ {
return new InAppModalContextBuilder(this);
}

protected create(dialogRef: DialogRef<any>, content: ContainerContent): Maybe<DialogRef<any>> {
protected create(dialogRef: DialogRef<any>, content: ContainerContent): DialogRef<any> {
if (dialogRef.inElement) {
dialogRef.overlayRef.instance.insideElement();
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/ngx-modialog/package.json
@@ -1,7 +1,7 @@
{
"name": "ngx-modialog",
"description": "Modal / Dialog for Angular",
"version": "4.0.0-beta.0",
"version": "4.0.0-beta.1",
"libConfig": {
"entry": "ngx-modialog",
"inlineResources": true,
Expand Down
2 changes: 1 addition & 1 deletion src/ngx-modialog/plugins/bootstrap/package.json
@@ -1,3 +1,3 @@
{
"version": "4.0.0-beta.0"
"version": "4.0.0-beta.1"
}
Empty file.
3 changes: 1 addition & 2 deletions src/ngx-modialog/plugins/bootstrap/src/modal.ts
Expand Up @@ -2,7 +2,6 @@ import { combineLatest } from 'rxjs/operator/combineLatest';
import { Injectable } from '@angular/core';

import {
Maybe,
ContainerContent,
Overlay,
DialogRef,
Expand Down Expand Up @@ -47,7 +46,7 @@ export class Modal extends Modal_ {
return new TwoButtonPresetBuilder(this, <any>{isBlocking: true, keyboard: null});
}

protected create(dialogRef: DialogRef<any>, content: ContainerContent): Maybe<DialogRef<any>> {
protected create(dialogRef: DialogRef<any>, content: ContainerContent): DialogRef<any> {

const backdropRef = this.createBackdrop(dialogRef, CSSBackdrop);
const containerRef = this.createContainer(dialogRef, BSModalContainer, content);
Expand Down
2 changes: 1 addition & 1 deletion src/ngx-modialog/plugins/js-native/package.json
@@ -1,3 +1,3 @@
{
"version": "4.0.0-beta.0"
"version": "4.0.0-beta.1"
}
7 changes: 2 additions & 5 deletions src/ngx-modialog/plugins/js-native/src/modal.ts
@@ -1,8 +1,7 @@
import { Injectable, ResolvedReflectiveProvider as RRP } from '@angular/core';
import { Injectable } from '@angular/core';

import {
DialogRef,
Maybe,
Overlay,
DROP_IN_TYPE,
Modal as Modal_
Expand All @@ -28,9 +27,7 @@ export class Modal extends Modal_ {
return new JSNativePresetBuilder(this, DROP_IN_TYPE.confirm);
}

protected create(dialogRef: DialogRef<any>,
type: any,
bindings?: RRP[]): Maybe<DialogRef<any>> {
protected create(dialogRef: DialogRef<any>, type: any): DialogRef<any> {
return dialogRef;
}

Expand Down
Expand Up @@ -19,7 +19,7 @@ export class JSNativePresetBuilder extends JSNativeModalContextBuilder<JSNativeM
* @param viewContainer If set opens the modal inside the supplied viewContainer
* @returns Promise<DialogRef>
*/
open(viewContainer?: ViewContainerRef): Promise<DialogRef<JSNativeModalContext>> {
open(viewContainer?: ViewContainerRef): DialogRef<JSNativeModalContext> {
let context: JSNativeModalContext = this.toJSON();

if (!(context.modal instanceof Modal)) {
Expand Down
2 changes: 1 addition & 1 deletion src/ngx-modialog/plugins/vex/package.json
@@ -1,3 +1,3 @@
{
"version": "4.0.0-beta.0"
"version": "4.0.0-beta.1"
}
3 changes: 1 addition & 2 deletions src/ngx-modialog/plugins/vex/src/modal.ts
Expand Up @@ -4,7 +4,6 @@ import { Injectable } from '@angular/core';

import {
ContainerContent,
Maybe,
Overlay,
DialogRef,
DROP_IN_TYPE,
Expand Down Expand Up @@ -44,7 +43,7 @@ export class Modal extends Modal_ {
} as any);
}

protected create(dialogRef: DialogRef<any>, content: ContainerContent): Maybe<DialogRef<any>> {
protected create(dialogRef: DialogRef<any>, content: ContainerContent): DialogRef<any> {

const backdropRef = this.createBackdrop(dialogRef, CSSBackdrop);
const containerRef = this.createContainer(dialogRef, CSSDialogContainer, content);
Expand Down
2 changes: 0 additions & 2 deletions src/ngx-modialog/src/framework/utils.ts
Expand Up @@ -88,6 +88,4 @@ export interface Class<T> {
new(...args: any[]): T;
}

export type Maybe<T> = T | Promise<T>;

export function noop() { }
2 changes: 1 addition & 1 deletion src/ngx-modialog/src/models/modal-open-context.ts
Expand Up @@ -53,7 +53,7 @@ export abstract class ModalOpenContextBuilder<T extends ModalOpenContext>
* @param viewContainer If set opens the modal inside the supplied viewContainer
* @returns Promise<DialogRef>
*/
open(viewContainer?: WideVCRef): Promise<DialogRef<T>> {
open(viewContainer?: WideVCRef): DialogRef<T> {
let context: T = this.toJSON();

if (!(context.modal instanceof Modal)) {
Expand Down
2 changes: 1 addition & 1 deletion src/ngx-modialog/src/models/overlay-context.ts
Expand Up @@ -106,7 +106,7 @@ export class OverlayContextBuilder<T extends OverlayContext> extends FluentAssig
}

export interface ModalControllingContextBuilder<T> {
open(viewContainer?: WideVCRef): Promise<DialogRef<T>>;
open(viewContainer?: WideVCRef): DialogRef<T>;
}

/**
Expand Down
6 changes: 2 additions & 4 deletions src/ngx-modialog/src/models/tokens.ts
Expand Up @@ -3,14 +3,12 @@ import {
Injector,
ViewContainerRef,
TemplateRef,
Type,
ResolvedReflectiveProvider
Type
} from '@angular/core';

import { ModalOverlay } from '../overlay/index';
import { DialogRef } from './dialog-ref';
import { OverlayContext } from '../models/overlay-context';
import { Maybe } from '../framework/utils';

export enum DROP_IN_TYPE {
alert,
Expand All @@ -23,7 +21,7 @@ export type WideVCRef = ViewContainerRef | string;
export type ContainerContent = string | TemplateRef<any> | Type<any>;

export interface OverlayPlugin extends Function {
<T>(component: any, dialogRef: DialogRef<T>, config: OverlayConfig): Maybe<DialogRef<any>>;
<T>(component: any, dialogRef: DialogRef<T>, config: OverlayConfig): DialogRef<any>;
}

export interface OverlayConfig {
Expand Down
2 changes: 1 addition & 1 deletion src/ngx-modialog/src/ngx-modialog.ts
@@ -1,7 +1,7 @@
import { Modal } from './providers/index';

export * from './framework/fluent-assign';
export { extend, arrayUnion, PromiseCompleter, Maybe } from './framework/utils';
export { extend, arrayUnion, PromiseCompleter } from './framework/utils';
export { createComponent, CreateComponentArgs } from './framework/createComponent';

export * from './models/errors';
Expand Down
30 changes: 11 additions & 19 deletions src/ngx-modialog/src/providers/modal.ts
@@ -1,7 +1,7 @@
import { ComponentRef } from '@angular/core';

import { Overlay } from '../overlay/index';
import { Class, Maybe } from '../framework/utils';
import { Class } from '../framework/utils';
import { OverlayConfig, ContainerContent } from '../models/tokens';
import { DialogRef } from '../models/dialog-ref';
import { ModalControllingContextBuilder } from '../models/overlay-context';
Expand Down Expand Up @@ -32,26 +32,18 @@ export abstract class Modal {
* @param config Additional settings.
* @returns {Promise<DialogRef>}
*/
open(content: ContainerContent, config?: OverlayConfig): Promise<DialogRef<any>> {
open(content: ContainerContent, config?: OverlayConfig): DialogRef<any> {
config = config || {} as any;
try {
let dialogs = this.overlay.open(config, this.constructor);
let dialogs = this.overlay.open(config, this.constructor);

if (dialogs.length > 1) {
console.warn(`Attempt to open more then 1 overlay detected.
Multiple modal copies are not supported at the moment,
only the first viewContainer will display.`);
}
// TODO: Currently supporting 1 view container, hence working on dialogs[0].
// upgrade to multiple containers.
return Promise.resolve(
this.create(dialogs[0], content)
);

} catch (e) {
return Promise.reject<DialogRef<any>>(e);
if (dialogs.length > 1) {
console.warn(`Attempt to open more then 1 overlay detected.
Multiple modal copies are not supported at the moment,
only the first viewContainer will display.`);
}

// TODO: Currently supporting 1 view container, hence working on dialogs[0].
// upgrade to multiple containers.
return this.create(dialogs[0], content)
}

/**
Expand All @@ -60,7 +52,7 @@ export abstract class Modal {
* @param type
* @returns {Maybe<DialogRef<any>>}
*/
protected abstract create(dialogRef: DialogRef<any>, type: ContainerContent): Maybe<DialogRef<any>>;
protected abstract create(dialogRef: DialogRef<any>, type: ContainerContent): DialogRef<any>;


protected createBackdrop<T>(dialogRef: DialogRef<any>, BackdropComponent: Class<T>): ComponentRef<T> {
Expand Down

0 comments on commit 21f54ee

Please sign in to comment.