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

disabled ripple in buttons are still rendered in the DOM, taking up memory #12647

Closed
AsafAgranat opened this issue Aug 13, 2018 · 7 comments
Closed
Assignees
Labels
area: material/core needs: discussion Further discussion with the team is needed before proceeding P5 The team acknowledges the request but does not plan to address it, it remains open for discussion perf This issue is related to performance

Comments

@AsafAgranat
Copy link

Bug, feature request, or proposal:

When using [disableRipple] on mat-button, the ripple effect is indeed disabled, but the ripple DOM element is still rendered. Since the CSS of ripple elements is position: absolute, each ripple instance is elevated to its own layer. Each layer consumes +15KB of memory. This is waste of precious browser memory. Sometimes there is an additional layer created to deal with overlapping elements, taking yet more memory. I exemplify below.

What is the expected behavior?

.mat-ripple elements should not be rendered in the DOM when [disableRipple] is set to true

What is the current behavior?

.mat-ripple elements are rendered in the DOM and consume memory

What are the steps to reproduce?

Set a mat-button to disable ripple effect using [disableRipple]="true". Open Chrome or Safari's devTools and go to Layers panel. Inspect the button and see that its ripple is present, and is elevated to its own layer. See in the bottom panel how much memory that layer consumes.

What is the use-case or motivation for changing an existing behavior?

Improve performance

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Material 6.3.3, Angular 6.0.4

Example

I attached example images showing a list of 5 items, each has a button with ripple disabled. 5 x ~20KB is roughly 100KB of browser memory taken for nothing.

Material ripple layers as is
material ripple disabled

Material ripple layers, with position: absolute removed
material ripple forced to display none

@devversion devversion self-assigned this Aug 13, 2018
@devversion
Copy link
Member

We had a related discussion about this in one my PRs. See #8864. There should be a discussion about this before we start going on specific way.

@AsafAgranat
Copy link
Author

Events overhead aside, there's also a browser memory overhead, when disabled ripple elements remain in the DOM, as explained above. The short of it is that every ripple elements is painted on it's own DOM layer, bulking up the memory. Therefore ,disabled ripples should at least have their position: absolute changed to position: unset in order to spare the extra layers.

@devversion devversion added the perf This issue is related to performance label Sep 22, 2018
@bsteffl
Copy link

bsteffl commented Jan 14, 2019

Additionally when you use the global config and disable this you still get the visual leftovers of ripple.
See example with slide-toggle below. I noticed this in buttons and radios also.
image

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

@devversion
Copy link
Member

@bsteffl These are the focus indicator ripples which are intentionally showing up if disabled. This is because otherwise there would be no visual indication that such a custom selection-control is focused.

Though it should not show up if users use their mouse. this will be fixed with #13562

@Chiragsorathiya
Copy link

Chiragsorathiya commented Dec 9, 2019

I have the same issue as @bsteffl
I only facing on slide toggle not checkbox and radio button

@mmalerba mmalerba added needs: discussion Further discussion with the team is needed before proceeding and removed discussion labels Mar 3, 2020
@mmalerba mmalerba added the needs triage This issue needs to be triaged by the team label May 20, 2020
@devversion devversion added area: material/core P5 The team acknowledges the request but does not plan to address it, it remains open for discussion and removed needs triage This issue needs to be triaged by the team labels May 27, 2020
@jelbourn
Copy link
Member

Closing this since we don't plan to change the ripple specifically for this. As we continue to align our implementation with MDC, I do expect to see more changes to the ripple rendering, though.

@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 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: material/core needs: discussion Further discussion with the team is needed before proceeding P5 The team acknowledges the request but does not plan to address it, it remains open for discussion perf This issue is related to performance
Projects
None yet
Development

No branches or pull requests

6 participants