Fix autoPlayLoop (#1239)

* chore: basic fix for autoPlayLoop

* chore: implement looping

* chore: add navigationDirection to deck state

* chore: mention the compromise made

* test: add tests for autoPlayLoop

* chore: add changeset for the fix

Author: fritz-c

.changeset/gentle-berries-judge.md

@@ -0,0 +1,5 @@
+---
+'spectacle': patch
+---
+
+autoPlayLoop now works as intended

packages/spectacle/src/components/appear.tsx

@@ -60,7 +60,9 @@ type SteppedComponentProps = {
   children: ReactNode | ((step: number, isActive: boolean) => ReactNode);
   className?: string;
   tagName?: keyof JSX.IntrinsicElements;
+  // TODO v10: change this type to React.CSSProperties
   activeStyle?: Partial<CSSStyleDeclaration>;
+  // TODO v10: change this type to React.CSSProperties
   inactiveStyle?: Partial<CSSStyleDeclaration>;
   numSteps?: number;
   alwaysAppearActive?: boolean;
@@ -117,6 +119,8 @@ type StepperProps<T extends unknown[] = unknown[]> = {
   tagName?: keyof JSX.IntrinsicElements;
   values: T;
   alwaysVisible?: boolean;
+  // TODO v10: change this type to React.CSSProperties
   activeStyle?: Partial<CSSStyleDeclaration>;
+  // TODO v10: change this type to React.CSSProperties
   inactiveStyle?: Partial<CSSStyleDeclaration>;
 };

packages/spectacle/src/components/deck/deck.test.tsx

@@ -1,9 +1,18 @@
 import Deck from './index';
 import Slide from '../slide/slide';
-import { render } from '@testing-library/react';
+import { act, render } from '@testing-library/react';
 import { CSSObject } from 'styled-components';
+import { Stepper } from '../appear';
 
 describe('<Deck />', () => {
+  beforeEach(() => {
+    jest.useFakeTimers();
+  });
+
+  afterEach(() => {
+    jest.useRealTimers();
+  });
+
   it('should allow for backdrop color overrides from theme prop', () => {
     const deckWithStyle = (
       backdropStyle: CSSObject,
@@ -34,4 +43,106 @@ describe('<Deck />', () => {
       backgroundColor: 'black'
     });
   });
+
+  it('should automatically advance with autoPlay=true', () => {
+    const slideNumberTracker = jest.fn();
+    const stepperActiveTracker = jest.fn();
+    const autoPlayInterval = 5000;
+
+    const deckWithExtraProps = (
+      props: Partial<React.ComponentProps<typeof Deck>>
+    ) => (
+      <Deck
+        autoPlay
+        autoPlayInterval={autoPlayInterval}
+        {...props}
+        template={({ slideNumber }) => {
+          slideNumberTracker(slideNumber);
+          return null;
+        }}
+      >
+        <Slide>Slide 1</Slide>
+        <Slide>Slide 2</Slide>
+        <Slide>
+          Slide 3
+          <Stepper values={[1]}>
+            {(_value, _step, isActive) => {
+              stepperActiveTracker(isActive);
+              return null;
+            }}
+          </Stepper>
+        </Slide>
+      </Deck>
+    );
+
+    const { rerender } = render(deckWithExtraProps({ autoPlayLoop: false }));
+
+    expect(slideNumberTracker).toHaveBeenLastCalledWith(1);
+    expect(stepperActiveTracker).toHaveBeenLastCalledWith(false);
+
+    act(() => {
+      jest.advanceTimersByTime(autoPlayInterval);
+    });
+
+    // autoPlay advanced to the next slide
+    expect(slideNumberTracker).toHaveBeenLastCalledWith(2);
+
+    act(() => {
+      jest.advanceTimersByTime(autoPlayInterval);
+    });
+
+    // autoPlay advanced to the final slide
+    expect(stepperActiveTracker).toHaveBeenLastCalledWith(false);
+    expect(slideNumberTracker).toHaveBeenLastCalledWith(3);
+
+    act(() => {
+      jest.advanceTimersByTime(autoPlayInterval);
+    });
+
+    // autoPlay activated the stepper on the third slide
+    expect(stepperActiveTracker).toHaveBeenLastCalledWith(true);
+    expect(slideNumberTracker).toHaveBeenLastCalledWith(3);
+
+    act(() => {
+      jest.advanceTimersByTime(autoPlayInterval);
+    });
+
+    // autoPlay stalled at the end as there are no more steps
+    expect(stepperActiveTracker).toHaveBeenLastCalledWith(true);
+    expect(slideNumberTracker).toHaveBeenLastCalledWith(3);
+
+    // Rerender with looping activated
+    rerender(deckWithExtraProps({ autoPlayLoop: true }));
+
+    act(() => {
+      jest.advanceTimersByTime(autoPlayInterval);
+    });
+
+    // autoPlay looped around to the first slide
+    expect(stepperActiveTracker).toHaveBeenLastCalledWith(false);
+    expect(slideNumberTracker).toHaveBeenLastCalledWith(1);
+
+    // Skipping ahead to the last step on the last slide
+    act(() => {
+      jest.advanceTimersByTime(autoPlayInterval);
+    });
+    act(() => {
+      jest.advanceTimersByTime(autoPlayInterval);
+    });
+    act(() => {
+      jest.advanceTimersByTime(autoPlayInterval);
+    });
+
+    // autoPlay brought us to the third slide with the stepper activated
+    expect(stepperActiveTracker).toHaveBeenLastCalledWith(true);
+    expect(slideNumberTracker).toHaveBeenLastCalledWith(3);
+
+    act(() => {
+      jest.advanceTimersByTime(autoPlayInterval);
+    });
+
+    // autoPlay looped around to the first slide
+    expect(stepperActiveTracker).toHaveBeenLastCalledWith(false);
+    expect(slideNumberTracker).toHaveBeenLastCalledWith(1);
+  });
 });

packages/spectacle/src/components/deck/deck.tsx

@@ -44,7 +44,10 @@ import { KEYBOARD_SHORTCUTS_IDS } from '../../utils/constants';
 export type DeckContextType = {
   deckId: string | number;
   slideCount: number;
+  slideIds: SlideId[];
   useAnimations: boolean;
+  autoPlayLoop: boolean;
+  navigationDirection: number;
   slidePortalNode: HTMLDivElement;
   onSlideClick(e: MouseEvent, slideId: SlideId): void;
   onMobileSlide(eventData: SwipeEventData): void;
@@ -54,8 +57,6 @@ export type DeckContextType = {
   backdropNode: HTMLDivElement;
   notePortalNode: HTMLDivElement;
   initialized: boolean;
-  passedSlideIds: Set<SlideId>;
-  upcomingSlideIds: Set<SlideId>;
   activeView: {
     slideId: SlideId;
     slideIndex: number;
@@ -173,6 +174,7 @@ export const DeckInternal = forwardRef<DeckRef, DeckInternalProps>(
       initialized,
       pendingView,
       activeView,
+      navigationDirection,
 
       initializeTo,
       skipTo,
@@ -290,11 +292,7 @@ export const DeckInternal = forwardRef<DeckRef, DeckInternalProps>(
       enabled: autoPlay,
       loop: autoPlayLoop,
       interval: autoPlayInterval,
-      navigation: {
-        skipTo,
-        stepForward,
-        isFinalSlide: activeView.slideIndex === slideIds.length - 1
-      }
+      stepForward
     });
 
     const handleSlideClick = useCallback<
@@ -310,22 +308,6 @@ export const DeckInternal = forwardRef<DeckRef, DeckInternalProps>(
     const activeSlideId = slideIds[activeView.slideIndex];
     const pendingSlideId = slideIds[pendingView.slideIndex];
 
-    const [passed, upcoming] = useMemo(() => {
-      const p = new Set<SlideId>();
-      const u = new Set<SlideId>();
-      let foundActive = false;
-      for (const slideId of slideIds) {
-        if (foundActive) {
-          u.add(slideId);
-        } else if (slideId === activeSlideId) {
-          foundActive = true;
-        } else {
-          p.add(slideId);
-        }
-      }
-      return [p, u] as const;
-    }, [slideIds, activeSlideId]);
-
     const fullyInitialized = initialized && slideIdsInitialized;
 
     // Slides don't actually render their content to their position in the DOM-
@@ -446,20 +428,21 @@ export const DeckInternal = forwardRef<DeckRef, DeckInternalProps>(
             value={{
               deckId,
               slideCount: slideIds.length,
+              slideIds,
               useAnimations,
               slidePortalNode: slidePortalNode!,
               onSlideClick: handleSlideClick,
               onMobileSlide: onMobileSlide,
               theme: restTheme,
+              autoPlayLoop,
+              navigationDirection,
 
               frameOverrideStyle: frameStyle,
               wrapperOverrideStyle: wrapperStyle,
 
               backdropNode: backdropRef.current!,
               notePortalNode: notePortalNode!,
               initialized: fullyInitialized,
-              passedSlideIds: passed,
-              upcomingSlideIds: upcoming,
               activeView: {
                 ...activeView,
                 slideId: activeSlideId
@@ -540,7 +523,10 @@ type BackdropOverrides = {
 
 export type DeckRef = Omit<
   DeckStateAndActions,
-  'pendingView' | 'commitTransition' | 'cancelTransition'
+  | 'cancelTransition'
+  | 'commitTransition'
+  | 'navigationDirection'
+  | 'pendingView'
 > & {
   numberOfSlides: number;
 };