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

bug(expansion-panel): [style*=...] attribute selector causes whole-page slowdown in Chromium #27946

Closed
1 task done
HMPerson1 opened this issue Oct 15, 2023 · 3 comments · Fixed by #30119
Closed
1 task done
Assignees
Labels
area: material/expansion P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent perf This issue is related to performance

Comments

@HMPerson1
Copy link

Is this a regression?

  • Yes, this behavior used to work in the previous version

The previous version in which this bug was not present was

before #24045 (probably, I haven't tested it)

Description

Strictly speaking, this is a Chromium bug, but (AFAICT) the only workaround is to change the CSS in the expansion panel component.

Currently, in Chromium-based browsers, the presence of a CSS rule selector like .blah[style*="something"] * means that any change to the style attribute of any element, anywhere on the page will force a style recalculation of that element's entire subtree. There is currently a rule like this in expanion-panel:

&[style*='visibility: hidden'] * {

Additionally, since expanion-panel has ViewEncapsulation.None, Angular won't limit the rule's scope using [_nghost]/[_ngcontent] attribute selectors, and Angular will copy the rule into any ShadowDom-encapsulated components.

encapsulation: ViewEncapsulation.None,

This issue was first noted in this comment.

Reproduction

StackBlitz link: https://stackblitz.com/edit/components-issue-bsuofa?file=src%2Fapp%2Fexample-component.ts

Steps to reproduce:

  1. Observe the animation is janky.

Proof that expansion-panel is responsible:

  1. Delete the <mat-expansion-panel> element (and its children) from example-component.html.
  2. Reload (to force Angular to clear unused component styles).
  3. Observe the animation is now smooth.

Proof that the [style] selector is responsible:

  1. Right-click > Inspect Element on "expanstion panel contents".
  2. In DevTools, ensure that <div class="mat-expansion-panel-body"> is selected.
  3. (optionally) Record a performance trace and observe that "Recalculate Style" affects 10,001 elements and takes a long time.
  4. Close the expansion panel.
  5. In DevTools, edit the CSS selector .mat-expansion-panel-content[style*="visibility: hidden"] * to remove [style*=...]
  6. Observe the animation is now smooth.
  7. (optionally) Record a performance trace and observe that "Recalculate Style" affects 1 element and is very fast.

Expected Behavior

The animation is smooth regardless of if an expansion panel exists or what the expansion panel component does internally.

Actual Behavior

The animation is smooth only after modifying an expansion-panel-internal CSS rule.

Environment

Angular CLI: 16.2.6
Node: 19.9.0 (Unsupported)
Package Manager: npm 9.6.3
OS: linux x64

Angular: 16.2.9
... animations, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1602.6
@angular-devkit/build-angular   16.2.6
@angular-devkit/core            16.2.6
@angular-devkit/schematics      16.2.6
@angular/cdk                    16.2.8
@angular/cli                    16.2.6
@angular/material               16.2.8
@schematics/angular             16.2.6
rxjs                            7.5.7
typescript                      5.1.6
zone.js                         0.13.3
@HMPerson1 HMPerson1 added the needs triage This issue needs to be triaged by the team label Oct 15, 2023
@HMPerson1
Copy link
Author

I don't have an easy way to test this, but I suspect this can be fixed by changing this animation to additionally change a CSS custom variable (e.g. --mat-expansion-panel-content-force-visibility) to 'hidden'/'' (so it's always the same as visibility):

bodyExpansion: trigger('bodyExpansion', [
state('collapsed, void', style({height: '0px', visibility: 'hidden'})),
// Clear the `visibility` while open, otherwise the content will be visible when placed in
// a parent that's `visibility: hidden`, because `visibility` doesn't apply to descendants
// that have a `visibility` of their own (see #27436).
state('expanded', style({height: '*', visibility: ''})),

and then changing this CSS rule to apply that variable to all children:

&[style*='visibility: hidden'] * {
visibility: hidden !important;
}

 & * { 
   visibility: var(--mat-expansion-panel-content-force-visibility) !important; 
 } 

@crisbeto
Copy link
Member

We should be able to fix this by setting a class on the component when it's opened/closed.

@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent perf This issue is related to performance area: material/expansion and removed needs triage This issue needs to be triaged by the team labels Oct 16, 2023
@crisbeto crisbeto self-assigned this Dec 5, 2024
crisbeto added a commit to crisbeto/material2 that referenced this issue Dec 5, 2024
Reworks the expansion panel to animate purely with CSS, rather than going through the `@angular/animations` module. This simplifies the setup and allows us to resolve several long-standing bug reports.

Fixes angular#27946.
Fixes angular#22715.
Fixes angular#21434.
Fixes angular#20610.
Fixes angular#20517.
Fixes angular#17177.
Fixes angular#16534.
Fixes angular#16503.
Fixes angular#14952.
Fixes angular#14759.
Fixes angular#14075.
Fixes angular#11765.
crisbeto added a commit to crisbeto/material2 that referenced this issue Dec 5, 2024
Reworks the expansion panel to animate purely with CSS, rather than going through the `@angular/animations` module. This simplifies the setup and allows us to resolve several long-standing bug reports.

Fixes angular#27946.
Fixes angular#22715.
Fixes angular#21434.
Fixes angular#20517.
Fixes angular#17177.
Fixes angular#16534.
Fixes angular#16503.
Fixes angular#14952.
Fixes angular#14759.
Fixes angular#14075.
Fixes angular#11765.
crisbeto added a commit that referenced this issue Dec 5, 2024
Reworks the expansion panel to animate purely with CSS, rather than going through the `@angular/animations` module. This simplifies the setup and allows us to resolve several long-standing bug reports.

Fixes #27946.
Fixes #22715.
Fixes #21434.
Fixes #20517.
Fixes #17177.
Fixes #16534.
Fixes #16503.
Fixes #14952.
Fixes #14759.
Fixes #14075.
Fixes #11765.

(cherry picked from commit aafa151)
@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 Jan 5, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: material/expansion P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent perf This issue is related to performance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants