From 21f54ee389ad54e98608164736be710aa48c070a Mon Sep 17 00:00:00 2001 From: Shlomi Assaf Date: Sun, 13 Aug 2017 16:20:12 +0300 Subject: [PATCH] refactor: dont return a promise when opening a dialog (#383) 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. --- README.md | 21 ++++++++----- package.json | 2 +- src/demo/app/demo-head/deam-head.ts | 9 +++--- src/demo/app/home/home.ts | 17 ++++++----- src/demo/app/home/in-app-plugin/modal.ts | 8 ++--- src/ngx-modialog/package.json | 2 +- .../plugins/bootstrap/package.json | 2 +- .../plugins/bootstrap/src/modal-container.ts | 0 .../plugins/bootstrap/src/modal.ts | 3 +- .../plugins/js-native/package.json | 2 +- .../plugins/js-native/src/modal.ts | 7 ++--- .../js-native/src/presets/js-native-preset.ts | 2 +- src/ngx-modialog/plugins/vex/package.json | 2 +- src/ngx-modialog/plugins/vex/src/modal.ts | 3 +- src/ngx-modialog/src/framework/utils.ts | 2 -- .../src/models/modal-open-context.ts | 2 +- .../src/models/overlay-context.ts | 2 +- src/ngx-modialog/src/models/tokens.ts | 6 ++-- src/ngx-modialog/src/ngx-modialog.ts | 2 +- src/ngx-modialog/src/providers/modal.ts | 30 +++++++------------ 20 files changed, 54 insertions(+), 70 deletions(-) delete mode 100644 src/ngx-modialog/plugins/bootstrap/src/modal-container.ts diff --git a/README.md b/README.md index 710a37d9..65c52500 100644 --- a/README.md +++ b/README.md @@ -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') @@ -82,22 +82,27 @@ export class AppComponent {
  • Close wth button click
  • HTML content
  • `) - .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. diff --git a/package.json b/package.json index de5d869c..6354ec67 100644 --- a/package.json +++ b/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" diff --git a/src/demo/app/demo-head/deam-head.ts b/src/demo/app/demo-head/deam-head.ts index df77a0a0..96edef37 100644 --- a/src/demo/app/demo-head/deam-head.ts +++ b/src/demo/app/demo-head/deam-head.ts @@ -10,7 +10,7 @@ import { DialogRef } from 'ngx-modialog'; export interface ModalCommandDescriptor { text: string; - factory: () => Promise>; + factory: () => DialogRef; } export interface DropInClickEvent { event: Event; @@ -51,11 +51,10 @@ export class DemoHead { this.processDialog(btn.factory()); } - private processDialog(dialog: Promise>) { - dialog.then((resultPromise) => { - return resultPromise.result.then((result) => { + private processDialog(dialog: DialogRef) { + return dialog.result.then((result) => { this.result = result; }, () => this.result = 'Rejected!'); - }); + } } diff --git a/src/demo/app/home/home.ts b/src/demo/app/home/home.ts index e5dbe4dc..6bd576e1 100644 --- a/src/demo/app/home/home.ts +++ b/src/demo/app/home/home.ts @@ -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); + }); } } diff --git a/src/demo/app/home/in-app-plugin/modal.ts b/src/demo/app/home/in-app-plugin/modal.ts index 62cd98d8..f0e3f375 100644 --- a/src/demo/app/home/in-app-plugin/modal.ts +++ b/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, @@ -24,7 +20,7 @@ export class Modal extends Modal_ { return new InAppModalContextBuilder(this); } - protected create(dialogRef: DialogRef, content: ContainerContent): Maybe> { + protected create(dialogRef: DialogRef, content: ContainerContent): DialogRef { if (dialogRef.inElement) { dialogRef.overlayRef.instance.insideElement(); } else { diff --git a/src/ngx-modialog/package.json b/src/ngx-modialog/package.json index a09a95f6..95ee16e5 100644 --- a/src/ngx-modialog/package.json +++ b/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, diff --git a/src/ngx-modialog/plugins/bootstrap/package.json b/src/ngx-modialog/plugins/bootstrap/package.json index 06ae6ce3..b5d41ff9 100644 --- a/src/ngx-modialog/plugins/bootstrap/package.json +++ b/src/ngx-modialog/plugins/bootstrap/package.json @@ -1,3 +1,3 @@ { - "version": "4.0.0-beta.0" + "version": "4.0.0-beta.1" } diff --git a/src/ngx-modialog/plugins/bootstrap/src/modal-container.ts b/src/ngx-modialog/plugins/bootstrap/src/modal-container.ts deleted file mode 100644 index e69de29b..00000000 diff --git a/src/ngx-modialog/plugins/bootstrap/src/modal.ts b/src/ngx-modialog/plugins/bootstrap/src/modal.ts index 0e2003dc..1310a2f4 100644 --- a/src/ngx-modialog/plugins/bootstrap/src/modal.ts +++ b/src/ngx-modialog/plugins/bootstrap/src/modal.ts @@ -2,7 +2,6 @@ import { combineLatest } from 'rxjs/operator/combineLatest'; import { Injectable } from '@angular/core'; import { - Maybe, ContainerContent, Overlay, DialogRef, @@ -47,7 +46,7 @@ export class Modal extends Modal_ { return new TwoButtonPresetBuilder(this, {isBlocking: true, keyboard: null}); } - protected create(dialogRef: DialogRef, content: ContainerContent): Maybe> { + protected create(dialogRef: DialogRef, content: ContainerContent): DialogRef { const backdropRef = this.createBackdrop(dialogRef, CSSBackdrop); const containerRef = this.createContainer(dialogRef, BSModalContainer, content); diff --git a/src/ngx-modialog/plugins/js-native/package.json b/src/ngx-modialog/plugins/js-native/package.json index 06ae6ce3..b5d41ff9 100644 --- a/src/ngx-modialog/plugins/js-native/package.json +++ b/src/ngx-modialog/plugins/js-native/package.json @@ -1,3 +1,3 @@ { - "version": "4.0.0-beta.0" + "version": "4.0.0-beta.1" } diff --git a/src/ngx-modialog/plugins/js-native/src/modal.ts b/src/ngx-modialog/plugins/js-native/src/modal.ts index 0f06e951..af0f6702 100644 --- a/src/ngx-modialog/plugins/js-native/src/modal.ts +++ b/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_ @@ -28,9 +27,7 @@ export class Modal extends Modal_ { return new JSNativePresetBuilder(this, DROP_IN_TYPE.confirm); } - protected create(dialogRef: DialogRef, - type: any, - bindings?: RRP[]): Maybe> { + protected create(dialogRef: DialogRef, type: any): DialogRef { return dialogRef; } diff --git a/src/ngx-modialog/plugins/js-native/src/presets/js-native-preset.ts b/src/ngx-modialog/plugins/js-native/src/presets/js-native-preset.ts index d3d5fba5..ee965438 100644 --- a/src/ngx-modialog/plugins/js-native/src/presets/js-native-preset.ts +++ b/src/ngx-modialog/plugins/js-native/src/presets/js-native-preset.ts @@ -19,7 +19,7 @@ export class JSNativePresetBuilder extends JSNativeModalContextBuilder */ - open(viewContainer?: ViewContainerRef): Promise> { + open(viewContainer?: ViewContainerRef): DialogRef { let context: JSNativeModalContext = this.toJSON(); if (!(context.modal instanceof Modal)) { diff --git a/src/ngx-modialog/plugins/vex/package.json b/src/ngx-modialog/plugins/vex/package.json index 06ae6ce3..b5d41ff9 100644 --- a/src/ngx-modialog/plugins/vex/package.json +++ b/src/ngx-modialog/plugins/vex/package.json @@ -1,3 +1,3 @@ { - "version": "4.0.0-beta.0" + "version": "4.0.0-beta.1" } diff --git a/src/ngx-modialog/plugins/vex/src/modal.ts b/src/ngx-modialog/plugins/vex/src/modal.ts index dbd2c009..b72abb53 100644 --- a/src/ngx-modialog/plugins/vex/src/modal.ts +++ b/src/ngx-modialog/plugins/vex/src/modal.ts @@ -4,7 +4,6 @@ import { Injectable } from '@angular/core'; import { ContainerContent, - Maybe, Overlay, DialogRef, DROP_IN_TYPE, @@ -44,7 +43,7 @@ export class Modal extends Modal_ { } as any); } - protected create(dialogRef: DialogRef, content: ContainerContent): Maybe> { + protected create(dialogRef: DialogRef, content: ContainerContent): DialogRef { const backdropRef = this.createBackdrop(dialogRef, CSSBackdrop); const containerRef = this.createContainer(dialogRef, CSSDialogContainer, content); diff --git a/src/ngx-modialog/src/framework/utils.ts b/src/ngx-modialog/src/framework/utils.ts index 90015bc9..55ba5be7 100644 --- a/src/ngx-modialog/src/framework/utils.ts +++ b/src/ngx-modialog/src/framework/utils.ts @@ -88,6 +88,4 @@ export interface Class { new(...args: any[]): T; } -export type Maybe = T | Promise; - export function noop() { } diff --git a/src/ngx-modialog/src/models/modal-open-context.ts b/src/ngx-modialog/src/models/modal-open-context.ts index 7bcea81b..9973df94 100644 --- a/src/ngx-modialog/src/models/modal-open-context.ts +++ b/src/ngx-modialog/src/models/modal-open-context.ts @@ -53,7 +53,7 @@ export abstract class ModalOpenContextBuilder * @param viewContainer If set opens the modal inside the supplied viewContainer * @returns Promise */ - open(viewContainer?: WideVCRef): Promise> { + open(viewContainer?: WideVCRef): DialogRef { let context: T = this.toJSON(); if (!(context.modal instanceof Modal)) { diff --git a/src/ngx-modialog/src/models/overlay-context.ts b/src/ngx-modialog/src/models/overlay-context.ts index 939076a2..df190779 100644 --- a/src/ngx-modialog/src/models/overlay-context.ts +++ b/src/ngx-modialog/src/models/overlay-context.ts @@ -106,7 +106,7 @@ export class OverlayContextBuilder extends FluentAssig } export interface ModalControllingContextBuilder { - open(viewContainer?: WideVCRef): Promise>; + open(viewContainer?: WideVCRef): DialogRef; } /** diff --git a/src/ngx-modialog/src/models/tokens.ts b/src/ngx-modialog/src/models/tokens.ts index d9732bbb..33463fc6 100644 --- a/src/ngx-modialog/src/models/tokens.ts +++ b/src/ngx-modialog/src/models/tokens.ts @@ -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, @@ -23,7 +21,7 @@ export type WideVCRef = ViewContainerRef | string; export type ContainerContent = string | TemplateRef | Type; export interface OverlayPlugin extends Function { - (component: any, dialogRef: DialogRef, config: OverlayConfig): Maybe>; + (component: any, dialogRef: DialogRef, config: OverlayConfig): DialogRef; } export interface OverlayConfig { diff --git a/src/ngx-modialog/src/ngx-modialog.ts b/src/ngx-modialog/src/ngx-modialog.ts index ef3053b5..312e0715 100644 --- a/src/ngx-modialog/src/ngx-modialog.ts +++ b/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'; diff --git a/src/ngx-modialog/src/providers/modal.ts b/src/ngx-modialog/src/providers/modal.ts index 526146c9..b4c43b4f 100644 --- a/src/ngx-modialog/src/providers/modal.ts +++ b/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'; @@ -32,26 +32,18 @@ export abstract class Modal { * @param config Additional settings. * @returns {Promise} */ - open(content: ContainerContent, config?: OverlayConfig): Promise> { + open(content: ContainerContent, config?: OverlayConfig): DialogRef { 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>(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) } /** @@ -60,7 +52,7 @@ export abstract class Modal { * @param type * @returns {Maybe>} */ - protected abstract create(dialogRef: DialogRef, type: ContainerContent): Maybe>; + protected abstract create(dialogRef: DialogRef, type: ContainerContent): DialogRef; protected createBackdrop(dialogRef: DialogRef, BackdropComponent: Class): ComponentRef {