Skip to content

perf(focus-monitor): avoid triggering unnecessary change detections #18668

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

Closed
wants to merge 3 commits into from

Conversation

mleibman
Copy link
Contributor

@mleibman mleibman commented Mar 2, 2020

This is similar to #15077, but IMHO does it in much simpler way.
I also fixed up the documentation, examples, and some call sites that incorrectly stated thatFocusMonitor emits the observable outside of NgZone.

Caretaker note: #15077 had to be reverted before (#15076) because there was an app that depended on the extra change detections. We may have to investigate further before we can merge this in again.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Mar 2, 2020
@jelbourn jelbourn added the target: patch This PR is targeted for the next patch release label Mar 2, 2020
jelbourn
jelbourn previously approved these changes Mar 2, 2020
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@mmalerba mmalerba added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Mar 2, 2020
mmalerba
mmalerba previously approved these changes Mar 2, 2020
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added the P2 The issue is important to a large percentage of users, with a workaround label Mar 3, 2020
@mmalerba mmalerba added the G This is is related to a Google internal issue label Mar 3, 2020
@mmalerba mmalerba removed the lgtm label Jul 31, 2020
@andrewseguin andrewseguin added needs rebase and removed action: merge The PR is ready for merge by the caretaker labels Aug 11, 2020
@andrewseguin
Copy link
Contributor

Please rebase, thanks!

@mmalerba mmalerba added action: merge The PR is ready for merge by the caretaker and removed needs rebase labels Nov 12, 2020
@mmalerba mmalerba force-pushed the perf-focusmonitor-ngzone branch 2 times, most recently from 734d08a to 20a56c9 Compare November 13, 2020 05:32
@mmalerba mmalerba force-pushed the perf-focusmonitor-ngzone branch from 20a56c9 to eaa12a0 Compare November 13, 2020 05:43
@andrewseguin
Copy link
Contributor

Needs rebase (sorry, this is taking a while due to failures internally)

@zarend zarend added the presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged label Jul 29, 2021
@devversion devversion removed their request for review August 18, 2021 12:59
@andrewseguin andrewseguin removed the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 28, 2021
@josephperrott josephperrott added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed needs rebase labels Nov 16, 2022
@crisbeto
Copy link
Member

This was addressed in #15077. Closing.

@crisbeto crisbeto closed this Feb 28, 2024
@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 Mar 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews action: merge The PR is ready for merge by the caretaker G This is is related to a Google internal issue P2 The issue is important to a large percentage of users, with a workaround presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants