PolymerElements / paper-dropdown-menu

A Material Design browser select element
https://www.webcomponents.org/element/PolymerElements/paper-dropdown-menu
61 stars 107 forks source link

Make noAnimations imply noink on the inner paper-ripple. #148

Closed rictic closed 6 years ago

rictic commented 8 years ago

Fixes #102 where animation-free dropdowns ended up with tons of Ripple objects endlessly and invisibly animating in requestAnimationFrames, though I'll admit it's not a principled fix, in that I'm not sure what the root cause of the bug is.

This seems to be roughly in line with user expectations I think, as someone who doesn't want animations probably also doesn't want a ripple animation.

I went this route rather than making noink work because the noink property isn't mapped anywhere, and the requestAnimationFrame leak would still occur for people using noAnimations.

cdata commented 8 years ago

@notwaldorf I would value your perspective on @rictic 's rationale for noAnimations → noink.

notwaldorf commented 8 years ago

I think noAnimations → noink makes sense, so 👍 to that. It's a bit more tragic because you'll still get animations from paper-input, but less flash overall.

I am also a bit concerned that clicking creates endless ripples -- is it because they don't cancel? Does this happen if noAnimations isn't set? Is that related to https://github.com/PolymerElements/paper-ripple/pull/76?

rictic commented 8 years ago

I didn't track down where the ripples were being created. Poking at the properties of the ripples that were created, it seemed like they weren't making any progress on each rAF, and the end point of their animation was beyond their stopping point.

It didn't look related to PolymerElements/paper-ripple#76, as each click created a new ripple. In my tests I was looking at ~12 ripples.

If we want a principled answer here I can track it down.