Guarded route support to ConfirmDialog

[ova2]

If you use ConfirmDialog with CanDeactivate guard, you normally have to return Promise or Observable in the canDeactivate method. Here is the method signature: https://angular.io/docs/ts/latest/api/router/index/CanDeactivate-interface.html This works very well as long the user clicks on accept or reject buttons. But when the user clicks on the dialog's close button (or hit escape key), the dialog gets closed and can not be opened after that. It stops to work.

Demo is created: https://github.com/ova2/frontend-tooling-tutorial/tree/master/angular-playground/router-primeng-confirmdialog

I think in the confirmdialog.ts, you should not invoke (click)="hide($event)" on close. You should invoke (click)="reject($event)". If you invoke reject which invokes hide on its parts, the dialog is ok. Also on hitting escape key, the this.reject(event) should be called instead of this.hide(event).

Should I provide a pull request?

Thanks.

[ova2]

I have found some other weak points in the ConfirmDialog.

• Confirmation object doesn't need to expose acceptEvent and rejectEvent because they are not used public. You can keep these references in the Component ConfirmDialog itself.
• Subscription like this.confirmation.acceptEvent.subscribe(...) should be unsubscribed when accept() resp. reject are called. There is unsubscribe() on the return object of subscribe(). See http://stackoverflow.com/questions/36494509/how-to-unsubscribe-from-eventemitter-in-angular-2 If you keep the references in the Component, you can call unsubscribe() in the destroy method.
• Don't use EventEmitter except for properties decorated with @Output. It's not recommended. Use Observable or Subject. You can read that in some docs. See the stackoverflow post I have mentioned above. Background: Angular will never guarantee us that EventEmitter will continue being an Observable.

[cagataycivici]

Send a PR please Oleg.

[ova2]

@cagataycivici I have created a PR for weak points: #3201 It looks cleaner now. The issue on close was already fixed earlier.

Thanks. Oleg.

[cagataycivici]

#921 should fix the original issue right?

But when the user clicks on the dialog's close button (or hit escape key), the dialog gets closed and can not be opened after that. It stops to work.

[ova2]

Yes, the original issue is fixed by #921. This PR fixes weak points I mentioned in "I have found some other weak points in the ConfirmDialog."

[ova2]

It's disappointing that PR has conflicts now. Why do we need this.executePostShowActions? Can we not always set focus on first button in the confirm dialog?

[cagataycivici]

Sorry, this.executePostShowActions is to focus button after show. I can merge manually later, no problem.

[ova2]

I've solved conflicts. No problem. You can merge the PR.