Skip to content
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 finding rnode #23193

Conversation

pkozlowski-opensource
Copy link
Member

Only the second commit to be reviewed (the first one is part of #23164)

As we no longer create native (RNode) comment nodes for containers,
we need to execute logic for finding a next sibiling node with RNode
when inserting a view.

The mentioned logic need to be updated for the case of dynamically
created containers (LContainerNode). Indeed, we need to be able to
descend into dynamically inserted views while looking for a RNode.
To achieve this we need to have a pointer from a host LNode to a
dynamically created LContainerNode).

@pkozlowski-opensource pkozlowski-opensource added action: review The PR is still awaiting reviews from at least one requested reviewer target: major This PR is targeted for the next major release comp: ivy labels Apr 5, 2018
@marclaval
Copy link
Contributor

LGTM.

If #23189 goes in, then you might want to merge the tests in the new structure.

@mhevery mhevery added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 5, 2018
@IgorMinar
Copy link
Contributor

g3 presubmiting missing

@vicb
Copy link
Contributor

vicb commented Apr 5, 2018

blocked: needs to be rebased on the first PR

@mhevery
Copy link
Contributor

mhevery commented Apr 5, 2018

As we no longer create native (RNode) comment nodes for containers,
we need to execute logic for finding a next sibiling node with RNode
when inserting a view.

The mentioned logic need to be updated for the case of dynamically
created containers (LContainerNode). Indeed, we need to be able to
descend into dynamically inserted views while looking for a RNode.
To achieve this we need to have a pointer from a host LNode to a
dynamically created LContainerNode).
@mary-poppins
Copy link

You can preview 36eb8d7 at https://pr23193-36eb8d7.ngbuilds.io/.

@mary-poppins
Copy link

You can preview bde3769 at https://pr23193-bde3769.ngbuilds.io/.

@vicb
Copy link
Contributor

vicb commented Apr 5, 2018

@IgorMinar IgorMinar closed this in d80e930 Apr 5, 2018
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants