Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New 'resolve' option for defining locals for Dialog controller #182

Merged
merged 9 commits into from
Apr 23, 2015
Merged

Conversation

rur
Copy link
Contributor

@rur rur commented Mar 7, 2015

See: Need resolves with the controller. #85

Allow users to define DI factories for dialog controller locals. I have emulated the specification and implementation of the angular-route $routeProvider 'resolve' option. Hence it supports async values via promises.

This feature is purely an enhancement, so it should not change or break the existing API in any way. To gain some confidence about this I have also added a basic testing setup to the project (karma + jasmine). This includes some rudimentary specs but is not comprehensive by any measure.

Here are what the docs look like for the feature:

resolve {Object.<string, function>=}

An optional map of dependencies which should be injected into the controller.
If any of these dependencies are promises, ngDialog will wait for them all to be resolved
or one to be rejected before the controller is instantiated.

If all the promises are resolved successfully, the values of the resolved promises are
injected.

The map object
is:

  • key{string}: a name of a dependency to be injected into the controller.
  • factory - {string|function}: If string then it is an alias for a service.
    Otherwise if function, then it is injected using $injector.invoke and the return
    value is treated as the dependency. If the result is a promise, it is resolved
    before its value is injected into the controller.
ngDialog.open({
    controller: function Ctrl(dep) {/*...*/},
    resolve: {
        dep: function depFactory() {
            return 'dep value';
        }
    }
});

@voronianski
Copy link
Member

@rur please rebase with latest master to have ability to merge this.

@rur
Copy link
Contributor Author

rur commented Mar 28, 2015

@voronianski ok, I'll do that in work on Monday. Are you comfortable with the test setup I have added?

@rur
Copy link
Contributor Author

rur commented Apr 22, 2015

Ok pull request is ready to be merged.

voronianski added a commit that referenced this pull request Apr 23, 2015
New 'resolve' option for defining locals for Dialog controller
@voronianski voronianski merged commit a49848c into likeastore:master Apr 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants