angular-modal-service
angular-modal-service copied to clipboard
dwmkerr
Controller of the dialog should be called before the template directives' links
Your article clearly show an execution order:
var modalController = $controller(controller, inputs);
var modalElementTemplate = angular.element(template);
var linkFn = $compile(modalElementTemplate);
var modalElement = linkFn(modalScope);
which is confirmed by other - articles. So far, so good... The code of your service, up to version 0.2.0, was doing that.
Commit https://github.com/dwmkerr/angular-modal-service/commit/bca78f18ad23d92f9cae07f307b8bcb1d51fc9a6 broke that, wanting to allow injecting $element into the controller, so calling the controller after the link of directives in the template have been called.
This is apparently a violation of a fundamental rule of AngularJS, for a minor, not so much used feature... :smile:
This bites me because I have a directive (from angular-selectize) which takes configuration data at link time, and then creates an instance of Selectize with this configuration. So, it is not a real double-binding and it cannot catch-up the configuration created too late when the controller is ran.
I imagine we can have similar issues with one-time binding (the new 1.3's :: annotation).
Honestly, I have no idea if there is a way to maintain the injection of $element while restoring the proper init order... At worst, you can add an option to the showModal() parameter object, telling $element must be injected, and thus calling the controller later.
Note: I use AngularJS 1.3.15.
+1 Faced the issue too, solved it by moving back the controller back at its original place.
+1 It currently breaks inheritance of scope. Which means, directive/controller inside our template for the modal cannot inherit and access the modal scope as a parent. They have $rootScope instead.
I can see another issue listing the same behavor but reffering to an old codeline: #90 #91 Our fix so far with is indeed to move the $controller line above the $compile (as angular js states it) and since you are passing "inputs" object for injecting stuff, we also set the modalScope to $scope in the input object to allow inheritance to happen.
From L97:
// If we have provided any inputs, pass them to the controller.
if(options.inputs) angular.extend(inputs, options.inputs);
// #### Start Change to make this happening in the right order, with the right parent scope
inputs.$scope = modalScope;
// Create the controller, explicitly specifying the scope to use.
var modalController = $controller(options.controller, inputs);
// #### End Change
// Compile then link the template element, building the actual element.
// Set the $element on the inputs so that it can be injected if required.
var linkFn = $compile(template);
Wow. Been smashing my head against this beast for a couple of hours until i figured this out. This is a major major bug.
@dwmkerr You stated 4 months ago that this was top of the list. Are you going to make a PR? Can I submit one?
@bramski please feel free to make the PR I'm looking at other issues at the moment and am low on time
@dariusjb Thanks for the workaround, when I try this in Angular 1.4.9, I need to explicitly state inputs.$element=undefined or inputs.$element=template before calling $controller, otherwise I'll get
Error: [$injector:unpr] Unknown provider: $elementProvider <- $element
but as this issue says in the beginning, we will be unable to inject $element into controller. I don't know how to do that neither.
@dwmkerr Any plan to fix this?
@dwmkerr I have implemented the reorder via this rejected commit: https://github.com/dwmkerr/angular-modal-service/pull/91/commits
You stated that the fix has other side effects, if we are not using the $element will we be fine or will we run into other issues? Can you share what the side effects of the reorder is
Last time I checked (which was a while ago!) it broke some tests, but I can re-check soon
The way it's implemented now, it's clearly wrong.
Suppose your template contains a directive foo which uses an input value from the controller <foo inputValue="vm.inputValue">. Currently, the directive's controller is constructed first, and it doesn't get any input values since template controller is still not instantiated.
The fix described by @dariusjb and requested by @MattMcLennan ensures the correct order. What is it breaking?
@dwmkerr I've checked what tests are broken, and it's just the single test that explicitly checks whether the $element is injected. So this can be fixed by giving up the $element injection, which is exactly what was done in commits on #90 .
What does $element offer that can't be done without it?
OK all, I'll make this change and bump the semver appropriately just in case, watch this space I'll get started shortly :smile:
Sorry everyone, it's been a while since I've been able to put some time into this project. I'm working through issues at the moment and will endeavour to get this in the next release.