Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

$modal does not provide proper scope to templates. #2110

Closed
monokrome opened this issue Apr 24, 2014 · 29 comments
Closed

$modal does not provide proper scope to templates. #2110

monokrome opened this issue Apr 24, 2014 · 29 comments

Comments

@monokrome
Copy link

When using templateUrl, the resulting template will not have the same scope as the instance controller. Instead, it is in the scope as passed to the scope option on $modal.open.

This is an issue, because it means that there is effectively no method by which to get the modal's scope without resolving the modal.

@paulpflug
Copy link

Same problem here. Do you have any sort of workaround?
I want to make a Login modal but the input values are put into a childscope of the controller scope and I can't access them.

@paulpflug
Copy link

I digged a bit and found a related issue. the Modal scope is not cleaned up on close.
Only the childscope of the controller gets cleaned up.
I am still clueless what the exact problem is.

@paulpflug
Copy link

I found a way to workaround:
doesn't work:

// in controller
$scope.username = ""

// in template
<input ng-model="username"></input>

works:

// in controller
$scope.user.username = ""

// in template
<input ng-model="user.username"></input>

When using a object in ng-model, it moves up the scope-chain. The problem persists nevertheless.

@monokrome
Copy link
Author

I had to work around it by using a nested model in the inherited scope. However, this is not ideal and I think that it's important to fix in this library when possible.

This is basically what you have done, but my use case requires maintaining a lot more login state at a higher scope than preferable.

@callado4
Copy link

callado4 commented May 1, 2014

This seems to be just how scopes in AngularJS work. See Understanding Scopes, especially paragraph 2 and 3.

@pkozlowski-opensource
Copy link
Member

@callado4 is right here, you guys need to realise that there is a scope between a controller and a template created by AngularJS transclusion. While I know it might be confusing, the main issue is that transclusion is creating a new scope in the current version of AngularJS. We could think of an implementation of a modal window that doesn't use transclusion but it would have number of other problems, I'm afraid.

@monokrome @paulpflug when you understand how scopes work it is pretty easy to handle things, please send a plunk if you've got cases that that give you a headache....

@monokrome
Copy link
Author

The issue is that the scope of the template is not provided anywhere. I understand how it works, but suggesting that the library just creates a scope which is impossible to get a reference to as an intentional design choice for a modal library is a really poor solution.

I guess the "right" solution is to create yet another controller to bind into the modal's template. That means that it requires 3 controllers to get a modal to work properly if you want to expose the ngModel values without using resolve or dismiss, so that's too bad.

@pkozlowski-opensource
Copy link
Member

@monokrome yes, it is a design choice of both AngularJS and this library. As bad as it might sound the alternatives are even worse... Having said this you've got reference, just use $parent. But more importantly - why do you need to have access to this scope. What is the use-case you are trying to solve? Please send a plunker with your use-case, will be easier to discuss.

@chrisirhc
Copy link
Contributor

@monokrome , I'm not sure what you mean. Here's a simple ngModel example: http://plnkr.co/edit/ZPoOJbVWABHrsIQMWN0y?p=preview

@paulpflug
Copy link

I think this behavior it is not design choice of AngularJS
When using ngRoute for example, this works:

$routeProvider.when "/",
      templateUrl: "main.html"
      controller: 'MainCtrl'

The given controller is directly bound to the template.
For modals however

$modal.open 
      templateUrl: "modal.html"
      controller: "ModalCtrl" 

The controller is NOT directly bound to the template.
I understand why and found a way to workaround, but that took me several hours to figure out.. as it doesn't behave like angular

beside that: closing the modal doesn't clean up all the used scopes.
For each modal two scopes persist after closing.

@monokrome
Copy link
Author

The suggestion which is being provided here is that the separation provided by the transclude is intended by Angular. The suggestion that I'm trying to make is a higher level suggestion.

Just because transclude works this way does not mean that transclude is the proper solution when a user is being given an interface which inhibits them or provides common confusion.

I am happy to make an example use case in plunkr. Is there a specific template that is recommended for using $modal with it?

@chrisirhc
Copy link
Contributor

