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

[MatCheckbox] nested ripple registers events even if disableRipple is set. #8854

Closed
takamori opened this issue Dec 6, 2017 · 2 comments · Fixed by #8882
Closed

[MatCheckbox] nested ripple registers events even if disableRipple is set. #8854

takamori opened this issue Dec 6, 2017 · 2 comments · Fixed by #8882
Assignees
Labels
G This is is related to a Google internal issue 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

@takamori
Copy link

takamori commented Dec 6, 2017

Bug, feature request, or proposal:

When using MatCheckbox, MatRipple and RippleRenderer is instantiated and registers events even if disableRipple is set on the checkbox.

What is the expected behavior?

If disableRipple is set, MatRipple and RippleRenderer should not be instantiated, or at the very least the events should not be registered during the initial render.

What is the current behavior?

There are 10 events registered for each checkbox, slowing down the render of the checkbox.

What are the steps to reproduce?

http://embed.plnkr.co/PZ0P3EerE2aeJPR4Kasa/

When clicking on the button to view Material Checkboxes, 30-45% of the time is spent on adding event listeners.

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

This is visible when using a virtual rendering component to show a large number of items with checkboxes and scrolling, or when rendering the full list as in here. The result is slow rendering or janky scrolling.

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

All versions are affected. Tested with Angular 2 with Chrome 62 on MacOS.

Is there anything else we should know?

@takamori
Copy link
Author

takamori commented Dec 6, 2017

I would have expected an *ngIf="_isRippleDisabled()" at this line: https://github.com/angular/material2/blob/master/src/lib/checkbox/checkbox.html#L19

@josephperrott josephperrott added the G This is is related to a Google internal issue label Dec 6, 2017
@jelbourn jelbourn 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 labels Dec 7, 2017
devversion added a commit to devversion/material2 that referenced this issue Dec 7, 2017
Removes the ripple container element if ripples should be disabled for most of the Material components. This gives us a performance boost because the ripple events aren't being added if ripples are disabled.

Fixes angular#8854
devversion added a commit to devversion/material2 that referenced this issue Dec 8, 2017
* No longer registers trigger event listeners if the ripples are disabled initially.

Fixes angular#8854.
devversion added a commit to devversion/material2 that referenced this issue Dec 8, 2017
* No longer registers trigger event listeners if the ripples are disabled initially.

Fixes angular#8854.
devversion added a commit to devversion/material2 that referenced this issue Dec 8, 2017
* No longer registers trigger event listeners if the ripples are disabled initially.

Fixes angular#8854.
devversion added a commit to devversion/material2 that referenced this issue Dec 9, 2017
* No longer registers trigger event listeners if the ripples are disabled initially.

Fixes angular#8854.
devversion added a commit to devversion/material2 that referenced this issue Dec 20, 2017
* No longer registers trigger event listeners if the ripples are disabled initially.

Fixes angular#8854.
devversion added a commit to devversion/material2 that referenced this issue Dec 31, 2017
* No longer registers trigger event listeners if the ripples are disabled initially.

Fixes angular#8854.
@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 Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
G This is is related to a Google internal issue 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
4 participants