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): only check for high contrast mode once #22561

Merged
merged 1 commit into from
May 3, 2021

Conversation

crisbeto
Copy link
Member

I noticed this in the profiler while working on angular/material.angular.io#961. The _applyBodyHighContrastModeCssClasses method is called in the constructor of the A11yModule and MatCommonModule which means that it'll be invoked whenever a new root injector is created for a module importing any Material module (e.g. lazy-loaded route or dynamically-created component). Since it calls into getHighContrastMode and it touches the class list of document.body, it has the potential of causing some some slow layouts. This can be observed either when switching pages in the dev app or by profiling the example creation on material.angular.io.

These changes resolve the issue by ensuring that we only run the logic once.

I noticed this in the profiler while working on angular/material.angular.io#961. The `_applyBodyHighContrastModeCssClasses` method is called in the constructor of the `A11yModule` and `MatCommonModule` which means that it'll be invoked whenever a new root injector is created for a module importing any Material module (e.g. lazy-loaded route or dynamically-created component). Since it calls into `getHighContrastMode` and it touches the class list of `document.body`, it has the potential of causing some some slow layouts. This can be observed either when switching pages in the dev app or by profiling the example creation on material.angular.io.

These changes resolve the issue by ensuring that we only run the logic once.
@crisbeto crisbeto added P2 The issue is important to a large percentage of users, with a workaround target: rc This PR is targeted for the next release-candidate labels Apr 25, 2021
@crisbeto crisbeto requested a review from jelbourn April 25, 2021 11:10
@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Apr 25, 2021
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

@jelbourn jelbourn added the action: merge The PR is ready for merge by the caretaker label Apr 26, 2021
@annieyw annieyw merged commit a92ad3d into angular:master May 3, 2021
annieyw pushed a commit that referenced this pull request May 3, 2021
I noticed this in the profiler while working on angular/material.angular.io#961. The `_applyBodyHighContrastModeCssClasses` method is called in the constructor of the `A11yModule` and `MatCommonModule` which means that it'll be invoked whenever a new root injector is created for a module importing any Material module (e.g. lazy-loaded route or dynamically-created component). Since it calls into `getHighContrastMode` and it touches the class list of `document.body`, it has the potential of causing some some slow layouts. This can be observed either when switching pages in the dev app or by profiling the example creation on material.angular.io.

These changes resolve the issue by ensuring that we only run the logic once.

(cherry picked from commit a92ad3d)
@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 Jun 3, 2021
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 cla: yes PR author has agreed to Google's Contributor License Agreement P2 The issue is important to a large percentage of users, with a workaround target: rc This PR is targeted for the next release-candidate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants