Skip to content

Instantly share code, notes, and snippets.

@ajoslin
Last active December 16, 2015 18:29
Show Gist options
  • Star 5 You must be signed in to star a gist
  • Fork 1 You must be signed in to fork a gist
  • Save ajoslin/5477996 to your computer and use it in GitHub Desktop.
Save ajoslin/5477996 to your computer and use it in GitHub Desktop.
ui-bootstrap $dialog service refactor discussion

(written by @pawel)

We've got a number of bug reports opened for the $dialog service and while some of them can be fixed by (relatively) simple changes to the existing code-base some other are difficult to solve due to the way current code is structured. Additionally the existing API would benefit from some some simplifications.

This docs aims to list existing issues and propose a new API plus some implementations details that would address existing issues.

As a remainder, docs for the current service are located here: https://github.com/angular-ui/bootstrap/blob/master/src/dialog/README.md

Known issues

API

There current API it is not very intuitive as it requires 2 function calls to open a dialog, ex.:

var d = $dialog.dialog({modalFade: false, resolve: {item: function(){ return angular.copy(item); } }});
d.open('dialogs/item-editor.html', 'EditItemController');

This is not only verbose but also confuses people - it is not evident where to pass which arguments (angular-ui/bootstrap#170). Worse yet, this design contributes to watches leak as the open/close semantics is not clear (angular-ui/bootstrap#232). On top of this the chain of 2 methods make mocking for unit tests more difficult that it could be with just one method call.

Then, the current $dialog service has 2 responsibilities: opening generic dialogs and message boxes. We could split this into 2 separate services (angular-ui/bootstrap#175).

Implementation

There are number of things that are not well supported in the current implementation:

  • opening of several modals / dialogs
  • people can't customize backdrops / modal windows as creation of the coresponding DOM elements is hard-coded in the service's JavaScript code (angular-ui/bootstrap#201, angular-ui/bootstrap#295). Moving backrop and modal elements to thier corresponding templates would allow us to move many CSS-related options from JavaScript code to directive templates.
  • as of today people don't have any means of providing visual feedback when opening of a modal was triggered but the modal itself is not yet open (for example, promises in the resolve section are not yet resolved). Example of such issue: angular-ui/bootstrap#354

(written by @pawel)

Changes proposal

API

First of all we should create 2 separate services - $modal and $dialog. The $modal service would take functionality of the current $dialog().open() while $dialog would have methods like: messageBox, alert, confirmation etc.

$modal API

We could support 2 different syntax options - one where people need to specify many options and another one for typical use-cases. The API for the full version could look like follows:

$modal.open({options});

where the set of supported options would be (general idea would be to mimic AngularJS routes as much as possible):

  • templateUrl / template - content of a modal
  • controller - controller for a modal
  • resolve - resolve section, as in AngularJS routes
  • display - a set of display options:
    • backdrop - Includes a modal-backdrop element. Alternatively, specify static for a backdrop which doesn't close the modal on click.
    • backdropClick - Indicates whether the dialog should be closable by clicking the backdrop area, defaults to true
    • keyboard - Indicates whether the dialog should be closable by hitting the ESC key, defaults to true

The simplified version would take the form:

$modal.open(templateUrl, controller, resolveSection);

$dialog API

The idea here is to use the $modal service to create highier-level, easier to use dialogs, offering the same functionality as the built-in JavaScript ones: http://www.tutorialspoint.com/javascript/javascript_dialog_boxes.htm

Implementation

@ajoslin
Copy link
Author

ajoslin commented Apr 28, 2013

I think it would be better if the simplified version of modal was just:

$modal(templateUrl, controller, resolveSection);

Then we wouldn't have an open function which takes two different sets of options.

Also, we should make sure to include support for ng-animate built in, and make the options friendly to that.

@B3nCr
Copy link

B3nCr commented Sep 11, 2013

Firstly, I'm not a contributor (yet), however, this all seems to be implemented now, it might be worth removing from the WIKI as it's confusing.

@shwetasingh
Copy link

Hi, I recently found out that '$dialog' is missing from Ui-bootstrap- 0.6.0 version. I know that I can potentially achieve the same functionality with $modal service. But I had started using $dialog a couple months back and baked it quite well into our application.

Is there a possibility to have $dialog back with the same functionality in future ui-bootstrap versions?

@leesei
Copy link

leesei commented Dec 10, 2013

Check this comment.

It is not in the official documentation.
Is this the final take on using dialog in the new code base?

@kuasha
Copy link

kuasha commented Nov 4, 2014

Is there a way to get the modalOptions from the modal controller? If we want to reuse a controller for non-modal task we can not inject / resolve - because provider will fail to discover it.

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