Angular dialogs should limit promise rejections to error conditions
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Evergreen |
Fix Released
|
Medium
|
Unassigned |
Bug Description
Evergreen 3.3
Currently Angular modal dialogs result in a promise rejection when the dialog is dismissed, a Cancel button is clicked, etc. Since these are expected outcomes of modal dialogs and are not error conditions, I propose they be propagated via the dialog promise resolver instead of being rejected. Rejections should be limited to error conditions (e.g. a failed database update performed by the dialog).
A side benefit of this approach is dismissed dialogs will no longer produce console errors for unhandled dismissals.
I propose we also add a 'dismissed' class variable to the base dialog class which may be inspected as needed by the caller, in cases where a dismissal should be treated differently than, say, a Cancel button.
As a simple example, a confirm dialog like this:
this.dialog.
confirmed => doStuff(),
// avoid console errors by "catching" the dismissal via dummy handler.
cancelOrDis
);
would look like this:
this.dialog.
result => { if (result) { doStuff(); } }
);
In the second example, an Exception is only thrown and errors logged (via the dialog internals) if an error condition occurs.
This will require modifying a number of existing dialogs in the Angular code. Better now than later, though.
Changed in evergreen: | |
milestone: | 3.3.1 → 3.3.2 |
Changed in evergreen: | |
assignee: | nobody → Jane Sandberg (sandbej) |
importance: | Undecided → Medium |
Changed in evergreen: | |
assignee: | nobody → Bill Erickson (berick) |
Changed in evergreen: | |
status: | Fix Committed → Fix Released |
+1 to not treating closing a modal as an error.
While we're looking at the modals, though, I'm curious if they should return a Promise or an Observable instead. I ask because it seems like it could be very nice to handle success, error, and complete conditions (which RxJs provides), rather than just success and error.
See Angular Material's Observable-based modal: https:/ /material. angular. io/components/ dialog/ overview
Also the "Handling the result as an observable" section of this blog post: https:/ /aspnetaddict. wordpress. com/2017/ 10/28/making- ngbootstraps- modal-work- more-like- angular- material- dialog/