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

@petebacondarwin fix(core): handle multiple i18n attributes with expression bindings (PATCH) #41912

Conversation

petebacondarwin
Copy link
Member

This is the backport to PATCH of #41882

When there are multiple attributes that are marked for i18n translation,
which contain expression bindings, we were generating i18n update op-codes
that did not accurately map to the correct value to be bound in the lView.
Each attribute's bindings were relative to the start of the lView first
attributes values rather than to their own.

This fix passes the current binding index to `generateBindingUpdateOpCodes()`
when processing i18n attributes to account for this.

Fixes angular#41869
@google-cla google-cla bot added the cla: yes label May 1, 2021
@pullapprove pullapprove bot requested a review from jessicajaniuk May 1, 2021 14:05
@petebacondarwin petebacondarwin added action: review The PR is still awaiting reviews from at least one requested reviewer PullApprove: disable target: patch This PR is targeted for the next patch release labels May 1, 2021
@petebacondarwin petebacondarwin requested review from mhevery and removed request for jessicajaniuk May 1, 2021 14:06
@petebacondarwin petebacondarwin added action: merge The PR is ready for merge by the caretaker action: presubmit The PR is in need of a google3 presubmit labels May 1, 2021
@AndrewKushnir AndrewKushnir added area: core Issues related to the framework runtime area: i18n and removed action: presubmit The PR is in need of a google3 presubmit action: review The PR is still awaiting reviews from at least one requested reviewer labels May 1, 2021
@ngbot ngbot bot modified the milestone: Backlog May 1, 2021
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, I've also updated g3 status to green (no presubmit required since it's a patch-only version of the change that has landed in master branch already).

@AndrewKushnir AndrewKushnir removed the request for review from mhevery May 1, 2021 21:35
@mhevery
Copy link
Contributor

mhevery commented May 3, 2021

Closed by commit 345f5c4

@mhevery mhevery closed this May 3, 2021
mhevery pushed a commit that referenced this pull request May 3, 2021
…41912)

When there are multiple attributes that are marked for i18n translation,
which contain expression bindings, we were generating i18n update op-codes
that did not accurately map to the correct value to be bound in the lView.
Each attribute's bindings were relative to the start of the lView first
attributes values rather than to their own.

This fix passes the current binding index to `generateBindingUpdateOpCodes()`
when processing i18n attributes to account for this.

Fixes #41869

PR Close #41912
@petebacondarwin petebacondarwin deleted the fw-i18n-opcodes-issue-41869-patch branch May 3, 2021 19:37
@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 Jun 3, 2021
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 area: core Issues related to the framework runtime area: i18n cla: yes PullApprove: disable target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants