PolymerElements / paper-button

A button à la Material Design
https://www.webcomponents.org/element/PolymerElements/paper-button
138 stars 64 forks source link

Remove unused paper-ripple #82

Closed abdonrd closed 8 years ago

abdonrd commented 8 years ago

Also, change polymerelements to PolymerElements

keanulee commented 8 years ago

paper-ripple is used and is dynamically created by PaperRippleBehavior

abdonrd commented 8 years ago

@keanulee Yes, but the paper-button-behavior.html imports the paper-ripple-behavior.html that imports the paper-ripple.html. :)

keanulee commented 8 years ago

Yes, but paper-behavior's bower.json lists paper-ripple as a dev dependency. I guess the HTML import isn't technically needed in paper-button.html though.

abdonrd commented 8 years ago

@keanulee I understand!

Now I've done the following:

And the demo works fine, with the ripple.

keanulee commented 8 years ago

@abdonrd If you bower install --production though, you'll see the paper-ripple is not included. The reason why paper-ripple was included is because (at least) one of the dev dependency is included the latest released version of paper-button, which includes paper-ripple

abdonrd commented 8 years ago

@keanulee oh! Now I understand much better. Thanks!

Then... The right thing would be that paper-behaviors include paper-ripple as dependencies instead of devDependencies. Right?

keanulee commented 8 years ago

@abdonrd Yes, paper-ripple should be a dependency of paper-behaviors - it has an HTML import for it too (https://github.com/PolymerElements/paper-behaviors/blob/master/paper-ripple-behavior.html#L12). Mind sending a PR for that, and we'll revisit this once that's been done and released?

abdonrd commented 8 years ago

@keanulee sure! We can fix it! :muscle:

abdonrd commented 8 years ago

@keanulee now we can open and merge this? :smiley:

keanulee commented 8 years ago

I can't reopen this PR because GitHub won't let me ("The remove_unused_import branch was force-pushed or recreated"). Feel free to submit a new PR.