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):KeyEventsPlugin should keep the same behavior #49330

Closed
wants to merge 1 commit into from

Conversation

JiaLiPassion
Copy link
Contributor

Close #45698

KeyEventsPlugin now has different behavior with EventsPlugin, it will always run inside ngZone with ngZone.runGuarded() no matter the component is initialized inside or outside of NgZone, this PR make sure KeyEventsPlugin bahave the same with other events.

@pullapprove pullapprove bot requested a review from jessicajaniuk March 6, 2023 05:13
@ngbot ngbot bot added this to the Backlog milestone Mar 6, 2023
@JiaLiPassion JiaLiPassion added the target: minor This PR is targeted for the next minor release label Mar 6, 2023
Close angular#45698

KeyEventsPlugin now has different behavior with EventsPlugin, it will
always run inside ngZone with ngZone.runGuarded() no matter the
component is initialized inside or outside of NgZone, this PR
make sure KeyEventsPlugin bahave the same with other events.
Copy link
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!

@jessicajaniuk jessicajaniuk added the action: merge The PR is ready for merge by the caretaker label Mar 6, 2023
@jessicajaniuk
Copy link
Contributor

This PR was merged into the repository by commit 2312eb5.

@jessicajaniuk
Copy link
Contributor

This has caused a test breakage in Google3. I'll revert this, and we'll have to run a TGP to see if this can land safely.

@jessicajaniuk jessicajaniuk reopened this Mar 7, 2023
@jessicajaniuk jessicajaniuk added state: blocked and removed action: merge The PR is ready for merge by the caretaker labels Mar 7, 2023
jessicajaniuk pushed a commit to jessicajaniuk/angular that referenced this pull request Mar 7, 2023
@AndrewKushnir AndrewKushnir added the action: global presubmit The PR is in need of a google3 global presubmit label Mar 7, 2023
jessicajaniuk pushed a commit that referenced this pull request Mar 7, 2023
@JiaLiPassion
Copy link
Contributor Author

Got it, Thank you @jessicajaniuk

@jessicajaniuk
Copy link
Contributor

TGP

@jessicajaniuk
Copy link
Contributor

There are definitely some failing tests, but the amount is very small. I'll see if I can address them in the next week so this can land.

@JiaLiPassion
Copy link
Contributor Author

Thank you @jessicajaniuk, if there are anything I can do, please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: global presubmit The PR is in need of a google3 global presubmit area: zones state: blocked target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HostListener on specific keys negates effect of ngZoneEventCoalescing
4 participants