@monokrome , see the Plunker that I've set up, you can fork that to make your Plunker.
Are you suggesting that we use something besides transclude? There's actually an implementation detail that prevents us from doing this in any other way. It's a little difficult to explain but let me try..

  • When a template is linked to a scope, all the Angular expressions that it contains ({{ things }}) are added as watchers into that scope.

In a modal, we're compiling and linking the template to the scope whenever the modal is displayed. Whenever the modal is dismissed, the modal's contents (the template that was linked) gets destroyed. Hence, when the modal is opened again, it needs to be compiled and linked again. Note that when the template is destroyed, the watchers are not removed from the scope.

Thus, there is then a watcher leak on the scope. The way to get around this is to create a new child scope for any template that is going destroyed and re-created, so that the scope itself can be destroyed along with it. If we don't use transclusion or a child scope, that means that the scope that you provide needs to be destroyed along with the template. I assume that's not desirable.

Alternatively, if we do keep the template when the modal is dismissed (so that later when it's displayed, it doesn't have to be re-linked again), then we have a DOM leak. No go as well.

Also note that Angular.js has an issue talking about how transclude's sibling scope doesn't work as expected: angular/angular.js#5489

@paulpflug , scope cleanup is a separate issue that may have been already addressed on the master.

@paulpflug
Copy link

wow. thanks for that response @chrisirhc !

why we don't use the proposed workaround for the transclude?

ui.directive('box', function() {
    return {
        restrict: 'E',
        transclude: true,
        template: '<div/>',
        replace: true,
        scope: {},
        link: function(scope, element, attrs, transclude) {
            transclude(scope.$parent, function(content) {
                element.append(content);
            });
        }
    };
});

@chrisirhc
Copy link
Contributor

As I mentioned in my comment, that causes a watcher leak. Any directive that adds watchers to the scope.$parent doesn't get cleaned up when the scope is destroyed because they're now on the scope's parent instead of being on the scope itself.

@paulpflug
Copy link

but when we destroy the parent scope on modal close, which should be done anyway?

@chrisirhc
Copy link
Contributor

I'm not sure what you mean, we're not destroying the parent scope on modal close. By default, the modal runs as a child scope of the root scope, are you saying we should be destroying the root scope?

@paulpflug
Copy link

no of course not.
in the version I am using 4 scopes are created for each modal

rootScope
>Scope in which I create the modal
>>ModalScope
>>>TranscludeScope
>>>Some other Scope I don't understand
>BackdropScope

only the backdrop and the TranscludeScope are destroyed on close.
the aim of the workaround is to unify TranscludeScope with ModalScope
The ModalScope equals the scope I pass to the Modal and is linked to the controller.
The TranscludeScope however is linked to the template.
Tell me If I'm wrong?

@chrisirhc
Copy link
Contributor

Note that the scope that you pass to the modal (ModalScope, in your example) is used as the parent of the scope created for the modal (parent of TranscludeScope). In this case, you could create the modal by passing in the scope that you create the modal with.

There's no need to create the modal scope, that's handled by the $modal service.

We can't unify ModalScope with TranscludeScope because the $modal can't destroy a scope that it didn't create, it just has too many possible side effects. How would the $modal service know if the ModalScope (as you mentioned above), isn't used by any other directive or service?

Any scope that you create must be handled by you, if you create a scope, pass it to the modal, it'll still be there when the modal is dismissed, you have to destroy it on your own. And the only way to use that scope that you passed in, without leaking watchers or DOM is to create a child scope from that scope that you passed in and then destroy it when done.

@paulpflug
Copy link

Yeah I understand that.
Here the modalScope is created as a child scope of the provided scope.
Here it is bound to the modal-window directive.
The nested ng-transclude directive, however, creates an additional child scope of the modalScope here
so we have
provided scope > modalScope(modal-window) > transclude scope

we need
provided scope > modalScope(modal-window)

the additional transclude scope makes no sense? all watchers are put on modalScope and on close modalScope can be destroyed (currently only transclude scope is destroyed)

this doesn't affect the provided scope at all

@chrisirhc
Copy link
Contributor

