-
Notifications
You must be signed in to change notification settings - Fork 6.7k
feat(modal): add support for bindToController #3965
Conversation
- Add `bindToController` support in modal options
@@ -8,6 +8,7 @@ The `$modal` service has only one method: `open(options)` where available option | |||
* `scope` - a scope instance to be used for the modal's content (actually the `$modal` service is going to create a child scope of a provided scope). Defaults to `$rootScope` | |||
* `controller` - a controller for a modal instance - it can initialize scope used by modal. Accepts the "controller-as" syntax in the form 'SomeCtrl as myctrl'; can be injected with `$modalInstance` | |||
* `controllerAs` - an alternative to the controller-as syntax, matching the API of directive definitions. Requires the `controller` option to be provided as well | |||
* `bindToController` - when used with `controllerAs` & set to `true`, it will bind the controller properties onto the `$scope` directly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onto the $scope
or onto this.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As it says, $scope
. If you see the implementation, it becomes instantly clear. This is similar to how bindToController
works in Angular for directives with isolate scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, you're right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that you understood bindToController
in the other way around:
https://docs.angularjs.org/api/ng/service/$compile
bindToController: When an isolate scope is used for a component (see above), and controllerAs is used, bindToController: true will allow a component to have its properties bound to the controller, rather than to scope.
Bind to controller enforces the use of controllerAs, and it publishes the controller into the scope as it happens since 1.2. The new point is that all bindings to the directive are made towards the controller instead of towards the scope, but the scope does not dissappear, and you always use in template: ctrl.property.
The problem in angular 1.X is that you need to use the dot notation because you have a high risk (specilly with you work with non expert programmers) to mess it up with double bindings and nested scopes, for instance:
Value: {{vm.value}}
<div ng-if="vm.isInputActive()">
<input ng-model="vm.value">
</div>
vs.
Value: {{value}}
<div ng-if="vm.isInputActive()">
<input ng-model="value">
</div>
Which will not work as expected according the current implementation proposed in this thread: it will save the value in the scope created by ngIf but not in the directive context.
So, one of two:
-
bindToController does not make sense at all here, there are no bindings to the modal, so better to remove it (my recommendation)
-
ok, we want to bind controller properties to the scope, although it is dangerous, but we should name another name like: controllerPropertiesInScope, to avoid confusions with angular meaning
And according the presented implementation, it does not uses the controller instance at all, it is just a temporary object which instance reference is lost in favor to scope:
angular.extend(modalScope, ctrlInstance);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check #4054 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
It happened to me looking for ways to make "mastermodals" that I can accomodate for many uses. Ex:
- A generic PurchaseModal which is extended by MacaroonsPurchaseModal in which you for example need to request remote macaroons colors and prices and pass it to the directive or whatever to choose macaroon, select a price and go ahead with purchase.
The main problem is how to extend specific existing controller with some nice locals. In such case something like but different to bindToController would be useful. For example binding resolve results to controller.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please continue this conversation on #4054.
@wesleycho LGTM |
bindToController
support in modal optionsThis implements #3404.