PolymerElements / paper-ripple

Adds a Material Design ripple effect to UI elements
https://webcomponents.org/element/PolymerElements/paper-ripple
56 stars 27 forks source link

Add PaperRipple.rippleAnimate() as an alias for animate(). #123

Closed aomarks closed 6 years ago

aomarks commented 6 years ago

Also don't emit the animate() method in the TypeScript types, because it conflicts with Element.animate().

This way the types compile, but there is still a way for users to type-safely call our animate() method.

e111077 commented 6 years ago

Please add a deprecation notice in the animate function description. Also what do you think about a console.warn called only when you call animate directly?

It's a major version bump; I think we can get away with at least encouraging people move off of the function.

e111077 commented 6 years ago

Actually scratch that. I doubt there will be another major version bump in which we actually remove the animate function

aomarks commented 6 years ago

Actually scratch that. I doubt there will be another major version bump in which we actually remove the animate function

I added a deprecation comment to use the other method instead.

Yeah, maybe the console warning is overkill given that we'll probably never remove it.

bicknellr commented 6 years ago

The @suppress {checkTypes} seems to cause an @ts-ignore annotation to show up in the output on the animate() signature, is that not enough to handle this case?

aomarks commented 6 years ago

No, it has no effect in this context.

bicknellr commented 6 years ago

What does PaperRippleElement become if it changes the type of animate? Does it just fail to compile completely or does it become something that's no longer considered a subtype of the stuff it extends but still has the same members (excluding animate)?

bicknellr commented 6 years ago

(discussed offline) This seems like the best way to go about this without making a breaking change by renaming animate.