The additional transclude scope is for the modal's content. It isolates conflicts with the scope variables needed to on the modal template itself. See https://github.com/angular-ui/bootstrap/blob/master/template/modal/window.html . The animate, index, close need to belong to a scope that the modal template can access but those shouldn't be in the same scope as the modal's content because otherwise, there's no telling when a user might use a variable with the same name and break the modal. Or worse, the modal changing the user's scope variables.

@pkozlowski-opensource
Copy link
Member

@chrisirhc @paulpflug thnx so much for looking into this. Actually I think that @paulpflug might have a point here, since I believe that scopes hierarchy looks like follows:

So what we need to do is to "collapse" 2 content scopes: 0.3 and 0.3.1 from the above diagram. In fact 0.3.1 shouldn't be created at all when angular/angular.js#5489 is solved. But this is not where we stand so far, so I simply wasn't sure how we could merge 0.3 and 0.3.1 and still use transclusion. But indeed, using a transclusion function we might be able to do this.

I've re-opened the issue to try it out but won't get to it till the weekend. If you guys got a bit of time before to take a stab at it, any help would be much appreciated.

@paulpflug
Copy link

made a PR which works for me and Karma.. but pls review ;)

@joshribakoff
Copy link

I can help. In my example, my controller initializes a variable. The user can edit in the view, and then press a "preview" button which should show the edited value within the modal.

Here's what you guys are trying:

// this is in MyCtrl
$scope.var = 'foo';
$scope.preview = function() {
   modalInstance = $modal.open({
            templateUrl:'preview.html',
            controller: 'MyCtrl',

If the user edits the value of var to 'bar', then presses my 'preview' button... it instanties a 2nd instance of MyCtrl with it's own child scope, and [re]-runs the MyCntrl against the child scope, therefore the modal shows the [re]-initialized value of "foo", rather than the value the user gave it of "bar". No good.

The solution is extremely simple:

$scope.var = 'foo';
$scope.preview = function() {
   modalInstance = $modal.open({
            templateUrl:'iframe-preview.html',
            scope: $scope

Here I'm simply asking for the $scope to be bound to my template. Inside the modal the value of var is correctly shown to "bar" after the user edited it, instead of it showing the old value of "foo" that the controller initialized.

You guys are just confusing yourselves. Just pass in the values & functions to the modal with scope. Do not give the modal a controller for a simple use case of displaying some values. There should be a "modal without controller" example in the documentation. The existing example makes it seem like a controller is mandatory, as evidenced by all the users complaining here that they do not want a controller.

@monokrome
Copy link
Author

👍 @joshribakoff

@patrick-fls
Copy link

The problem I hit - and don't know how to resolve, is that I use ControllerAs syntax throughout my application.
while this works

$scope.var = 'foo';
$scope.preview = function() {
   modalInstance = $modal.open({
            templateUrl:'iframe-preview.html',
            scope: $scope

It would be nice to be able to do this too:

var vm = this;
vm.var = 'foo';
vm.preview = function() {
   modalInstance = $modal.open({
            templateUrl:'iframe-preview.html',
            controllerAs: vm

to be able to pass the current instance of our controller.

@Abrissirba
Copy link

I'm having the same problem as patrick-fls with how to pass arguments to the modal controller when using the controllerAs syntax.

@chrisirhc
Copy link
Contributor

@Abrissirba @patrick-fls That's not how controllerAs works. It's meant to be an alias for the controller that you assign to the modal. See description of controllerAs on: https://docs.angularjs.org/api/ngRoute/provider/$routeProvider if the description on our documentation wasn't adequate.

@spock123
Copy link

spock123 commented Apr 9, 2015

Hi guys,

  • as I see it, when using controllerAs, there is no $scope injected into the controller.
    Any of you know how to inject a scope to a modal service from a controller that doesn't have $scope injected in the first place?

It feels a bit akward having to inject the $scope into a controller JUST to be able to put a scope on the dialog... I'm not feeling it :)

Also: when using ng-controller inside the modal template, how am I supposed to listen to events from the modal? I'm trying to figure out when the modal has been closed, but since I'm not injecting a scope to the modal service, I have no idea how to capture events from the modal...

@MistyShrivastava
Copy link

@joshribakoff thanks a lot... your simple solution solved my issue.
It literally saved hours of wild goose chase..

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.