perf: remove element if ripples are disabled

[devversion]

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.

In theory I would say that using *ngIf instead of [matRippleDisabled] is more appropriate in most of the components, but there are also cases where the ripple container can't be removed completely and for that, the [matRippleDisabled] binding is the right choice.

From a performance perspective, if someone often changes the disabled/disableRipple state, the event listeners will be removed and re-added. This seems to be like a trade-off. A possible solution for that would be to just keep the current implementation and just not register the events if the ripple directive is set to disabled initially (and afterwards register events if it turns enabled again).

Fixes #8854.

[jelbourn]

Will this still register the events on the initial render if _isRippleDisabled it false on start-up?

[devversion]

No, the events won't be registered. And the directive won't be initialized either.

[crisbeto]

I'm not sure whether the gains from not having the element at all outweigh the overhead of having to re-create everything. It might be better if we kept the element around, but only added/removed the event listeners when disabled.

[devversion]

@crisbeto Yeah I agree. That should be also better for performance in the cases where the element's disabled state is toggled pretty often (what I mentioned in the PR description)

I will try to work more on this PR. I think the only change would be to don't register the events if the ripples are disabled initially.

[AsafAgranat]

Events overhead aside, there's also a browser memory overhead, when disabled ripple elements remain in the DOM, as explained here. 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]

@AsafAgranat This is a good point. Can we move that discussion to the new issue you've filed? I've just wanted to link those two threads because they have a related discussion.

(meaning that you just copy what you wrote to the new issue)

[AsafAgranat]

@devversion, sure. will do.

[angular-automatic-lock-bot]

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.}