Skip to content

Instantly share code, notes, and snippets.

@pkozlowski-opensource
Forked from ajoslin/problems.md
Created May 15, 2013 16:57
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save pkozlowski-opensource/5585469 to your computer and use it in GitHub Desktop.
Save pkozlowski-opensource/5585469 to your computer and use it in GitHub Desktop.

(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

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