Link2Twenty / l2t-paper-slider

Polymer element for displaying slides in a carousel
https://www.webcomponents.org/element/Link2Twenty/l2t-paper-slider
52 stars 24 forks source link

suggested improvements: no bg color def; handle undefined in setInterval #14

Closed edwardsharp closed 7 years ago

edwardsharp commented 8 years ago

hi there!

...cool element! i made two changes in the element itself because overriding them was proving to be a pain.

1.) removed the #FFCDD2 background style. i got something like this working once, but when i use the polymer starter kit this doesn't work. (i'm pretty new to polymer so am probably missing something)

::content .slider__slides{
  background: none!important;
}

2.) when i navigated away from a page/element that has a <l2t-paper-slider> element errors would get thrown from the _moveInd function because the setInterval was still running but the slider had been torn down. i first tried to detach my element but that was no use (maybe if your element had a detached event handler?). also, it would probably be good to actually clearInterval instead of just checking != undefined...

Link2Twenty commented 8 years ago

I think for the background, I'd like to change it to a Custom property. Something like:

background: var(--paper-slide-background, rgba(0,0,0,0));

That way it's transparent as default but you can easily give it a colour if you want to.

JDenne commented 7 years ago

Hi Link2Twenty

Was just about to upload a pull request to do exactly what you've suggested above.

background: var(--paper-slide-background, rgba(0,0,0,0));

Are you going to commit it, it works well!

Thanks

Joel

Link2Twenty commented 7 years ago

@JDenne I was waiting for the pull request to be updated, I'll do it manually now 👍

JDenne commented 7 years ago

Nice one, thanks very much!

Will the update publish to webcomponents.org too?

Thanks again!

Link2Twenty commented 7 years ago

@JDenne it's there now https://beta.webcomponents.org/element/Link2Twenty/l2t-paper-slider You might need to do a hard refresh to see changes (CTRL + F5)

JDenne commented 7 years ago

Got it, thank you!