-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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
Vcref projection #23272
Vcref projection #23272
Conversation
You can preview d1373e4 at https://pr23272-d1373e4.ngbuilds.io/. |
merge-assistance -> depends on 23189 |
@@ -490,4 +490,7 @@ export function appendProjectedNode( | |||
addRemoveViewFromContainer(node as LContainerNode, views[i], true, null); | |||
} | |||
} | |||
if (node.dynamicLContainerNode) { | |||
node.dynamicLContainerNode.data.renderParent = currentParent as LElementNode; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By looking at this is code, it is not clear to me why currentParent
is a LElementNode
.
Add a comment and/or assert ?
|
||
beforeEach(() => { directiveInstance = null; }); | ||
constructor(public vcref: ViewContainerRef) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Add a comment in the lines of "injecting a ViewContainerRef to create a dynamic blablabla"
.toEqual('<child><div><header vcref="">blah</header><span>bar</span></div></child>'); | ||
}); | ||
|
||
@Component({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to belong in the it()
below.
EDIT: Nope it doesn't. Either leave it here or wrap the Component + the 2 tests in a describe ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see inline comments
d1373e4
to
71b0783
Compare
I rebased this and force-pushed |
You can preview 71b0783 at https://pr23272-71b0783.ngbuilds.io/. |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Only the second commit is to be reviewed, the first one comes from #23189