Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking change in Snackbar onExited behavior in 4.12 #28339

Closed
2 tasks done
fzaninotto opened this issue Sep 14, 2021 · 9 comments
Closed
2 tasks done

Breaking change in Snackbar onExited behavior in 4.12 #28339

fzaninotto opened this issue Sep 14, 2021 · 9 comments
Labels
component: snackbar This is the name of the generic UI component, not the React module! v4.x

Comments

@fzaninotto
Copy link
Contributor

fzaninotto commented Sep 14, 2021

It's impossible to use Snackbar in 4.12 with a custom onExited callback that works the same as in 4.11 without deprecation warnings.

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

Current Behavior 😯

The prop <Snackbar onExited> was deprecated in 4.12. We are now supposed to pass <Snackbar TransitionProps={{ onExited }} > instead to prepare 5.0 compatibility. This is fine and all, except the 2 behaviors aren't equivalent.

The Snackbar code in 4.x reads like this:

https://github.com/mui-org/material-ui/blob/77a8daeed0e8efa67c6d86f3cc40239dc6eb3d94/packages/material-ui/src/Snackbar/Snackbar.js#L235-L249

This means that if a callback is passed via onExited, it will be executed together with the default handleExited. But if it's passed via TransitionProps, it will be executed alone, and the default handleExited won't be executed.

The problem is that the default handleExited cannot be reproduced in a custom callback:

https://github.com/mui-org/material-ui/blob/77a8daeed0e8efa67c6d86f3cc40239dc6eb3d94/packages/material-ui/src/Snackbar/Snackbar.js#L195-L197

As a consequence, I can't use the new Snackbar syntax, and I can't get rid of the deprecation warnings...

Note that the problem doesn't exist in 5.0 (see https://github.com/mui-org/material-ui/blob/master/packages/mui-material/src/Snackbar/Snackbar.js).

Expected Behavior 🤔

Full compatibility without deprecation warnings between 4.11 and 4.12

Context 🔦

I want to remove those pesky deprecation warnings in react-admin (cf marmelab/react-admin#6451)

@fzaninotto fzaninotto added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Sep 14, 2021
@eps1lon
Copy link
Member

eps1lon commented Sep 15, 2021

Could you provide a repro instead of interpreting source code for us? A repro helps us better understand the issue.

@eps1lon eps1lon added component: snackbar This is the name of the generic UI component, not the React module! status: waiting for author Issue with insufficient information v4.x and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 15, 2021
@eps1lon
Copy link
Member

eps1lon commented Oct 1, 2021

Closing since no reproduction was provided within 14 days.

@eps1lon eps1lon closed this as completed Oct 1, 2021
@jaminthorns
Copy link
Contributor

Here's a reproduction of the issue: https://codesandbox.io/s/material-demo-forked-5vtzo

@jaminthorns
Copy link
Contributor

@eps1lon Forgot to tag you with my previous message. I've provided a reproduction of the issue above.

@kgregory
Copy link
Member

kgregory commented Jan 11, 2022

@jaminthorns I ran into this too. Thanks for supplying the reproduction.

Seems like some work needs to be done to TransitionProps before it's spread.

Something like this:

const amendedTransitionProps = {
  ...TransitionProps,
  onEnter: TransitionProps.onEnter && createChainedFunction(handleEnter, TransitionProps.onEnter),
  onExited: TransitionProps.onExited && createChainedFunction(handleExited, TransitionProps.onExited),
};

For now I'm going to live with the warning until this is fixed or my team can upgrade to v5.

@ryancogswell
Copy link
Contributor

ryancogswell commented Feb 21, 2022

I just ran into this as well. Following the instructions for onExited in the deprecation causes an invisible Snackbar to remain after it "closes" and it then prevents clicking whatever is behind it.

Here's a reproduction demonstrating how content underneath where the Snackbar was opened becomes unclickable: https://codesandbox.io/s/material-demo-forked-g7hzjb?file=/demo.js. This demo allows the Snackbar to be opened more than once by ignoring the "clickaway" close reason which triggers instantly on open attempts after the first open (whereas you can only open @jaminthorns's Snackbar once). I've added a background color on the Snackbar root to make it more obvious that it doesn't completely close.

import React, { useState } from "react";
import Button from "@material-ui/core/Button";
import Snackbar from "@material-ui/core/Snackbar";

export function BrokenSnackbar() {
  const [open, setOpen] = useState(false);
  console.log("open", open);
  return (
    <div>
      <Button onClick={() => setOpen(true)}>Open broken snackbar</Button>
      <Snackbar
        style={{ backgroundColor: "#ddd" }}
        open={open}
        message="After opening, I don't completely go away"
        anchorOrigin={{ vertical: "bottom", horizontal: "left" }}
        autoHideDuration={2000}
        onClose={(event, reason) => {
          if (reason === "clickaway") {
            console.log("clickaway happens instantly on opens after the first");
            return;
          }
          setOpen(false);
        }}
        TransitionProps={{
          onExited: () => {
            console.log("onExited");
          }
        }}
      />
      <Button
        style={{ position: "fixed", left: 16, bottom: 24 }}
        onClick={() => console.log("clicked!")}
      >
        Button covered by Snackbar
      </Button>
    </div>
  );
}

I don't know if you care about fixing this at this point, but reopening for now since it was closed due to the lack of a reproduction and it impacts people who are trying to gradually transition from v4 to v5 by first going to the final v4 version and addressing deprecations.

@ryancogswell ryancogswell reopened this Feb 21, 2022
@github-actions
Copy link

github-actions bot commented Mar 2, 2022

Since the issue is missing key information, and has been inactive for 7 days, it has been automatically closed.
If you wish to see the issue reopened, please provide the missing information.

@github-actions github-actions bot closed this as completed Mar 2, 2022
@ryancogswell ryancogswell reopened this Mar 2, 2022
@ryancogswell ryancogswell removed the status: waiting for author Issue with insufficient information label Mar 2, 2022
@ryancogswell
Copy link
Contributor

I'm removing the "status: incomplete" label since it caused a bot to close the issue and I don't believe it is still missing information -- it just hasn't been reviewed again since the info was added.

@mbrookes
Copy link
Member

v4 is no longer in active development.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: snackbar This is the name of the generic UI component, not the React module! v4.x
Projects
None yet
Development

No branches or pull requests

6 participants