There are a couple choices I made for this PR that I would like to share, as I can change them pretty easily before merging if we want to.
Choice 1: Autoplay timing
In the released code as well as in this PR, if you have an animation that takes 500ms (via speed={500}) and autoplayInterval={1000} , the first slide will be shown for 1000ms, after which the autoplay timeout begins at the same time the animation starts. Following the 500ms for the animation to play out, the subsequent slides are only shown holding still for 500ms a piece.
(Each segment ("| |" or "|====|") is 500ms, X marks the beginning of the autoplay timeout/animation)
Choice 1A (existing behavior)
Animation | | |X===| |X===| |X===|
Autoplay Timeout |X===|====|X===|====|X===|====|X===|====
Slide Still |====|====| |====| |====| |====
Choice 1B (alternate behavior)
Animation | | |X===| | |X===| |
Autoplay Timeout |X===|====| |X===|====| |X===|====
Slide Still |====|====| |====|====| |====|====
In this PR, I chose the existing behavior, 1A.
Choice 2: Autoplay pausing
When the carousel is paused by hovering over it with the default pauseOnHover={true} and then unpaused, you have an option of how to handle the interrupted timeout.
Choice 2A (existing behavior): restart the timeout, so with autoplayInterval={1000}, it will be 1000ms until the slide changes again.
Choice 2B (alternate behavior): essentially resume the timeout from where it was when you paused it. So if you were 600ms into a 1000ms autoplayInterval, it will be 400ms until the slide changes again.
In this PR, I chose the alternate behavior, 2B, because it seemed more appropriate for the "pause" wording that is used.
Type of Change
[x] Bug fix (non-breaking change which fixes an issue)
[?] Breaking change (fix or feature that would cause existing functionality to not work as expected)
How Has This Been Tested?
I created an integration test that simulates the carousel autoplay timing, and makes sure everything fires at the expected time.
Description
The autoplay timing is uneven when
wrapAround
istrue
. Note the extra pause after one loop through this demo.There are a couple choices I made for this PR that I would like to share, as I can change them pretty easily before merging if we want to.
Choice 1: Autoplay timing
In the released code as well as in this PR, if you have an animation that takes 500ms (via
speed={500}
) andautoplayInterval={1000}
, the first slide will be shown for 1000ms, after which the autoplay timeout begins at the same time the animation starts. Following the 500ms for the animation to play out, the subsequent slides are only shown holding still for 500ms a piece.In this PR, I chose the existing behavior, 1A.
Choice 2: Autoplay pausing
When the carousel is paused by hovering over it with the default
pauseOnHover={true}
and then unpaused, you have an option of how to handle the interrupted timeout.autoplayInterval={1000}
, it will be 1000ms until the slide changes again.autoplayInterval
, it will be 400ms until the slide changes again.In this PR, I chose the alternate behavior, 2B, because it seemed more appropriate for the "pause" wording that is used.
Type of Change
How Has This Been Tested?
I created an integration test that simulates the carousel autoplay timing, and makes sure everything fires at the expected time.