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

DialogCancelError - class or interface? #323

Open
thinkOfaNumber opened this issue Aug 22, 2017 · 6 comments
Open

DialogCancelError - class or interface? #323

thinkOfaNumber opened this issue Aug 22, 2017 · 6 comments

Comments

@thinkOfaNumber
Copy link

I'm submitting a feature request

  • Library Version:
    aurelia-dialog 1.0.0-rc.1.0.3

(I think the other bug template questions don't apply to a feature request but I can provide if required)

I'm using aurelia-dialog to confirm an action that gets posted to the server to change some object. It seems using rejectOnCancel is a good way to skip over the promise chain that deals with successfully changing the object. In the catch handler I need to differentiate between the respective DialogCancelError or some other error (e.g. from the httpClient).

Because DialogCancelError is an interface and not a class, I can't use the simple pattern to detect the type of error:

catch(error => {
    if (error instanceof DialogCancelError) {
        // handle the dialog cancel action
    } else {
        // handle a http method error
    }
}

Instead I have to use some sort of duck-typing.

My feature request (and I could create a PR if someone likes the idea) is to use a class instead so that I can use the pattern above.

If there's a better way of doing this, please let me know! I would rather not set rejectOnCancel: false because it's a very neat way of skipping the rest of the promise chain which would otherwise require passing and checking this cancel state through each then() method:

someMethod() {
    this.dialogService.open({
        viewModel: ...,
        model: ...,
        rejectOnCancel: true
    }).whenClosed(response => {
        this.busy = true;
        return this.service.someServerMethod(...);
    }).then(() => {
        // do stuff if the server method is successful, such as notifications, updates, etc.
        // there may be multiple then() handlers chained up here
    }).catch(error => {
        if (error instanceof DialogCancelError) {
            // handle the dialog cancel action
        } else {
            // handle a http method error
        }
    }).then(() => {
        this.busy = false;
    });
}
@StrahilKazlachev
Copy link
Contributor

The reason we used an interface are the Error subclassing issues when targeting <ES2015.
I prefer not introducing them. Also I don't find duck-typing that uncommon.
@PWKad Any comment from you?

@thinkOfaNumber
Copy link
Author

Ohhhh... that!

@Alexander-Taran
Copy link
Contributor

@thinkOfaNumber do you think it can be closed?

@thinkOfaNumber
Copy link
Author

It would be nice to move away from duck-typing, but I have no idea how error subclassing issues would affect other users of the framework! Given this is just my opinion and it's been "noted", then yes, I'll close 👍

@StrahilKazlachev
Copy link
Contributor

For me this issue is complementary to #328.

@Navyashreess
Copy link

from the dialog service I am opening the popup , but on close we have to open some validation popup . how to do that on close action from dialog service

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

No branches or pull requests

4 participants