$modal breaks forms b/c of transclusion

[boneskull]

Say I instantiate a modal:

$modal.open({
  controller: 'MyCtrl'
});

In my modal, I have a form:

<form name="myForm">...</form>

Because of transclusion, myForm is not available on MyCtrl's scope and I can't check for $valid, for example. A workaround I've found is to simply put an ng-controller="MyCtrl" within my modal template itself, and do this:

$modal.open({
  controller: function($scope, $modalInstance) {
    $scope.$modalInstance = $modalInstance;
  }
});

...and now I can get to the instance from within MyCtrl.

This is not ideal, but I'm not sure of what the fix should be. Instead of transclusion, would it be a good idea to actually insert an ngController directive within the template/modal/window.html template itself then say, ngTransclude within a child tag? Any better way to tackle this problem?

thanks
Chris

[georgiosd]

Chris,

I do exactly the same - have forms in my modals but I'm checking $valid just fine. Can you do a plunker?

[damrbaby]

Just ran into the same issue here... wondering if it's a bug in angular 1.2.0-rc.2

[boneskull]

I'll try to plunk/fiddle this in the next few days.

[AGiorgetti]

Same issue here, I'm going to try create a spiked version of the new $modal that do not use ngTransclude. That's the cause of the creation of the 'unwanted' additional scope here. The scope that gets the validation injected is the one created by the transclusion, and it's a child of you modal controller's scope.

[VaclavObornik]

Same issue:
http://plnkr.co/edit/TN37ED4E83XfsI4GrQhA?p=preview

[AGiorgetti]

