amp-carousel: Tapping arrow buttons triggers slideChange action twice

[dreamofabear]

This can cause unexpected behavior for non-idempotent actions triggered by slideChange.

slidescroll#customSnap_ is called twice:

• At animation start in go
• When animation completes in scrollHandler_

/cc @camelburrito

[camelburrito]

This is expected . Go happens via button click so we custom snap from the beginning.

In scrollhandler we monitor scroll and decide when to start snapping (custom snap is needed here too)

[dreamofabear]

Thanks for the context.

[camelburrito]

Can we close this or is there something else that we need to discuss?

[dreamofabear]

No, this is definitely a bug since slideChange should not be triggered twice. Need to add guards or change the implementation.

[camelburrito]

ooops - i did not see the slidechange event being triggered.

https://github.com/ampproject/amphtml/blob/master/extensions/amp-carousel/0.1/slidescroll.js#L584

this.slideChange_ will not do anything if it is already snapping at the slideindex , this is also how we do analytics for the slides, i deally the event should be triggered exactly where the analytics gets triggered.

https://github.com/ampproject/amphtml/blob/master/extensions/amp-carousel/0.1/slidescroll.js#L572