basicLightbox icon indicating copy to clipboard operation
basicLightbox copied to clipboard

Listen for transition end

Open jimblue opened this issue 6 years ago • 3 comments

You're using requestAnimationFrame two times but without changing dom propriety from javascript. I don't see how this can improve performance as you add class to animate dom element.

I can be wrong of course, if it's the case please light me candle 👍

https://github.com/electerious/basicLightbox/blob/e82ed1060f1a48e068d4a296eb29d741214a9b37/src/scripts/main.js#L123 https://github.com/electerious/basicLightbox/blob/e82ed1060f1a48e068d4a296eb29d741214a9b37/src/scripts/main.js#L150

jimblue avatar Feb 19 '18 16:02 jimblue

The first requestAnimationFrame is there to ensure that the class change triggers the animation. You have to wait a frame after adding the element to the DOM. Otherwise it wouldn't do the transition to the basicLightbox--visible class.

I'm not sure if there's a reason for the second requestAnimationFrame. I would say that it's useless. Might be worth a try. We could also use a keyframe animation and avoid requestAnimationFrame completely.

electerious avatar Feb 23 '18 19:02 electerious

Ok I got it, thanks you 😄 !

Also I think you could provide a better callback here: https://github.com/electerious/basicLightbox/blob/ab2a2df6378c50946dc45b315d9d4268f4e741a7/src/scripts/main.js#L154

Why don't you listen for animation end?

jimblue avatar Feb 23 '18 19:02 jimblue

Removed the second requestAnimationFrame: https://github.com/electerious/basicLightbox/blob/master/CHANGELOG.md#401---2018-02-23

Listening for animation end would be better. PR welcome :)

electerious avatar Feb 23 '18 20:02 electerious