Dialog.done should return itself for chaining

[iwehrman]

With the the recent modal dialog changes, Dialogs.showModalDialogUsingTemplate() returns a Dialog object that encapsulates a jQuery object representing the displayed dialog and a dismissal promise. For convenience and some measure of compatibility with the previous implementation, the Dialog object directly exposes the done() method of its promise.

This is great, but it would be even better if the Dialog.done() returned itself to support the following idiom:

var dlg = Dialogs.showModalDialogUsingTemplate(template).done(function () {
  // do something when the dialog closes
}); 

$otherThing.on("click", function () {
  // do something else and close the dialog
  dlg.close()
});

I like this because it reminds me of the promise chaining idiom that Dialog.done() mimicks.

This pull request just adds return this; to the definition of Dialog.prototype.done to accomplish this.

CC @TomMalbran

[njx]

Hmmm, I'm not 100% sure about this. If we want it to feel like the promise case, then it seems like done() should return the promise, not the Dialog.

[iwehrman]

Well, then you lose the reference to the Dialog. Anyway, I'm certainly not going to go to the mat on this one. I like it, but maybe nobody else does!

[TomMalbran]

I like the idea. I think I wrote something like that in the brackets-edge-web-fonts extension. It would be easy to get the promise in a chained way right after the done to with getPromise(), or we can add more promise methods to the Dialog object that user might want to use, to make it feel more like a promise return.

[dangoor]

I'm with @njx on this in feeling ambivalent about this change. Given how often we have .done() calls in our code, a caller would reasonably assume that it's a promise that's being returned. Generally, I'm against taking an idiom that's used all over and breaking the convention.

That said, the dialog can't fail. No one would need to add a .fail or .always or whatever. Given that, I think I'm okay with this change. It just has that feel of something that could be a (minor) annoyance later.

[dangoor]

Hmm... the change in #4102 actually appears to be fixing a case where the return value was treated like a promise. So, maybe the problem I fear has actually already occurred.

[jasonsanjose]

Filed #4125. I don't believe we use chaining that much in our own APIs. I understand that done is here for backwards compatibility, but I'd rather just see it removed.

[iwehrman]

Closing this out because everybody hates it. Some day I'll show you all!