ken-burns-carousel icon indicating copy to clipboard operation
ken-burns-carousel copied to clipboard

Use slots for the images instead of attribute.

Open enjikaka opened this issue 5 years ago • 3 comments

Having the images specced in an attribute is a bit too React-ish. Consider using slots instead. This would also give to possibility to use and for assigning srcset for responsive images or using WebP with fallback etc :)

<ken-burns-carousel>
   <slot>
      <img src="https://source.unsplash.com/Qh9Swf_8DyA" alt="">
      <img src="https://source.unsplash.com/O453M2Liufs" alt="">
   </slot>
</ken-burns-carousel>

For img elements getting the imageURL could be done like so: https://codepen.io/enjikaka/pen/mYKWGP?editors=0010

enjikaka avatar May 27 '19 10:05 enjikaka

Hey @enjikaka, that's a good idea! If you'd like to make a PR for this, please feel free to do so. We don't have a lot of time on our hands at the moment, so it could take a while for us to make these changes ourselves.

Your code example looks good. I'd maybe only wait for the first image to load before showing the carousel as that could minimize the initial load time.

If you have any questions, contact me anytime :)

leolabs avatar Jun 06 '19 10:06 leolabs

We‘d also need to see how we can wrap each image in a div to make the animations fast. Not sure if this works properly with slots.

NeoLegends avatar Jun 06 '19 10:06 NeoLegends

That should be possible. We could just wrap every image element in the slot with our custom div before inserting them into the carousel. It would be interesting though to see how changes in the slot can be smoothly applied as we do it at the moment.

leolabs avatar Jun 06 '19 10:06 leolabs