OK, it seems I have two working patches for this:
1- I changed the way the scopes are created and nested (it uses two different scopes: one to manage the 'dialog specific' properties (animate, index, window-class) and a second tied to the controler of the modal content
2- this one just use one single scope (as previous implementation) and enrich it with an object (named $$modal) which contains the modal specific properties.

both of them do not use the ngTransclude directive in the template, they just insert the modal html content inside the template frame using angular DOM manipulation.

My bro is testing them, if they work well (and if I find how to post patches) I'll upload them.
I don't like the idea of positing the big bunch of code right here.

[AGiorgetti]

This is the 'option 2' implementation:
the main differences are in the 'modal-window' directive and in the '$modalStack.open' function.
Things can be improved to use templates again for the modal frame.

Sorry for the big bunch of code, but I don't know how to submit a patch properly atm.

Update: fixed the code markdown

angular.module('ui.bootstrap.modal', [])

/**
 * A helper, internal data structure that acts as a map but also allows getting / removing
 * elements in the LIFO order
 */
  .factory('$$stackedMap', function () {
      return {
          createNew: function () {
              var stack = [];

              return {
                  add: function (key, value) {
                      stack.push({
                          key: key,
                          value: value
                      });
                  },
                  get: function (key) {
                      for (var i = 0; i < stack.length; i++) {
                          if (key == stack[i].key) {
                              return stack[i];
                          }
                      }
                  },
                  keys: function () {
                      var keys = [];
                      for (var i = 0; i < stack.length; i++) {
                          keys.push(stack[i].key);
                      }
                      return keys;
                  },
                  top: function () {
                      return stack[stack.length - 1];
                  },
                  remove: function (key) {
                      var idx = -1;
                      for (var i = 0; i < stack.length; i++) {
                          if (key == stack[i].key) {
                              idx = i;
                              break;
                          }
                      }
                      return stack.splice(idx, 1)[0];
                  },
                  removeTop: function () {
                      return stack.splice(stack.length - 1, 1)[0];
                  },
                  length: function () {
                      return stack.length;
                  }
              };
          }
      };
  })

/**
 * A helper directive for the $modal service. It creates a backdrop element.
 */
  .directive('modalBackdrop', ['$modalStack', '$timeout', function ($modalStack, $timeout) {
      return {
          restrict: 'EA',
          replace: true,
          templateUrl: 'template/modal/backdrop.html',
          link: function (scope, element, attrs) {

              //trigger CSS transitions
              $timeout(function () {
                  scope.animate = true;
              });

              scope.close = function (evt) {
                  var modal = $modalStack.getTop();
                  if (modal && modal.value.backdrop && modal.value.backdrop != 'static') {
                      evt.preventDefault();
                      evt.stopPropagation();
                      $modalStack.dismiss(modal.key, 'backdrop click');
                  }
              };
          }
      };
  }])

  .directive('modalWindow', ['$timeout', function ($timeout) {
      return {
          restrict: 'EA',
          //scope: {
          //  index: '@'
          //},
          replace: true,
          //transclude: true,
          //templateUrl: 'template/modal/window.html',
          link: function (scope, element, attrs) {
              scope.$$modal = {};
              scope.$$modal.index = attrs.index || 0;
              scope.$$modal.windowClass = attrs.windowClass || '';

              //trigger CSS transitions
              $timeout(function () {
                  scope.$$modal.animate = true;
              });
          }
      };
  }])

  .factory('$modalStack', ['$document', '$compile', '$rootScope', '$$stackedMap',
    function ($document, $compile, $rootScope, $$stackedMap) {

        var backdropjqLiteEl, backdropDomEl;
        var backdropScope = $rootScope.$new(true);
        var body = $document.find('body').eq(0);
        var openedWindows = $$stackedMap.createNew();
        var $modalStack = {};

        function backdropIndex() {
            var topBackdropIndex = -1;
            var opened = openedWindows.keys();
            for (var i = 0; i < opened.length; i++) {
                if (openedWindows.get(opened[i]).value.backdrop) {
                    topBackdropIndex = i;
                }
            }
            return topBackdropIndex;
        }

        $rootScope.$watch(backdropIndex, function (newBackdropIndex) {
            backdropScope.index = newBackdropIndex;
        });

        function removeModalWindow(modalInstance) {

            var modalWindow = openedWindows.get(modalInstance).value;

            //clean up the stack
            openedWindows.remove(modalInstance);

            //remove window DOM element
            modalWindow.modalDomEl.remove();

            //remove backdrop if no longer needed
            if (backdropIndex() == -1) {
                backdropDomEl.remove();
                backdropDomEl = undefined;
            }

            //destroy scope
            modalWindow.modalScope.$destroy();
        }

        $document.bind('keydown', function (evt) {
            var modal;

            if (evt.which === 27) {
                modal = openedWindows.top();
                if (modal && modal.value.keyboard) {
                    $rootScope.$apply(function () {
                        $modalStack.dismiss(modal.key);
                    });
                }
            }
        });

        $modalStack.open = function (modalInstance, modal) {

            openedWindows.add(modalInstance, {
                deferred: modal.deferred,
                modalScope: modal.scope,
                backdrop: modal.backdrop,
                keyboard: modal.keyboard
            });

            var angularDomEl = angular.element("<div modal-window class=\"modal fade {{ $$modal.windowClass }}\" ng-class=\"{in: $$modal.animate}\" ng-style=\"{'z-index': 1050 + $$modal.index*10}\"></div>");
            angularDomEl.attr('window-class', modal.windowClass);
            angularDomEl.attr('index', openedWindows.length() - 1);
            angularDomEl.html(modal.content);
            var modalDomEl = $compile(angularDomEl)(modal.scope);

            openedWindows.top().value.modalDomEl = modalDomEl;
            body.append(modalDomEl);

            if (backdropIndex() >= 0 && !backdropDomEl) {
                backdropjqLiteEl = angular.element('<div modal-backdrop></div>');
                backdropDomEl = $compile(backdropjqLiteEl)(backdropScope);
                body.append(backdropDomEl);
            }
        };

        $modalStack.close = function (modalInstance, result) {
            var modal = openedWindows.get(modalInstance);
            if (modal) {
                modal.value.deferred.resolve(result);
                removeModalWindow(modalInstance);
            }
        };

        $modalStack.dismiss = function (modalInstance, reason) {
            var modalWindow = openedWindows.get(modalInstance).value;
            if (modalWindow) {
                modalWindow.deferred.reject(reason);
                removeModalWindow(modalInstance);
            }
        };

        $modalStack.getTop = function () {
            return openedWindows.top();
        };

        return $modalStack;
    }])

  .provider('$modal', function () {

      var $modalProvider = {
          options: {
              backdrop: true, //can be also false or 'static'
              keyboard: true
          },
          $get: ['$injector', '$rootScope', '$q', '$http', '$templateCache', '$controller', '$modalStack',
            function ($injector, $rootScope, $q, $http, $templateCache, $controller, $modalStack) {

                var $modal = {};

                function getTemplatePromise(options) {
                    return options.template ? $q.when(options.template) :
                      $http.get(options.templateUrl, { cache: $templateCache }).then(function (result) {
                          return result.data;
                      });
                }

                function getResolvePromises(resolves) {
                    var promisesArr = [];
                    angular.forEach(resolves, function (value, key) {
                        if (angular.isFunction(value) || angular.isArray(value)) {
                            promisesArr.push($q.when($injector.invoke(value)));
                        }
                    });
                    return promisesArr;
                }

                $modal.open = function (modalOptions) {

                    var modalResultDeferred = $q.defer();
                    var modalOpenedDeferred = $q.defer();

                    //prepare an instance of a modal to be injected into controllers and returned to a caller
                    var modalInstance = {
                        result: modalResultDeferred.promise,
                        opened: modalOpenedDeferred.promise,
                        close: function (result) {
                            $modalStack.close(modalInstance, result);
                        },
                        dismiss: function (reason) {
                            $modalStack.dismiss(modalInstance, reason);
                        }
                    };

                    //merge and clean up options
                    modalOptions = angular.extend({}, $modalProvider.options, modalOptions);
                    modalOptions.resolve = modalOptions.resolve || {};

                    //verify options
                    if (!modalOptions.template && !modalOptions.templateUrl) {
                        throw new Error('One of template or templateUrl options is required.');
                    }

                    var templateAndResolvePromise =
                      $q.all([getTemplatePromise(modalOptions)].concat(getResolvePromises(modalOptions.resolve)));


                    templateAndResolvePromise.then(function resolveSuccess(tplAndVars) {

                        var modalScope = (modalOptions.scope || $rootScope).$new();
                        modalScope.$close = modalInstance.close;
                        modalScope.$dismiss = modalInstance.dismiss;

                        var ctrlInstance, ctrlLocals = {};
                        var resolveIter = 1;

                        //controllers
                        if (modalOptions.controller) {
                            ctrlLocals.$scope = modalScope;
                            ctrlLocals.$modalInstance = modalInstance;
                            angular.forEach(modalOptions.resolve, function (value, key) {
                                ctrlLocals[key] = tplAndVars[resolveIter++];
                            });

                            ctrlInstance = $controller(modalOptions.controller, ctrlLocals);
                        }

                        $modalStack.open(modalInstance, {
                            scope: modalScope,
                            deferred: modalResultDeferred,
                            content: tplAndVars[0],
                            backdrop: modalOptions.backdrop,
                            keyboard: modalOptions.keyboard,
                            windowClass: modalOptions.windowClass
                        });

                    }, function resolveError(reason) {
                        modalResultDeferred.reject(reason);
                    });

                    templateAndResolvePromise.then(function () {
                        modalOpenedDeferred.resolve(true);
                    }, function () {
                        modalOpenedDeferred.reject(false);
                    });

                    return modalInstance;
                };

                return $modal;
            }]
      };

      return $modalProvider;
  });

[tech-no-logical]

I've hit this issue as well. trying AGiorgetti's version now, seems to work. thanks !

[boneskull]

Thanks for taking a look.

[georgiosd]

This is really strange. I have several models with ng-model bindings and they all work fine. With 0.6 and NG 1.2 RC1

[tech-no-logical]

this plunk demonstrates my problem :

http://plnkr.co/edit/A0Q8Nh?p=preview

notice how the ng-change doesn't see the 'correct' value when typing in the form-field. I've tried various configurations, with and without passing a $scope to the modal instance, I haven't gotton it to work

[damrbaby]

@georgiosd the ng-model bindings are working for me, but accessing the form by name from the scope (e.g. $scope.myForm) doesn't work.

[ProLoser]

You guys are aware that this ENTIRE bug is just because there's a superfluous childscope being created right?

In other words, if you had ng-model="directScopeProperty" it must be changed to ng-model="$parent.directScopeProperty". If you had ng-model="object.property" then it will continue working.

This is still a bug that should be addressed however, but again, it's just because there's 1 extra child scope being created for some reason.

@AGiorgetti I believe giant pastes like that aren't really helpful in comments.

[pkozlowski-opensource]

Thnx for this comment @ProLoser :-) I will update this issue with more explanations on what is going on and work-arround later today. And yes, planning on fixing it as I understand that it might be confusing. Just still pondering different options.

