Fix deck state transitions. Add test cases for deck state reducer. (#1290)

* Fix deck state transitions. Add test cases for deck state reducer.

* Annotate the tests explaining each action.

* Remove debugging console log.

* Remove temporary stepper from example.

* Changeset

Author: carloskelly13

.changeset/heavy-knives-sniff.md

@@ -0,0 +1,5 @@
+---
+'spectacle': patch
+---
+
+Fixed deck transitions for presenter mode, added test coverage around deck reducer.

packages/spectacle/src/components/presenter-mode/index.tsx

@@ -1,4 +1,11 @@
-import { useRef, useCallback, useState, useEffect, ReactNode } from 'react';
+import {
+  useRef,
+  useCallback,
+  useState,
+  useEffect,
+  ReactNode,
+  ReactElement
+} from 'react';
 import styled from 'styled-components';
 import { DeckInternal, DeckRef, TemplateFn } from '../deck/deck';
 import { Text, SpectacleLogo } from '../../index';
@@ -29,7 +36,7 @@ const PreviewSlideWrapper = styled.div<{ visible?: boolean }>(
   })
 );
 
-const PresenterMode = (props: PresenterModeProps): JSX.Element => {
+const PresenterMode = (props: PresenterModeProps): ReactElement => {
   const { children, theme, backgroundImage, template } = props;
   const deck = useRef<DeckRef>(null);
   const previewDeck = useRef<DeckRef>(null);

packages/spectacle/src/hooks/use-deck-state.test.ts

@@ -0,0 +1,153 @@
+import { renderHook, act } from '@testing-library/react';
+import useDeckState, { DeckView } from './use-deck-state';
+
+describe('useDeckState', () => {
+  const initialState: DeckView = {
+    slideIndex: 1,
+    stepIndex: 1
+  };
+
+  /**
+   * The INITIALIZE_TO should set the active and pending views
+   * to the values provided in the payload.
+   */
+  it('should handle INITIALIZE_TO action', () => {
+    const { result } = renderHook(() => useDeckState(initialState));
+
+    act(() => {
+      result.current.initializeTo({ slideIndex: 2, stepIndex: 2 });
+    });
+
+    expect(result.current.activeView).toEqual({ slideIndex: 2, stepIndex: 2 });
+    expect(result.current.pendingView).toEqual({ slideIndex: 2, stepIndex: 2 });
+    expect(result.current.initialized).toBe(true);
+  });
+
+  /**
+   * The SKIP_TO action should set the pending view slide index to
+   * the slide index provided by the payload and set the navigation
+   * direction based on a delta of the previous and pending slides.
+   */
+  it('should handle SKIP_TO action', () => {
+    const { result } = renderHook(() => useDeckState(initialState));
+
+    act(() => {
+      result.current.skipTo({ slideIndex: 3 });
+    });
+
+    expect(result.current.navigationDirection).toBe(1);
+    expect(result.current.pendingView.slideIndex).toBe(3);
+  });
+
+  it('should handle SKIP_TO action in reverse', () => {
+    const { result } = renderHook(() =>
+      useDeckState({ slideIndex: 5, stepIndex: 0, slideId: 0 })
+    );
+
+    act(() => {
+      result.current.skipTo({ slideIndex: 3 });
+    });
+
+    expect(result.current.navigationDirection).toBe(-1);
+    expect(result.current.pendingView.slideIndex).toBe(3);
+  });
+
+  /**
+   * The STEP_FORWARD action should increment the pending slide index by 1
+   * and have a positive navigation direction.
+   */
+  it('should handle STEP_FORWARD action', () => {
+    const { result } = renderHook(() => useDeckState(initialState));
+
+    act(() => {
+      result.current.stepForward();
+    });
+
+    expect(result.current.pendingView.stepIndex).toBe(
+      initialState.stepIndex + 1
+    );
+    expect(result.current.navigationDirection).toBe(1);
+  });
+
+  /**
+   * The STEP_FORWARD action should decrement the pending slide index by 1
+   * and have a negative navigation direction.
+   */
+  it('should handle STEP_BACKWARD action', () => {
+    const { result } = renderHook(() => useDeckState(initialState));
+
+    act(() => {
+      result.current.stepBackward();
+    });
+
+    expect(result.current.pendingView.stepIndex).toBe(
+      initialState.stepIndex - 1
+    );
+    expect(result.current.navigationDirection).toBe(-1);
+  });
+
+  /**
+   * The ADVANCE_SLIDE action should increment the pending slide index by 1,
+   * reset the step index, and have a positive navigation direction.
+   */
+  it('should handle ADVANCE_SLIDE action', () => {
+    const { result } = renderHook(() => useDeckState(initialState));
+
+    act(() => {
+      result.current.advanceSlide();
+    });
+
+    expect(result.current.pendingView.slideIndex).toBe(
+      initialState.slideIndex + 1
+    );
+    expect(result.current.pendingView.stepIndex).toBe(0);
+    expect(result.current.navigationDirection).toBe(1);
+  });
+
+  /**
+   * The REGRESS_SLIDE action should decrement the pending slide index by 1,
+   * reset the step index, and have a negative navigation direction.
+   */
+  it('should handle REGRESS_SLIDE action', () => {
+    const { result } = renderHook(() => useDeckState(initialState));
+
+    act(() => {
+      result.current.regressSlide({ stepIndex: 0 });
+    });
+
+    expect(result.current.pendingView.slideIndex).toBe(
+      initialState.slideIndex - 1
+    );
+    expect(result.current.pendingView.stepIndex).toBe(0);
+    expect(result.current.navigationDirection).toBe(-1);
+  });
+
+  /**
+   * The COMMIT_TRANSITION action should set the active slide view
+   * and pending slide view to the payload values.
+   */
+  it('should handle COMMIT_TRANSITION action', () => {
+    const { result } = renderHook(() => useDeckState(initialState));
+
+    act(() => {
+      result.current.commitTransition({ slideIndex: 2, stepIndex: 2 });
+    });
+
+    expect(result.current.activeView).toEqual({ slideIndex: 2, stepIndex: 2 });
+    expect(result.current.pendingView).toEqual({ slideIndex: 2, stepIndex: 2 });
+  });
+
+  /**
+   * The CANCEL_TRANSITION action should cancel the slide transition
+   * by reverting the pending view values to what the current active slide values are.
+   */
+  it('should handle CANCEL_TRANSITION action', () => {
+    const { result } = renderHook(() => useDeckState(initialState));
+
+    act(() => {
+      result.current.cancelTransition();
+    });
+
+    expect(result.current.pendingView).toEqual(result.current.activeView);
+  });
+});

packages/spectacle/src/hooks/use-deck-state.ts

@@ -1,6 +1,7 @@
 import { useReducer, useMemo } from 'react';
 import { merge } from 'merge-anything';
 import { SlideId } from '../components/deck/deck';
+import clamp from '../utils/clamp';
 
 export const GOTO_FINAL_STEP = null as unknown as number;
 
@@ -16,7 +17,7 @@ export type DeckState = {
   pendingView: DeckView;
 };
 
-const initialDeckState: DeckState = {
+export const initialDeckState: DeckState = {
   initialized: false,
   navigationDirection: 0,
   pendingView: {
@@ -49,8 +50,15 @@ function deckReducer(state: DeckState, { type, payload = {} }: ReducerActions) {
         initialized: true
       };
     case 'SKIP_TO':
+      const navigationDirection = (() => {
+        if ('slideIndex' in payload && payload.slideIndex) {
+          return clamp(payload.slideIndex - state.activeView.slideIndex, -1, 1);
+        }
+        return null;
+      })();
       return {
         ...state,
+        navigationDirection: navigationDirection || state.navigationDirection,
         pendingView: merge(state.pendingView, payload)
       };
     case 'STEP_FORWARD':
@@ -109,8 +117,16 @@ export default function useDeckState(userProvidedInitialState: DeckView) {
     { initialized, navigationDirection, pendingView, activeView },
     dispatch
   ] = useReducer(deckReducer, {
-    ...initialDeckState,
-    ...userProvidedInitialState
+    initialized: initialDeckState.initialized,
+    navigationDirection: initialDeckState.navigationDirection,
+    pendingView: {
+      ...initialDeckState.pendingView,
+      ...userProvidedInitialState
+    },
+    activeView: {
+      ...initialDeckState.activeView,
+      ...userProvidedInitialState
+    }
   });
   const actions = useMemo(
     () => ({

packages/spectacle/src/utils/clamp.test.ts

@@ -0,0 +1,38 @@
+import clamp, { toFiniteNumber } from './clamp';
+
+describe('toFiniteNumber', () => {
+  it('should return 0 for NaN', () => {
+    expect(toFiniteNumber(NaN)).toBe(0);
+    expect(toFiniteNumber(Number.NaN)).toBe(0);
+  });
+
+  it('should convert finite values to finite numbers', () => {
+    expect(toFiniteNumber(123)).toBe(123);
+    expect(toFiniteNumber(-456.789)).toBe(-456.789);
+  });
+
+  it('should return Number.MAX_SAFE_INTEGER for Infinity', () => {
+    expect(toFiniteNumber(Infinity)).toBe(Number.MAX_SAFE_INTEGER);
+    expect(toFiniteNumber(-Infinity)).toBe(-Number.MAX_SAFE_INTEGER);
+  });
+});
+
+describe('clamp', () => {
+  it('should return NaN for NaN input', () => {
+    expect(clamp(NaN)).toBeNaN();
+  });
+
+  it('should clamp value to specified range', () => {
+    expect(clamp(10, 0, 5)).toBe(5);
+    expect(clamp(-3, -10, 0)).toBe(-3);
+    expect(clamp(7, 5, 10)).toBe(7);
+    expect(clamp(-15, -10, 10)).toBe(-10);
+  });
+
+  it('should not clamp when range is not specified', () => {
+    expect(clamp(10)).toBe(10);
+    expect(clamp(-3)).toBe(-3);
+    expect(clamp(7)).toBe(7);
+    expect(clamp(-15)).toBe(-15);
+  });
+});