Skip to content
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

refactor: dont return a promise when opening a dialog #383

Merged
merged 1 commit into from
Aug 13, 2017

Conversation

shlomiassaf
Copy link
Owner

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.

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.
@shlomiassaf shlomiassaf merged commit 21f54ee into master Aug 13, 2017
@shlomiassaf shlomiassaf deleted the refactor/remove-promise-based-overlay-opening branch August 13, 2017 13:20
@alfupe
Copy link

alfupe commented Aug 16, 2017

Thanks a lot @shlomiassaf. So sorry, I'm a bit lost, how do you refactor this?

openDetailModal(event) {
  event.preventDefault();

  const dialog = this.modal.open(BuyerCRMDetailModalComponent, overlayConfigFactory({buyer: this.buyer}, BSModalContext));

  dialog.then(resultPromise => {
    return resultPromise.result
      .then(
        result => {
          this.log.info('openDetailModal()', 'then', result);

          if (!result) return;
          this.log.info('do something with', result);
        },
        () => {
          this.log.info('openDetailModal()', 'Rejected');
        });
    });
}

@Gbacc
Copy link

Gbacc commented Dec 15, 2017

Looking at the code change i guess :

openDetailModal(event) {
  event.preventDefault();

  const dialog = this.modal.open(BuyerCRMDetailModalComponent, overlayConfigFactory({buyer: this.buyer}, BSModalContext));

  dialog.then(
        result => {
          this.log.info('openDetailModal()', 'then', result);

          if (!result) return;
          this.log.info('do something with', result);
        },
        () => {
          this.log.info('openDetailModal()', 'Rejected');
        });
}

@alfupe
Copy link

alfupe commented Feb 19, 2018

@Gbacc Thanks for helping out!

The code you put is almost working though we need to call result on dialog:

...
dialog.result.then(
...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants