fix(Toast): Reset autohide timer only if show or autohide changed (#5220

) * fix(Toast): Reset autohide timer only if show or autohide changed * Update src/Toast.js Co-authored-by: Jimmy Jia <tesrin@gmail.com> * Pre-calc autohide toast condition * Update Toast.js * format Co-authored-by: Jimmy Jia <tesrin@gmail.com>
react-bootstrap · Jun 22, 2020 · 20b532d · 20b532d
1 parent 501b3bf
commit 20b532d
Show file tree

Hide file tree

Showing 2 changed files with 147 additions and 62 deletions.
diff --git a/src/Toast.js b/src/Toast.js
@@ -1,4 +1,4 @@
-import React, { useEffect, useRef, useMemo, useCallback } from 'react';
+import React, { useEffect, useMemo, useRef, useCallback } from 'react';
 import PropTypes from 'prop-types';
 import classNames from 'classnames';
 
@@ -71,30 +71,39 @@ const Toast = React.forwardRef(
     ref,
   ) => {
     bsPrefix = useBootstrapPrefix(bsPrefix, 'toast');
+
+    // We use refs for these, because we don't want to restart the autohide
+    // timer in case these values change.
     const delayRef = useRef(delay);
     const onCloseRef = useRef(onClose);
 
     useEffect(() => {
-      // We use refs for these, because we don't want to restart the autohide
-      // timer in case these values change.
       delayRef.current = delay;
       onCloseRef.current = onClose;
     }, [delay, onClose]);
 
     const autohideTimeout = useTimeout();
+    const autohideToast = !!(autohide && show);
+
     const autohideFunc = useCallback(() => {
-      if (!(autohide && show)) {
-        return;
+      if (autohideToast) {
+        onCloseRef.current();
       }
-      onCloseRef.current();
-    }, [autohide, show]);
+    }, [autohideToast]);
 
-    autohideTimeout.set(autohideFunc, delayRef.current);
+    useEffect(() => {
+      // Only reset timer if show or autohide changes.
+      autohideTimeout.set(autohideFunc, delayRef.current);
+    }, [autohideTimeout, autohideFunc]);
+
+    const toastContext = useMemo(
+      () => ({
+        onClose,
+      }),
+      [onClose],
+    );
 
-    const hasAnimation = useMemo(() => Transition && animation, [
-      Transition,
-      animation,
-    ]);
+    const hasAnimation = !!(Transition && animation);
 
     const toast = (
       <div
@@ -113,10 +122,6 @@ const Toast = React.forwardRef(
       </div>
     );
 
-    const toastContext = {
-      onClose,
-    };
-
     return (
       <ToastContext.Provider value={toastContext}>
         {hasAnimation ? (

diff --git a/test/ToastSpec.js b/test/ToastSpec.js
@@ -4,6 +4,16 @@ import { mount } from 'enzyme';
 import Toast from '../src/Toast';
 
 describe('<Toast>', () => {
+  let clock;
+
+  beforeEach(() => {
+    clock = sinon.useFakeTimers();
+  });
+
+  afterEach(() => {
+    clock.restore();
+  });
+
   it('should render an entire toast', () => {
     mount(
       <Toast>
@@ -44,58 +54,128 @@ describe('<Toast>', () => {
   });
 
   it('should trigger the onClose event after the autohide delay', () => {
-    const clock = sinon.useFakeTimers();
-
-    try {
-      const onCloseSpy = sinon.spy();
-      mount(
-        <Toast onClose={onCloseSpy} delay={500} show autohide>
-          <Toast.Header>header-content</Toast.Header>
-          <Toast.Body>body-content</Toast.Body>
-        </Toast>,
-      );
-      clock.tick(1000);
-      expect(onCloseSpy).to.have.been.calledOnce;
-    } finally {
-      clock.restore();
-    }
+    const onCloseSpy = sinon.spy();
+    mount(
+      <Toast onClose={onCloseSpy} delay={500} show autohide>
+        <Toast.Header>header-content</Toast.Header>
+        <Toast.Body>body-content</Toast.Body>
+      </Toast>,
+    );
+    clock.tick(1000);
+    expect(onCloseSpy).to.have.been.calledOnce;
   });
 
   it('should not trigger the onClose event if autohide is not set', () => {
-    const clock = sinon.useFakeTimers();
-
-    try {
-      const onCloseSpy = sinon.spy();
-      mount(
-        <Toast onClose={onCloseSpy}>
-          <Toast.Header>header-content</Toast.Header>
-          <Toast.Body>body-content</Toast.Body>
-        </Toast>,
-      );
-      clock.tick(3000);
-      expect(onCloseSpy).not.to.have.been.called;
-    } finally {
-      clock.restore();
-    }
+    const onCloseSpy = sinon.spy();
+    mount(
+      <Toast onClose={onCloseSpy}>
+        <Toast.Header>header-content</Toast.Header>
+        <Toast.Body>body-content</Toast.Body>
+      </Toast>,
+    );
+    clock.tick(3000);
+    expect(onCloseSpy).not.to.have.been.called;
   });
 
   it('should clearTimeout after unmount', () => {
-    const clock = sinon.useFakeTimers();
-
-    try {
-      const onCloseSpy = sinon.spy();
-      const wrapper = mount(
-        <Toast delay={500} onClose={onCloseSpy} show autohide>
-          <Toast.Header>header-content</Toast.Header>
-          <Toast.Body>body-content</Toast.Body>
-        </Toast>,
-      );
-      wrapper.unmount();
-      clock.tick(1000);
-      expect(onCloseSpy).not.to.have.been.called;
-    } finally {
-      clock.restore();
-    }
+    const onCloseSpy = sinon.spy();
+    const wrapper = mount(
+      <Toast delay={500} onClose={onCloseSpy} show autohide>
+        <Toast.Header>header-content</Toast.Header>
+        <Toast.Body>body-content</Toast.Body>
+      </Toast>,
+    );
+    wrapper.unmount();
+    clock.tick(1000);
+    expect(onCloseSpy).not.to.have.been.called;
+  });
+
+  it('should not reset autohide timer when element re-renders with same props', () => {
+    const onCloseSpy = sinon.spy();
+    const wrapper = mount(
+      <Toast delay={500} onClose={onCloseSpy} show autohide>
+        <Toast.Header>header-content</Toast.Header>
+        <Toast.Body>body-content</Toast.Body>
+      </Toast>,
+    );
+
+    clock.tick(250);
+
+    // Trigger render with no props changes.
+    wrapper.setProps({});
+
+    clock.tick(300);
+    expect(onCloseSpy).to.have.been.calledOnce;
+  });
+
+  it('should not reset autohide timer when delay is changed', () => {
+    const onCloseSpy = sinon.spy();
+    const wrapper = mount(
+      <Toast delay={500} onClose={onCloseSpy} show autohide>
+        <Toast.Header>header-content</Toast.Header>
+        <Toast.Body>body-content</Toast.Body>
+      </Toast>,
+    );
+
+    clock.tick(250);
+
+    wrapper.setProps({ delay: 10000 });
+
+    clock.tick(300);
+    expect(onCloseSpy).to.have.been.calledOnce;
+  });
+
+  it('should not reset autohide timer when onClosed is changed', () => {
+    const onCloseSpy = sinon.spy();
+    const onCloseSpy2 = sinon.spy();
+    const wrapper = mount(
+      <Toast delay={500} onClose={onCloseSpy} show autohide>
+        <Toast.Header>header-content</Toast.Header>
+        <Toast.Body>body-content</Toast.Body>
+      </Toast>,
+    );
+
+    clock.tick(250);
+
+    wrapper.setProps({ onClose: onCloseSpy2 });
+
+    clock.tick(300);
+    expect(onCloseSpy).not.to.have.been.called;
+    expect(onCloseSpy2).to.have.been.calledOnce;
+  });
+
+  it('should not call onClose if autohide is changed from true to false', () => {
+    const onCloseSpy = sinon.spy();
+    const wrapper = mount(
+      <Toast delay={500} onClose={onCloseSpy} show autohide>
+        <Toast.Header>header-content</Toast.Header>
+        <Toast.Body>body-content</Toast.Body>
+      </Toast>,
+    );
+
+    clock.tick(250);
+
+    wrapper.setProps({ autohide: false });
+
+    clock.tick(300);
+    expect(onCloseSpy).not.to.have.been.called;
+  });
+
+  it('should not call onClose if show is changed from true to false', () => {
+    const onCloseSpy = sinon.spy();
+    const wrapper = mount(
+      <Toast delay={500} onClose={onCloseSpy} show autohide>
+        <Toast.Header>header-content</Toast.Header>
+        <Toast.Body>body-content</Toast.Body>
+      </Toast>,
+    );
+
+    clock.tick(250);
+
+    wrapper.setProps({ show: false });
+
+    clock.tick(300);
+    expect(onCloseSpy).not.to.have.been.called;
   });
 
   it('should render with bsPrefix', () => {