Merge pull request #561 from facultyai/debounce-spinner

Add debounce prop to spinner + tests

Author: tcbegley

src/components/Spinner.js

@@ -1,6 +1,6 @@
-import React, {Fragment} from 'react';
+import React, {useEffect, useRef, useState} from 'react';
 import PropTypes from 'prop-types';
-import {omit, type} from 'ramda';
+import {omit} from 'ramda';
 import {Spinner as RSSpinner} from 'reactstrap';
 import {bootstrapColors} from '../private/BootstrapColors';
 
@@ -21,9 +21,27 @@ const Spinner = props => {
     fullscreen,
     fullscreenClassName,
     fullscreen_style,
+    debounce,
+    show_initially,
     ...otherProps
   } = props;
 
+  const [showSpinner, setShowSpinner] = useState(show_initially);
+  const timer = useRef();
+
+  useEffect(() => {
+    if (loading_state) {
+      if (timer.current) {
+        clearTimeout(timer.current);
+      }
+      if (loading_state.is_loading && !showSpinner) {
+        setShowSpinner(true);
+      } else if (!loading_state.is_loading && showSpinner) {
+        timer.current = setTimeout(() => setShowSpinner(false), debounce);
+      }
+    }
+  }, [loading_state]);
+
   const isBootstrapColor = bootstrapColors.has(color);
 
   const fullscreenStyle = {
@@ -75,8 +93,6 @@ const Spinner = props => {
       ...spinner_style
     };
 
-    const showSpinner = loading_state && loading_state.is_loading;
-
     return (
       <div style={showSpinner ? hiddenStyle : {}}>
         {children}
@@ -105,6 +121,11 @@ const Spinner = props => {
 
 Spinner._dashprivate_isLoadingComponent = true;
 
+Spinner.defaultProps = {
+  debounce: 0,
+  show_initially: true
+};
+
 Spinner.propTypes = {
   /**
    * The ID of this component, used to identify dash components
@@ -162,7 +183,19 @@ Spinner.propTypes = {
    * Boolean that determines if the loading spinner will be displayed
    * full-screen or not.
    */
-  fullscreen: PropTypes.bool
+  fullscreen: PropTypes.bool,
+
+  /**
+   * When using the spinner as a loading spinner, add a time delay (in ms) to
+   * the spinner being removed to prevent flickering.
+   */
+  debounce: PropTypes.number,
+
+  /**
+   * Whether the Spinner should show on app start-up before the loading state
+   * has been determined. Default True.
+   */
+  show_initially: PropTypes.bool
 };
 
 export default Spinner;

src/components/__tests__/Spinner.test.js

@@ -1,7 +1,9 @@
 import React from 'react';
-import {render} from '@testing-library/react';
+import {act, render} from '@testing-library/react';
 import Spinner from '../Spinner';
 
+jest.useFakeTimers();
+
 describe('Spinner', () => {
   test('renders a div with class "border-spinner"', () => {
     const spinner = render(<Spinner />);
@@ -12,21 +14,39 @@ describe('Spinner', () => {
   });
 
   test("renders its content if object isn't loading", () => {
-    const {container: All, rerender} = render(
-      <Spinner>Some spinner content</Spinner>
+    const {container: container, rerender} = render(
+      <Spinner loading_state={{is_loading: false}}>
+        Some spinner content
+      </Spinner>
     );
 
-    expect(All).toHaveTextContent('Some spinner content');
+    // spinner is initially visible until we've had time to update based on
+    // loading state. this can be disabled with show_initially={false}
+    act(() => jest.advanceTimersByTime(10));
+
+    expect(container).toHaveTextContent('Some spinner content');
+    expect(container.querySelector('div.spinner-border')).toBe(null);
 
     rerender(
       <Spinner loading_state={{is_loading: true}}>Some spinner content</Spinner>
     );
 
-    const overAll = All.firstChild;
+    const overAll = container.firstChild;
     const spinner = overAll.lastChild;
 
-    expect(overAll).toHaveTextContent('Some spinner content');
-    expect(spinner.firstChild).toHaveClass('spinner-border');
+    act(() => jest.advanceTimersByTime(10));
+
+    expect(container).toHaveTextContent('Some spinner content');
+    expect(container.querySelector('div.spinner-border')).not.toBe(null);
+  });
+
+  test("doesn't show initially when show_initially is false", () => {
+    const {container: container} = render(
+      <Spinner show_initially={false}>Some spinner content</Spinner>
+    );
+
+    expect(container).toHaveTextContent('Some spinner content');
+    expect(container.querySelector('div.spinner-border')).toBe(null);
   });
 
   test('applies additional CSS classes when props are set', () => {
@@ -64,4 +84,31 @@ describe('Spinner', () => {
     expect(spinnerSuccess).toHaveClass('text-success');
     expect(spinnerDark).toHaveClass('text-dark');
   });
+
+  test('spinner can be debounced with debounce prop', () => {
+    const {container: container, rerender} = render(
+      <Spinner loading_state={{is_loading: true}} debounce={1000}>
+        Some spinner content
+      </Spinner>
+    );
+
+    const overAll = container.firstChild;
+    const spinner = overAll.lastChild;
+
+    expect(overAll).toHaveTextContent('Some spinner content');
+    expect(spinner.firstChild).toHaveClass('spinner-border');
+
+    rerender(
+      <Spinner loading_state={{is_loading: false}} debounce={1000}>
+        Some spinner content
+      </Spinner>
+    );
+
+    expect(overAll).toHaveTextContent('Some spinner content');
+    expect(spinner.firstChild).toHaveClass('spinner-border');
+
+    act(() => jest.advanceTimersByTime(1000));
+
+    expect(container.querySelector('div.spinner-border')).toBe(null);
+  });
 });