Make sure we only remove view title from the directive that set it#14
Make sure we only remove view title from the directive that set it#14vstene wants to merge 2 commits intoapparentlymart:masterfrom
Conversation
| }); | ||
|
|
||
| mod.directive('viewTitle', ['$rootScope', 'viewTitleDataService', function ($rootScope, viewTitleDataService) { | ||
| return { |
There was a problem hiding this comment.
Rather than having the separate viewTitleDataService service, could we instead just have a local variable here inside this factory function, which would then be part of the closure for the link function and shared between all instances? I'm actually a bit rusty on Angular since I've not used it in six months or so, but IIRC this factory function is called once per injector, meaning that any local variables it creates should behave effectively as singletons within a given injector.
This is a relatively minor thing, but this way we'd prevent other directives and services from accessing this private data structure, I think is worth doing if it's easy.
There was a problem hiding this comment.
I was avoiding the local variable on purpose. Ideally the two directives should be splitted into two files and be merged during a build process. In this case you'll have no access to a local variable. But yes, having a publicly accessible service is also not ideal.
|
Thanks for this! It's a great improvement to the robustness of the title handling. I just had a design-ish question inline. If you don't have time to work on this further then no worries... just thinking about what is the best way to represent this internal state. |
2 participants
When doing a page change in
ui-router, thescope.on('$destroy')is called after the new view title directive has been initialized, resulting that there is no view title present.