[AGiorgetti]

yes @ProLoser I know that, and I also said sorry in advance, I apologize again. Yes, that's the problem and it was spotted by @boneskull (the extra scope was created by the transclusion). The implementation I posted there get rid of the translusion and does not create the extra scope. It still need to be improved by using the template cache again for the modal frame (instead of inlining it as I did in the quick fix).

[lucassus]

I have similar issue.
Inside a dialog template I have a form <form name="editForm" ...> but inside the controller $scope.editForm is undefined. Here is my example yakworks/angle-grinder@a3a1bc4#L4R28

[acornejo]

Is there anyone working on a fix? I'll admit that it is a bit dirty to inserting ng-style/ng-class styles directly into the dom, but @AGiorgetti code seems to do the trick.

PS: Thinking about it some more, I do think that modal-dialog and modal-content should not be inserted by transclusion in directives, but should be part of the user template (i.e. templateUrl parameter in modal). So this is extra points for that solution.

[BastianVoigt]

I found it hard to see what @AGiorgetti changed, so I made a fork.

BastianVoigt@ddc275a

The change breaks some tests though:

Chrome 30.0 (Linux) $modal option by option custom window classes should support additional window class as string FAILED
Expected '

With custom window class

' to have class 'additional'.

[BastianVoigt]

The fix does not work for me :-(

At least not the backport to 0.6. The modal is not displayed at all.

[boneskull]

How about the workaround?

On Sep 23, 2013, at 5:30 AM, Bastian Voigt notifications@github.com wrote:

The fix does not work for me :-(

At least not the backport to 0.6. The modal is not displayed at all.

—
Reply to this email directly or view it on GitHub.

[matt-way]

You don't have to use .$modelValue if you also set your ng-model references to the container. eg:

<input type="text" ng-model="container.textValue" name="textValue">

This way you access the form information at container.myForm.textValue, and the model value at container.textValue. Nice and easy.

This all comes back to the correct use of scope inheritance in angular. Have a read of this: http://jimhoskins.com/2012/12/14/nested-scopes-in-angularjs.html

[kimardenmiller]

@matt-way , thanks so much! I learned a ton in this exercise: So it's all about the read before write when child scopes set up objects and write to them. By setting an object on the parent scope with the same name as the model, the (child scope) template form uses the parent's object by that same name to hang the form on, and in so doing one is able to fake the model back to the parent scope. Very clever. The Hoskins link above explains it very well.

Thanks for the lesson!
/kim

[jeffgabhart]

we could require people to systematically provide the whole modal window markup for each modal - this would be really bad as it would mean repeating code over and over again

Can someone help me understand why this is so much worse than the effort it takes to understand and workaround the scope issue?

[NickBarton]

Total cludge solution http://plnkr.co/edit/oiOuwSGcJDzkPBIdVURv?p=preview

[kimardenmiller]

One approach that fixes this scope issue (and has helped us avoid other similar issues) is to link the model to a separate service which does not rely on the scope stack.

So in the template (in CoffeeScript):

Which links to a service:

[pgaertig]

Clever solution @matt-way. I improved it further:

<form name="$parent.form">

And it just works! I have got form entry in modal $scope and I don't have to modify my form markup inside. The only thing I don't like about this is the hacky name value.

[brianmarco]

Tx @pgaertig your tip worked great for me!

[plentz]

@AGiorgetti you closed this referencing #972, and then @pkozlowski-opensource closed it referencing #981 which I don't think it's related to this one and this one should be re-open. Am I wrong? 😓

[AGiorgetti]

Hi, i didn't close anything, the reference at #972 was made because the problem reported there (a typeahead malfunction inside a modal) was related to the issues caused by the use of transclusion in the modal dialog. As of now I'm using a custom made version of the modal dialog which does not use transclusion at all, I'm just doing it manually playing with the templates (as I did in the past with the 'patched' version posted above at the time).

[pkozlowski-opensource]

@plentz we've got a work-around in master 0b31e86, not released yet, so no need to re-open anything...

[plentz]

@AGiorgetti thanks!

@pkozlowski-opensource coool! thanks a lot!

[kulicuu]

boneskull thx for the brilliant workaround !

[LeleDev]

I'm still having the same issue... 0b31e86 hasn't been released yet?

[LeleDev]

Ok, I've compiled the master version with grunt and now it works. Hope that ui-boostrap 0.12 will be released soon!

[KKrisu]

I just want to confirm - After upgrading ui-bootstrap to 0.12 it works correctly!

[devendher]

check it this plunker
might be it will helpful and i think it is complete solution that really what do you want http://plnkr.co/edit/ZM3UUt16P3cRainp0LK3?p=preview

[solerman]

I was fighting over ng-model/ng-form myself this days. My problem was a bit different but also cased by the way scopes/controllers are handled. Basically I have a directive to watch from the parent form for changes and warn the user if he's about to leave the page without saving and that wasnt working if you only changed the modal's values. The workaround:

In the parent controller inject your form

   modalInstance = $modal.open({
        resolve: {
          containerForm: function () {
            return $scope.containerForm;
          }
        }
      });

And in the modal's template set the parent form before any control has the chance to initialize

    <ng-form name="$parent.modalForm">
        <any ng-init="$parent.modalForm.$$parentForm = containerForm"/>

[icfantv]

@solerman, I should point out that using ng-init outside the very limited, documented areas is not correct.

Also, doing this with your form is not correct. You should be returning the model from the form in the modal as a result of the $promise from the modal and then, if needed, update your form's model with that data.

[solerman]

The problem is, with bindings the model is already updated in the model. To do what you say I would need to clone the (very complex) model, provide it to the modal and then hand-copy all the values back. For every modal.

[icfantv]

@solerman, that sounds like an issue with design and UX rather than our library. do you need to send the entire model to the modal or can you just send the bit that the model intends to edit?

For the record, this is a very common paradigm - say, having a grid of data and then editing a grid item in a modal.

[solerman]

@icfantv the parent its a dashboard and the modal its just for a single dashboard item. There is a lot of stuff to configure in the item, I ain't sending the entire dashboard but just a single item. Think a bar chart, all the series, axis titles, colors, ranges, etc.

[icfantv]

@solerman, without knowing exactly what you're using the modal to configure, it's hard to make a recommendation. It sounds like you're using the modal to configure chart options, but that's just a guess.

I can certainly appreciate not wanting to copy a "huge" model object. The only other options I can think of are:

1. Don't copy the model, rather, send it live and use a cancel to restore the model, or
2. Don't use a modal. Rather, use a physically separate details page. This would depend on the amount of data that needs to be edited.

I still think this boils down to a design and UX issue and not the library. We do get a lot of issues surrounding the use of parent forms wrapping "compartmentalized" forms (tabs, accordions, etc...). There are ways of working around our replace: true directives, including an example in our tabs demo - but in general, I'd argue that doing this is a somewhat bad approach. Part of that is due to how angular form validation was architected for 1.x. I don't know if or how this changes for ng2.