Skip to content

fix(platform-browser): wait until animation completion before destroying renderer#50677

Closed
alan-agius4 wants to merge 0 commit intoangular:mainfrom
alan-agius4:wait-animation-remove-css
Closed

fix(platform-browser): wait until animation completion before destroying renderer#50677
alan-agius4 wants to merge 0 commit intoangular:mainfrom
alan-agius4:wait-animation-remove-css

Conversation

@alan-agius4
Copy link
Copy Markdown
Contributor

@alan-agius4 alan-agius4 commented Jun 12, 2023

Prior to this commit, the renderer destroy method was being called before the animation complete. This is problematic when using REMOVE_STYLES_ON_COMPONENT_DESTROY as it causes the styles to be removed too early.

This commit, updates this destroy logic to be call the render destroy once the animations complete.

This has been reported internally in:

@alan-agius4 alan-agius4 added area: animations target: patch This PR is targeted for the next patch release labels Jun 12, 2023
@ngbot ngbot bot modified the milestone: Backlog Jun 12, 2023
@alan-agius4 alan-agius4 force-pushed the wait-animation-remove-css branch 2 times, most recently from 233f2f5 to 979c7a1 Compare June 12, 2023 13:34
@alan-agius4 alan-agius4 added the action: global presubmit The PR is in need of a google3 global presubmit label Jun 12, 2023
@alan-agius4
Copy link
Copy Markdown
Contributor Author

alan-agius4 commented Jun 12, 2023 •
edited
Loading

TGP.

Caretaker note: TGP is failing but it seems that this changes improves the screenshots as previously there was some unstyled content.

https://scuba.corp.google.com/#/report/7c3c19c9-fb5c-4033-8fac-f0aa75a91754?groupBy=Golden+Location&pageSize=25&pageIndex=0&selected_node=learning%2Fdeepmind%2Fui%2Fresearchhub%2Fapp%2Fcomponents%2Fscuba_goldens%2Fchrome-linux%2Ffilter_option_dropdown_options.png&status=2

@alan-agius4 alan-agius4 added action: review The PR is still awaiting reviews from at least one requested reviewer merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jun 12, 2023
@alan-agius4 alan-agius4 marked this pull request as ready for review June 12, 2023 15:32
@pullapprove pullapprove bot requested a review from alxhub June 12, 2023 15:33
@alxhub alxhub requested a review from jessicajaniuk June 12, 2023 15:46
jessicajaniuk
jessicajaniuk approved these changes Jun 12, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@jessicajaniuk jessicajaniuk left a comment

Choose a reason for hiding this comment

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

LGTM! This looks much cleaner than what was there before.

@alan-agius4 alan-agius4 removed the request for review from alxhub June 12, 2023 15:53
@alan-agius4
Copy link
Copy Markdown
Contributor Author

alan-agius4 commented Jun 14, 2023

Caretaker note: this change required a bulk screenshots update in G3 as this causes some diffs. This fixes a real bug as some content in the reference images is unstyled but is now correctly styled.

A CLI that requires global approval is needed, please see discussion in https://angular-team.slack.com/archives/G011LHKQM1Q/p1686585245856779.

@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker and removed action: global presubmit The PR is in need of a google3 global presubmit labels Jun 14, 2023
@alan-agius4 alan-agius4 mentioned this pull request Jun 21, 2023
Closed
@alan-agius4 alan-agius4 linked an issue Jun 21, 2023 that may be closed by this pull request
REMOVE_STYLES_ON_COMPONENT_DESTROY breaks :leave animations #50791
Closed
@AndrewKushnir
Copy link
Copy Markdown
Contributor

AndrewKushnir commented Jun 22, 2023

FYI, adding the "blocked" label for now, since landing this PR needs coordination. We plan to land it on Monday (06/26).

@AndrewKushnir AndrewKushnir added the action: global presubmit The PR is in need of a google3 global presubmit label Jun 23, 2023
@alan-agius4 alan-agius4 removed state: blocked action: global presubmit The PR is in need of a google3 global presubmit labels Jun 26, 2023
@alan-agius4
Copy link
Copy Markdown
Contributor Author

alan-agius4 commented Jun 26, 2023

Caretaker note:

  • TGP
  • CL for screenshot updates (For some reason, around 8 screenshots failed due to permissions (Waymo)).

@AndrewKushnir
Copy link
Copy Markdown
Contributor

AndrewKushnir commented Jun 26, 2023

This PR was merged into the repository by commit d227347.

