-
Notifications
You must be signed in to change notification settings - Fork 27k
fix(core): improve support for i18n hydration of projected content #56192
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
AndrewKushnir
left a comment
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.
@devknoll thanks for the fix 👍 The code looks great, just left a couple minor comments.
27861d2 to
4dbf906
Compare
ce91198 to
00b2c30
Compare
AndrewKushnir
left a comment
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.
@devknoll looks great, thanks for the fix! I've left a few minor comments, but I think this PR is ready for final review (please switch it to the "Ready for review" state when you get a chance).
00b2c30 to
3e80491
Compare
|
@AndrewKushnir The previous iteration using The new iteration adds a |
packages/core/src/render3/instructions/i18n_icu_container_visitor.ts
Outdated
Show resolved
Hide resolved
3e80491 to
b456f4a
Compare
AndrewKushnir
left a comment
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.
@devknoll thanks for addressing the feedback 👍
When collecting nodes from the DOM for hydration, we need to treat nodes with paths (e.g. content projection) as the new root for all subsequent elements, not just the next one. Additionally, when using content projection it's possible for translated content to become disconnected, e.g. when it doesn't match a selector and there isn't a default. We need to handle such cases by manipulating the disconnected node data associated with hydration as usual.
b456f4a to
29fdd6e
Compare
|
@devknoll would it be ok to proceed with the next steps for this PR (presubmit and merge) or do you plan to perform some additional changes? |
|
@AndrewKushnir No additional changes planned at the moment 👍 |
|
Caretaker note: presubmit is "green", this PR is ready for merge. |
|
This PR was merged into the repository by commit 29ca6d1. The changes were merged into the following branches: main, 18.0.x |
…56192) When collecting nodes from the DOM for hydration, we need to treat nodes with paths (e.g. content projection) as the new root for all subsequent elements, not just the next one. Additionally, when using content projection it's possible for translated content to become disconnected, e.g. when it doesn't match a selector and there isn't a default. We need to handle such cases by manipulating the disconnected node data associated with hydration as usual. PR Close #56192
…ngular#56192) When collecting nodes from the DOM for hydration, we need to treat nodes with paths (e.g. content projection) as the new root for all subsequent elements, not just the next one. Additionally, when using content projection it's possible for translated content to become disconnected, e.g. when it doesn't match a selector and there isn't a default. We need to handle such cases by manipulating the disconnected node data associated with hydration as usual. PR Close angular#56192
|
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. |
…ngular#56192) When collecting nodes from the DOM for hydration, we need to treat nodes with paths (e.g. content projection) as the new root for all subsequent elements, not just the next one. Additionally, when using content projection it's possible for translated content to become disconnected, e.g. when it doesn't match a selector and there isn't a default. We need to handle such cases by manipulating the disconnected node data associated with hydration as usual. PR Close angular#56192
When collecting nodes from the DOM for hydration, we need to treat nodes with paths (e.g. content projection) as the new root for all subsequent elements, not just the next one.
Additionally, when using content projection it's possible for translated content to become disconnected, e.g. when it doesn't match a selector and there isn't a default. We need to handle such cases by manipulating the disconnected node data associated with hydration as usual.
Fixes #56176
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #56176
What is the new behavior?
Does this PR introduce a breaking change?
Other information