-
Notifications
You must be signed in to change notification settings - Fork 3k
Allow element and attr injections #70
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
Conversation
This is a 'directive controller' therefore needed those injectables
Actually I think that is wrong. This suddenly allow for DOM manipulation from the View Controllers, and IMO that should be discouraged. These controllers are meant to handle what data is brought into the view. This is also in line with the original Angular routing implementation, if you take a look at their "ng-view", you can't access the attr nor the element from the controllers. Animations should be done by options, paired directives or something else, but not by the viewController... I think the easy way to see when a controller is a directive controller is that it would have a one-to-one relation with it's implementation because they are bound together, here you will have thousands of different implementations of a controller. That is my opinion anyway. |
@lmessinger where in core AngularJS is this supported? I don't thing ng-view does this. |
Directives in general do support element/attr injection - see here: http://docs.angularjs.org/guide/directive. But that's right - ngView does not support it, it loses it. Since routers are about transitions, and transitions are about animations, I thought it's important to allow this flexibility. |
@lmessinger Directive controllers does yes, but in my mind your mixing things together here... A directive controller is all knowledgeable about a specific directive, like in a normal Java, C, or .NET app you could imagine having controllers for components like Buttons, TextBoxes, and bigger components like Grids, Calendars etc... Which can manage low level things like "disabled", "border styles" and much more"... but they aren't the wizer about your business model as these are all general purpose, reusable components... That is what directives are to me... A View controller should not care about how things are presented, and therefore not about how the changes of a page is rendered, it should only concern it self about the data it needs to provide to the view (through the scope in this case)... And business level operations... Save Record, Discard, Etc... And I even think that allowing these controllers to manipulate the DOM is directly against the spirit of Angular. It kind of defeats the entire purpose of all those directives as is... Here is a little snippet from http://docs.angularjs.org/guide/dev_guide.mvc.understanding_controller Do not use controllers for:
Take note of the very first bullet |
@jeme thanks - I understand but i beg to differ. Angular rightfully decouples a standard controller from the presentation , but here the View directive is as its name reveals... the presentation. In other words, when users define a ui-view template html, users define the physical layout, the presentation. That's why they might need a way to control that presentation. indeed, in my project's design i use the ui-view solely to define the layout presentation. no data, no {{}} at all. I think that's the reason why Angular team allowed injection of an element to a directive since the directive is the component that deals with the presentation side. On any case I think this is about different interpretations of the 'spirit of Angular' (a new goddess? :) ) so opinions might differ. I suggest to leave it as an option for those who might need it. thanks. |
@lmessinger They indeed need a way to control the presentation in templates, that is why you can use Angular directives inside templates. But it's not the job of the controller. Keep in mind that you can very well end up with an application only consisting of these view controllers and services... If you need to do some animation or something on the entire view that should either be incorporated into the ui-view directive or you could write an entirely new directive for it, alternatively if you wanted to make it extendable an animate provider could be created to add support for this. After all... where are your "standard controllers" anyways if this is not the ones? I don't think it's a fluke they left it out of the Original ng-view... I actually think it was on purpose. After all they mention "ng-view" specifically in the series of Angular MVC http://docs.angularjs.org/guide/dev_guide.mvc.understanding_view The view has knowledge of the controller through Angular directives, such as ngController and ngView, and through bindings of this form: {{someControllerFunction()}}. In these ways, the view can call functions in an associated controller function. |
I agree with @jeme on this. If you think $element/$attr injection is important, I think that's a case that should be made and a discussion to be had for AngularJS in general, i.e. ng-view, not something we add onto ui-view. As far as animation is concerned, there's already #22. I don't know what shape exactly the API is going to take, but in either case for animations you'd want to be able to access the DOM element corresponding to the previous and new content being placed into the ui-view, not just the new one. The patch also changes of the name of the data attribute for the controller; again this is a deviation from ng-view that hasn't been justified in the discussion or comments (as far as I'm aware the Angular convention is to name these after the directive itself; I don't think it makes any sense to use the individual view instance name for it). |
@nfx I appreciate your intentions however this issue had not reached a consensus when you merged it. You must review these issues, (especially those with opposition) and at the very least state your reasons before simply merging them in. |
@ProLoser sure |
The controller in ui-router is really a 'directive controller' as it is tied with $ViewDirective. In Angular.js this type of controller is injectable with $element and $attr, so i've copied the same to here as well.
I needed it to manipulate the $element (specifically, for animation of the ui-view).