AndrewKushnir pushed a commit that referenced this pull request Jun 26, 2023
@alan-agius4 @AndrewKushnir
refactor(animations): remove redundant castings (#50677)
d227347 
This commit removes unnecessary castings.

PR Close #50677
AndrewKushnir pushed a commit that referenced this pull request Jun 26, 2023
@alan-agius4 @AndrewKushnir
fix(platform-browser): wait until animation completion before destroy…
79dd6a8 
…ing renderer (#50677)

Prior to this commit, the renderer destroy method was being called before the animation complete. This is problematic when using `REMOVE_STYLES_ON_COMPONENT_DESTROY` as it causes the styles to be removed too early.

This commit, updates this destroy logic to be call the render destroy once the animations complete.

This has been reported internally in:
- http://b/271251353#comment12
- http://b/282004950#comment5

PR Close #50677
AndrewKushnir pushed a commit that referenced this pull request Jun 26, 2023
@alan-agius4 @AndrewKushnir
refactor(animations): remove redundant castings (#50677)
21ced23 
This commit removes unnecessary castings.

PR Close #50677
AndrewKushnir added a commit to AndrewKushnir/angular that referenced this pull request Jun 26, 2023
@AndrewKushnir
Revert "refactor(animations): remove redundant castings (angular#50677)"
3ecc87e 
This reverts commit d227347.
AndrewKushnir added a commit to AndrewKushnir/angular that referenced this pull request Jun 26, 2023
@AndrewKushnir
Revert "fix(platform-browser): wait until animation completion before…
6a23390 
… destroying renderer (angular#50677)"

This reverts commit 2b55103.
@AndrewKushnir AndrewKushnir mentioned this pull request Jun 26, 2023
Closed
AndrewKushnir added a commit that referenced this pull request Jun 26, 2023
@AndrewKushnir
Revert "refactor(animations): remove redundant castings (#50677)" (#5…
5a83c0c 
…0857)

This reverts commit d227347.

PR Close #50857
AndrewKushnir added a commit that referenced this pull request Jun 26, 2023
@AndrewKushnir
Revert "fix(platform-browser): wait until animation completion before…
9b7bc80 
… destroying renderer (#50677)" (#50857)

This reverts commit 2b55103.

PR Close #50857
AndrewKushnir added a commit that referenced this pull request Jun 26, 2023
@AndrewKushnir
Revert "refactor(animations): remove redundant castings (#50677)" (#5…
ed8b088 
…0857)

This reverts commit d227347.

PR Close #50857
AndrewKushnir added a commit that referenced this pull request Jun 26, 2023
@AndrewKushnir
Revert "fix(platform-browser): wait until animation completion before…
452a3e9 
… destroying renderer (#50677)" (#50857)

This reverts commit 2b55103.

PR Close #50857
@AndrewKushnir AndrewKushnir reopened this Jun 26, 2023
@alan-agius4 alan-agius4 added action: global presubmit The PR is in need of a google3 global presubmit and removed docsarea: global labels Jun 26, 2023
@AndrewKushnir AndrewKushnir force-pushed the wait-animation-remove-css branch from 979c7a1 to 39935ee Compare June 27, 2023 01:51
@alan-agius4 alan-agius4 mentioned this pull request Jun 27, 2023
Closed
sunilbaba pushed a commit to sunilbaba/angular that referenced this pull request Jul 26, 2023
@alan-agius4 @sunilbaba
fix(platform-browser): wait until animation completion before destroy…
2621792 
…ing renderer (angular#50677)

Prior to this commit, the renderer destroy method was being called before the animation complete. This is problematic when using `REMOVE_STYLES_ON_COMPONENT_DESTROY` as it causes the styles to be removed too early.

This commit, updates this destroy logic to be call the render destroy once the animations complete.

This has been reported internally in:
- http://b/271251353#comment12
- http://b/282004950#comment5

PR Close angular#50677
sunilbaba pushed a commit to sunilbaba/angular that referenced this pull request Jul 26, 2023
@alan-agius4 @sunilbaba
refactor(animations): remove redundant castings (angular#50677)
24844ad 
This commit removes unnecessary castings.

PR Close angular#50677
sunilbaba pushed a commit to sunilbaba/angular that referenced this pull request Jul 26, 2023
@AndrewKushnir @sunilbaba
Revert "refactor(animations): remove redundant castings (angular#50677)…
629a349 
…" (angular#50857)

This reverts commit d227347.

PR Close angular#50857
sunilbaba pushed a commit to sunilbaba/angular that referenced this pull request Jul 26, 2023
@AndrewKushnir @sunilbaba
Revert "fix(platform-browser): wait until animation completion before…
a0e2516 
… destroying renderer (angular#50677)" (angular#50857)

This reverts commit 2b55103.

PR Close angular#50857
@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented Jul 28, 2023

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 Jul 28, 2023
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@alan-agius4 @ChellappanRajan
fix(platform-browser): wait until animation completion before destroy…
464becf 
…ing renderer (angular#50677)

Prior to this commit, the renderer destroy method was being called before the animation complete. This is problematic when using `REMOVE_STYLES_ON_COMPONENT_DESTROY` as it causes the styles to be removed too early.

This commit, updates this destroy logic to be call the render destroy once the animations complete.

This has been reported internally in:
- http://b/271251353#comment12
- http://b/282004950#comment5

PR Close angular#50677
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@alan-agius4 @ChellappanRajan
refactor(animations): remove redundant castings (angular#50677)
f430f87 
This commit removes unnecessary castings.

PR Close angular#50677
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@alxhub alxhub alxhub approved these changes

+1 more reviewer

@jessicajaniuk jessicajaniuk jessicajaniuk approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: global presubmit The PR is in need of a google3 global presubmit action: merge The PR is ready for merge by the caretaker area: animations merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note state: blocked target: patch This PR is targeted for the next patch release

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

REMOVE_STYLES_ON_COMPONENT_DESTROY breaks :leave animations

4 participants

@alan-agius4 @AndrewKushnir @alxhub @jessicajaniuk