Closed joeljeske closed 10 years ago
There are no tests for your proposed features.
I am fine with activate
and deactivate
returning promises.
I don't like the callbacks.
There are many whitespace issues with your changes. When submitting a PR, it's best to note and follow existing code formatting conventions.
I've added promises for just activate/deactivate. If you really want callbacks like that (consider broadcasting events instead) you can do something like this:
angular.module('myModal', []).
factory('myModalFactory', function (btfModal) {
return function (opts) {
var modal = btfModal(opts);
// decorate with your methods
modal.close = function () { ... };
return modal;
};
});
For my application, I needed a couple different hooks on the open and close of the modal. I added two configuration options for open and close event handlers. They are called in the activate and deactivate functions respectively. I also added a return to the activate function. It returns a promise that will resolve when deactivate was called with the parameter that deactivate would be called with. The promise would only fail if the HTML could not resolve and therefore the modal would not ever open. I made changes to the readme as you can see.