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

fix(platform-browser): use correct parent in animation removeChild ca… #44033

Conversation

roman-savitski
Copy link

@roman-savitski roman-savitski commented Nov 3, 2021

…llback

Animation's onRemovalComplete callback is using incorrect parentNode.
Should be parentNode provided by delegate.parentNode(element) instead of direct element.parentNode.
This is not a problem with default renderer but can cause problems with custom renderer if it uses another
logic to add/remove nodes.

Fixes #44023

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: 44023

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@google-cla google-cla bot added the cla: yes label Nov 3, 2021
@pullapprove pullapprove bot requested review from alxhub and crisbeto November 3, 2021 17:56
@crisbeto
Copy link
Member

crisbeto commented Nov 4, 2021

The change looks good, but the CI flaked. @raman-savitski can you try to push the same commit again so the CI can run?

…llback

Animation's `onRemovalComplete` callback is using incorrect `parentNode`.
Should be parentNode provided by `delegate.parentNode(element)` instead of direct `element.parentNode`.
This is not a problem with default renderer but can cause problems with custom renderer if it uses another
logic to add/remove nodes.

Fixes angular#44023
@roman-savitski
Copy link
Author

The change looks good, but the CI flaked. @raman-savitski can you try to push the same commit again so the CI can run?

Done, CI checks passed this time.

Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM

@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker action: presubmit The PR is in need of a google3 presubmit target: patch This PR is targeted for the next patch release type: bug/fix labels Nov 4, 2021
@AndrewKushnir
Copy link
Contributor

Presubmit.

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.

@raman-savitski thanks for the fix 👍

@AndrewKushnir AndrewKushnir removed the request for review from alxhub November 9, 2021 17:28
@AndrewKushnir AndrewKushnir added area: animations and removed action: presubmit The PR is in need of a google3 presubmit labels Nov 9, 2021
@ngbot ngbot bot added this to the Backlog milestone Nov 9, 2021
@atscott
Copy link
Contributor

atscott commented Nov 10, 2021

This PR was merged into the repository by commit 236bdff.

atscott pushed a commit that referenced this pull request Nov 10, 2021
…llback (#44033)

Animation's `onRemovalComplete` callback is using incorrect `parentNode`.
Should be parentNode provided by `delegate.parentNode(element)` instead of direct `element.parentNode`.
This is not a problem with default renderer but can cause problems with custom renderer if it uses another
logic to add/remove nodes.

Fixes #44023

PR Close #44033
@atscott atscott closed this in 236bdff Nov 10, 2021
@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 Dec 11, 2021
dimakuba pushed a commit to dimakuba/angular that referenced this pull request Dec 28, 2021
…llback (angular#44033)

Animation's `onRemovalComplete` callback is using incorrect `parentNode`.
Should be parentNode provided by `delegate.parentNode(element)` instead of direct `element.parentNode`.
This is not a problem with default renderer but can cause problems with custom renderer if it uses another
logic to add/remove nodes.

Fixes angular#44023

PR Close angular#44033
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: animations cla: yes target: patch This PR is targeted for the next patch release type: bug/fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Platform-browser animation engine possible inaccurate "parentNode" behavior.
4 participants