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

perf(cdk/a11y): avoid triggering change detection if there are no subscribers to stream #15077

Merged

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Feb 4, 2019

This is a resubmit of #14964.

Currently we have an NgZone.run call on each focus and blur event of a monitored event in order to bring its subscribers into the NgZone, however this means that we're also triggering change detection to any consumers that aren't subscribed to changes (e.g. mat-button only cares about the classes being applied). These changes move around some logic so that the NgZone.run is only hit if somebody has subscribed to the observable.

Caretaker note: this PR 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.

@crisbeto crisbeto added merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: minor This PR is targeted for the next minor release labels Feb 4, 2019
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Feb 4, 2019
@mmalerba mmalerba added the action: merge The PR is ready for merge by the caretaker label Feb 6, 2019
@mmalerba mmalerba added aaa and removed aaa labels Apr 25, 2019
@mmalerba mmalerba removed their assignment Aug 12, 2019
@crisbeto crisbeto added the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label Aug 18, 2019
@crisbeto crisbeto force-pushed the focus-monitor-change-detection-again branch from bb3ff4d to d5ca30d Compare August 18, 2019 20:19
@crisbeto crisbeto force-pushed the focus-monitor-change-detection-again branch from d5ca30d to 6b96ee6 Compare May 9, 2020 14:31
@crisbeto crisbeto force-pushed the focus-monitor-change-detection-again branch from 6b96ee6 to f3ce4d8 Compare December 25, 2020 14:52
@crisbeto crisbeto changed the title perf(focus-monitor): avoid triggering change detection if there are no subscribers to stream perf(cdk/a11y): avoid triggering change detection if there are no subscribers to stream Dec 25, 2020
@crisbeto crisbeto force-pushed the focus-monitor-change-detection-again branch from f3ce4d8 to cac70ac Compare May 7, 2021 06:06
@devversion devversion removed their request for review August 18, 2021 12:55
@andrewseguin andrewseguin added needs rebase and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels Dec 29, 2021
…scribers to stream

Currently we have an `NgZone.run` call on each `focus` and `blur` event of a monitored
event in order to bring its subscribers into the `NgZone`, however this means that we're
also triggering change detection to any consumers that aren't subscribed to changes
(e.g. `mat-button` only cares about the classes being applied). These changes move
around some logic so that the `NgZone.run` is only hit if somebody has subscribed
to the observable.
@crisbeto crisbeto force-pushed the focus-monitor-change-detection-again branch from cac70ac to 81c4c97 Compare March 16, 2022 05:59
@crisbeto crisbeto merged commit ee452de into angular:master Mar 17, 2022
forsti0506 pushed a commit to forsti0506/components that referenced this pull request Apr 3, 2022
…scribers to stream (angular#15077)

Currently we have an `NgZone.run` call on each `focus` and `blur` event of a monitored
event in order to bring its subscribers into the `NgZone`, however this means that we're
also triggering change detection to any consumers that aren't subscribed to changes
(e.g. `mat-button` only cares about the classes being applied). These changes move
around some logic so that the `NgZone.run` is only hit if somebody has subscribed
to the observable.
@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 Apr 17, 2022
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 merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants