Closed dlongley closed 9 years ago
Hey, thanks very much for the pull request!
So, the reason that I added getInheritedData
is that I wanted to find the first controller of either type, and I couldn't figure out how to do that using require
; do you know if the approach you took with require
works well with arbitrarily nested multitranscludes? At first glance seems like it could accidentally grab the wrong controller.
Everything else seems pretty good, I'll look at merging it in soon. Thanks again!
@zachsnow -- You're welcome!
I added another commit that should result in the selection of the controller that is deepest in the DOM and therefore closest to the target element that will receive the transcluded content. I assumed this is what you meant by first; it would be the first found when walking back up the DOM tree.
I used $element.getInheritedData
, without having to reimplement it; if two controllers are detected, the code checks them against one another to see which is the "child" controller. The child is selected as the one nearest to the target element.
Awesome, way more straightforward than what I had. I'll look to merge this over the weekend. I may break it up into a few commits. Cheers!
On Friday, October 3, 2014, Dave Longley notifications@github.com wrote:
@zachsnow https://github.com/zachsnow -- You're welcome!
I added another commit that should result in the selection of the controller that is deepest in the DOM. I used $element.getInheritedData, without having to reimplement it. If two controllers are detected, the code checks them against one another to see which is the "child" controller. The child is selected as the one nearest to the target element that will receive the transcluded content.
— Reply to this email directly or view it on GitHub https://github.com/zachsnow/ng-multi-transclude/pull/13#issuecomment-57893436 .
I've done some refactoring of this module for our internal use and this is a PR that reflects those changes, in case you're interested.
Here's a fork of the original demo plunker w/the code from this PR pasted into the top of
app.js
. Everything seems to be working AFAICT -- this repo should probably have some test support added at some point.The details for this refactoring are here: