Material-ui: [Snackbar] Auto hide timer restarts when parent component re-renders

Created on 13 Nov 2019  路  2Comments  路  Source: mui-org/material-ui

  • [x] The issue is present in the latest release.
  • [x] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 馃槸


When the parent component re-renders before the autoHideDuration has elapsed, the Snackbar stays open. onClose is not called with the timeout reason.

Expected Behavior 馃


If the Snackback autoHideDuration and onClose props don't change, it should call onClose with the timeout reason after autoHideDuration passes.

Steps to Reproduce 馃暪

Steps:

  1. Create a component which displays a timer that counts up every 1000ms
  2. Within that component, create a Snackbar with autoHideDuration=2000ms

https://codesandbox.io/s/hidden-waterfall-75ulu

( The Snackbar hides 2000ms after the timer is disabled: https://codesandbox.io/s/fervent-golick-fw1hg )

Your Environment 馃寧

| Tech | Version |
| ----------- | ------- |
| Material-UI | v4.6.1 |
| React | v16.11.0|

bug 馃悰 Snackbar good first issue

All 2 comments

@ambersz Thank you for the report. Right, it comes from the new reference of onClose you provide at each render, it resets the logic. We should be able to work around it, and simplify the logic with:

diff --git a/packages/material-ui/src/Snackbar/Snackbar.js b/packages/material-ui/src/Snackbar/Snackbar.js
index 5bf3e555e..b17def76a 100644
--- a/packages/material-ui/src/Snackbar/Snackbar.js
+++ b/packages/material-ui/src/Snackbar/Snackbar.js
@@ -4,6 +4,7 @@ import clsx from 'clsx';
 import withStyles from '../styles/withStyles';
 import { duration } from '../styles/transitions';
 import ClickAwayListener from '../ClickAwayListener';
+import useEventCallback from '../utils/useEventCallback';
 import capitalize from '../utils/capitalize';
 import createChainedFunction from '../utils/createChainedFunction';
 import Grow from '../Grow';
@@ -130,37 +131,26 @@ const Snackbar = React.forwardRef(function Snackbar(props, ref) {
   const [exited, setExited] = React.useState(true);

   // Timer that controls delay before snackbar auto hides
-  const setAutoHideTimer = React.useCallback(
-    autoHideDurationParam => {
-      const autoHideDurationBefore =
-        autoHideDurationParam != null ? autoHideDurationParam : autoHideDuration;
-
-      if (!onClose || autoHideDurationBefore == null) {
-        return;
-      }
+  const setAutoHideTimer = useEventCallback(autoHideDurationParam => {
+    if (!onClose || autoHideDurationParam == null) {
+    if (!onClose || autoHideDurationParam == null) {
+      return;
+    }

-      clearTimeout(timerAutoHide.current);
-      timerAutoHide.current = setTimeout(() => {
-        const autoHideDurationAfter =
-          autoHideDurationParam != null ? autoHideDurationParam : autoHideDuration;
-        if (!onClose || autoHideDurationAfter == null) {
-          return;
-        }
-        onClose(null, 'timeout');
-      }, autoHideDurationBefore);
-    },
-    [autoHideDuration, onClose],
-  );
+    clearTimeout(timerAutoHide.current);
+    timerAutoHide.current = setTimeout(() => {
+      onClose(null, 'timeout');
+    }, autoHideDurationParam);
+  });

   React.useEffect(() => {
     if (open) {
-      setAutoHideTimer();
+      setAutoHideTimer(autoHideDuration);
     }

     return () => {
       clearTimeout(timerAutoHide.current);
     };
-  }, [open, setAutoHideTimer]);
+  }, [open, autoHideDuration, setAutoHideTimer]);

   // Pause the timer when the user is interacting with the Snackbar
   // or when the user hide the window.

What do you think? Do you want to submit a pull request? We could a test case at the same time.

I'm working on a PR for this problem :)

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ghost picture ghost  路  3Comments

reflog picture reflog  路  3Comments

iamzhouyi picture iamzhouyi  路  3Comments

mb-copart picture mb-copart  路  3Comments

sys13 picture sys13  路  3Comments