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

Performance issue when using many mat-button-toggle-group components #17252

Closed
Alin764 opened this issue Oct 1, 2019 · 4 comments · Fixed by #17253, lingounet/testage#9, hrueger/AGLight#112 or hrueger/AGLight#114
Assignees
Labels
P2 The issue is important to a large percentage of users, with a workaround

Comments

@Alin764
Copy link

Alin764 commented Oct 1, 2019

Reproduction

https://stackblitz.com/edit/angular-cacnzk

I have a HTML table containing 250 rows
Each row has a mat-button-toggle-group with 5 options + another two mat-select components

When I toggle a button in the first rows of the table I’m having a significant delay. The delay is decreasing if move to the bottom of the page

Also, If I change from mat-button-toggle-group to mat-radio-group I don’t have these performance issue.

Steps to reproduce:

  1. toggle a button in the first row of the table
  2. toggle a button the last row of the table
  3. comment mat-button-toggle-group tag and enable mat-radio-group tag
  4. repeat steps 1 and 2 ( there is no delay when using mat-radio-group )

Expected Behavior

Similar performance as mat-radio-group

Actual Behavior

There is significant delay when using mat-button-toggle-group instead of mat-radio-group

Environment

  • Angular: 8.0.0
  • CDK/Material: 8.2.1
  • Browser(s): Chrome Version 77.0.3865.90 (Official Build)
  • Operating System: Windows
@crisbeto crisbeto self-assigned this Oct 1, 2019
@crisbeto crisbeto added has pr P2 The issue is important to a large percentage of users, with a workaround labels Oct 1, 2019
crisbeto added a commit to crisbeto/material2 that referenced this issue Oct 1, 2019
A while ago we removed the `transform` from the `.mat-ripple` element in order to avoid creating extra layers for elements that the user isn't interacting with. As a result the ripple elements can cause layouts to be triggered on the entire page, which is noticeable on larger pages (see angular#17252).

These changes are a hybrid solution where we add the `transform`, but only if the `.mat-ripple` element has content. This isn't bulletproof since the `:empty` selector can be invalidated by whitespace. For now this is our best solution that works across browsers, until the `contain` property gets better support.

Fixes angular#17252.
@Alin764
Copy link
Author

Alin764 commented Oct 3, 2019

@crisbeto , is there an easy way that I can add this fix into my existing solution. I tried to add the css part which was modified, but is not working.

@crisbeto
Copy link
Member

crisbeto commented Oct 3, 2019

You should be able to just apply the same styles in your app. If you're doing it from inside a view-encapsulated component it might not be reaching the ripple elements though.

@Alin764
Copy link
Author

Alin764 commented Oct 3, 2019

Thanks, it worked by adding this in my styles.scss

.mat-ripple { overflow: hidden; position: relative; &:not(:empty) { transform: translateZ(0); } }

jelbourn pushed a commit that referenced this issue Jan 23, 2020
A while ago we removed the `transform` from the `.mat-ripple` element in order to avoid creating extra layers for elements that the user isn't interacting with. As a result the ripple elements can cause layouts to be triggered on the entire page, which is noticeable on larger pages (see #17252).

These changes are a hybrid solution where we add the `transform`, but only if the `.mat-ripple` element has content. This isn't bulletproof since the `:empty` selector can be invalidated by whitespace. For now this is our best solution that works across browsers, until the `contain` property gets better support.

Fixes #17252.
jelbourn pushed a commit that referenced this issue Jan 29, 2020
A while ago we removed the `transform` from the `.mat-ripple` element in order to avoid creating extra layers for elements that the user isn't interacting with. As a result the ripple elements can cause layouts to be triggered on the entire page, which is noticeable on larger pages (see #17252).

These changes are a hybrid solution where we add the `transform`, but only if the `.mat-ripple` element has content. This isn't bulletproof since the `:empty` selector can be invalidated by whitespace. For now this is our best solution that works across browsers, until the `contain` property gets better support.

Fixes #17252.
yifange pushed a commit to yifange/components that referenced this issue Jan 30, 2020
A while ago we removed the `transform` from the `.mat-ripple` element in order to avoid creating extra layers for elements that the user isn't interacting with. As a result the ripple elements can cause layouts to be triggered on the entire page, which is noticeable on larger pages (see angular#17252).

These changes are a hybrid solution where we add the `transform`, but only if the `.mat-ripple` element has content. This isn't bulletproof since the `:empty` selector can be invalidated by whitespace. For now this is our best solution that works across browsers, until the `contain` property gets better support.

Fixes angular#17252.
@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 Feb 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.