mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.97k stars 32.27k forks source link

[Snackbar] Please describe how to easily set the slide direction for a toast #21053

Closed dandv closed 3 years ago

dandv commented 4 years ago

I'm running into another instance of simple things being super-complicated :)

What I'm trying to do is simply display a toast sliding up. I've tried following the Control slide direction documentation section, but it doesn't actually explain how to control the direction; instead, it gives a complicated example with variable directions, and maybe it's too late in the day or I haven't had enough coffee, but I haven't been able to figure out how to set up one simple sliding direction.

And I'm not the only one, see this or #16637.

By comparison, in other UI toolkits, you simply set the direction property as a string.

oliviertassinari commented 4 years ago

@dandv Thanks for reporting this case, I don't see any improvement opportunity. We document how to use a different transition component, with its option. I think that going in the direction of Grommet is a no go, it breaks composition and tree-shaking, without gaining much more simplicity.

Do you see a change in the documentation that could have helped?

dandv commented 4 years ago

Indeed it was too late, and I figured it out this morning:

<Snackbar
  ...
  TransitionComponent={props => <Slide {...props} direction="down" />}
>

What would've helped me is the clarity of documenting how to set up a specific transition (like with the example above), before giving the example with functions returning functions. Most of the time, developers want one transition type in the code, and the reality is that most people copy/paste examples.

oliviertassinari commented 4 years ago

@dandv Avoid the above, you will create a new component at each render, breaking the animation.

Maybe we should add a plain example, just before the demo?

function TransitionLeft(props) {
  return <Slide {...props} direction="left" />;
}

export default function MyComponent() {
  return <Snackbar TransitionComponent={TransitionLeft} />;
}
dandv commented 4 years ago

Yes! A plain example is what I was after, and what I think every component and option should have.

It would also help people avoid traps like the one I fell in, with creating the transition on every render.

dandv commented 4 years ago

Another "first principles" thought on toasts, maybe @dtassone can also opine:

The state-based paradigm feels ill-suited for toasts, IMO. Toasts are displayed imperatively as a result of an action, as opposed to changing their state (shown/hidden/slid out) in response to events. They're not really reactive.

To me at least, it would feel far easier and more intuitive to launch a toast from an onClick handler, rather than have it always present as a component and toggle its open state. The code would be far simpler too: onClick={toast(...options)}; no more separate Snackbar component wrapping a MUIAlert with a Transition parameter and open/close handlers for what ultimately is just an autohiding message with a background.

This is the shortest MUI code I could come up with for displaying a toast:

function Transition(props): ReactElement {
  return <Slide {...props} direction="down" />;
}

const [toastOpenState, setToastOpenState] = React.useState(false);

function handleToastClose(): void {
  setToastOpenState(false);
}

return (
  <Snackbar
    anchorOrigin={{ vertical: 'top', horizontal: 'center' }}
    autoHideDuration={3000}
    TransitionComponent={Transition}
    open={toastOpenState}
    onClose={handleSignUpToastClose}
  >
    <MuiAlert
      severity={myCondition ? 'success' : 'error'}
      variant="filled" elevation={6}
      onClose={handleToastClose}
    >
      {myCondition ? "Success!" : "Error :(" }
    </MuiAlert>
  </Snackbar>
   ...
 );

I know I'm new to MUI and not writing the best code (for example there are two handlers for the same close button?), and I realize the imperative approach that other frameworks take (example) doesn't fit the general approach of MUI; hence, just a thought.

oliviertassinari commented 4 years ago

@dandv you are no the first one to raise concerns about making the toasts easier to use. You can find notistack that fills some of the problems by @iamhosseindhv.

I have been tracking the features we could support with the snackbar in the future in https://trello.com/c/tMdlZIb6/1989-snackbar-more-features:

In the react community, I believe https://github.com/fkhadra/react-toastify is the most used component.

dandv commented 4 years ago

Thanks for listening to my repeated concerns, esp. given how contrarian they have been :)

sravani1906 commented 3 years ago

@dandv Avoid the above, you will create a new component at each render, breaking the animation.

Maybe we should add a plain example, just before the demo?

function TransitionLeft(props) {
  return <Slide {...props} direction="left" />;
}

export default function MyComponent() {
  return <Snackbar TransitionComponent={TransitionLeft} />;
}

@oliviertassinari I am willing to add this example to the guide. Can you please let me know if its still needed? happy to submit a PR

oliviertassinari commented 3 years ago

@sravani1906 It's still needed, the idea was to do this:

diff --git a/docs/src/pages/components/snackbars/snackbars.md b/docs/src/pages/components/snackbars/snackbars.md
index 67bcc1caf4..85fd850b77 100644
--- a/docs/src/pages/components/snackbars/snackbars.md
+++ b/docs/src/pages/components/snackbars/snackbars.md
@@ -61,10 +61,22 @@ Snackbars should appear above FABs (on mobile).

 {{"demo": "pages/components/snackbars/FabIntegrationSnackbar.js", "iframe": true, "maxWidth": 400}}

-### Change Transition
+### Change transition

 [Grow](/components/transitions/#grow) is the default transition but you can use a different one.

+```jsx
+import Slide from '@material-ui/core/Slide';
+
+function TransitionLeft(props) {
+  return <Slide {...props} direction="left" />;
+}
+
+export default function MyComponent() {
+  return <Snackbar TransitionComponent={TransitionLeft} />;
+}
+```
+
 {{"demo": "pages/components/snackbars/TransitionsSnackbar.js"}}

 ### Control Slide direction
BagchiMB commented 3 years ago

Hi @oliviertassinari, I am a newbie to open source, as this issue was inactive for more than 7 days, I thought of giving it a shot. I read the contribution guidelines, and was able to set up my project locally. Right now a weird thing is happening, as soon as I tried navigating to Snackbar component on my local server, it threw an error, attaching a screenshot.

I am very new to this, sorry if it is some naive mistake on my end.

Screenshot (325)

oliviertassinari commented 3 years ago

@BagchiMB Try to clean up your state:

  1. Make sure you are on next and on the latest commit
  2. Make sure all the dependencies are installed (yarn)
  3. Empty the cache of Next.js rm -rf docs/.next

Outside of this, I have no ideas, I won't be able to help.

BagchiMB commented 3 years ago

Hi @oliviertassinari , I was not able to get it done by the steps you mentioned, thanks for your help, on further investigation I found this is not working for any component, not only snackbar

ansh-saini commented 3 years ago

@oliviertassinari I'm facing the same issue. It's not just happening with the snackbar. It is happening with every component. I console logged the demo in MarkdownDocs.js and it was undefined.

--- a/docs/src/modules/components/MarkdownDocs.js
+++ b/docs/src/modules/components/MarkdownDocs.js 
    const name = renderedMarkdownOrDemo.demo;
    const demo = demos?.[name];
+   console.log({ demo,  demos })
    if (demo === undefined) {
    const errorMessage = [
        `Missing demo: ${name}. You can use one of the following:`,

By logging the demos variable I found the demo paths to contain backslashes, pages/\\src\\... and when I performed the diff below, the docs started working for that component. Don't exactly know what led to this issue. I was playing around with the code on next branch few days ago and it was working fine. PS: I'm on windows 10.

index c4d326d0af..48dc184456 100644
--- a/docs/src/pages/components/box/box.md
+++ b/docs/src/pages/components/box/box.md
The Box component packages [all the style functions](/system/basics/#all-inclusi
All system properties are available via the [`sx` prop](/system/basics/#the-sx-prop).
In addition, the `sx` prop allows you to specify any other CSS rules you may need. Here's an example of how you can use it:

-{{"demo": "pages/components/box/BoxSx.js", "defaultCodeOpen": true }}
+{{"demo": "pages/\\src\\pages\\components\\box/BoxSx.js", "defaultCodeOpen": true }}

## Overriding Material-UI components

The Box component wraps your component.
It creates a new DOM element, a `<div>` by default that can be changed with the `component` prop.
Let's say you want to use a `<span>` instead:

-{{"demo": "pages/components/box/BoxComponent.js", "defaultCodeOpen": true }}
+{{"demo": "pages/\\src\\pages\\components\\box/BoxComponent.js", "defaultCodeOpen": true }}

This works great when the changes can be isolated to a new DOM element.
For instance, you can change the margin this way.
oliviertassinari commented 3 years ago

Ok, then it comes from #26774

ansh-saini commented 3 years ago

@BagchiMB I guess for now, you can perform the same "hack" in the snackbar.md that I did with box, and start working on it.

BagchiMB commented 3 years ago

good catch @ansh-saini, I will make those changes and will also track changes on #26774 as mentioned by @oliviertassinari. Thank you guys! Cheers!

goncalovf commented 3 years ago

I went ahead and proposed a solution with the PR above. I should have asked for permission before doing it, but since it was such a quick fix, I went ahead with it. I hope I didn't do anything wrong.

bensonmk commented 2 years ago

@dandv Avoid the above, you will create a new component at each render, breaking the animation.

Maybe we should add a plain example, just before the demo?

function TransitionLeft(props) {
  return <Slide {...props} direction="left" />;
}

export default function MyComponent() {
  return <Snackbar TransitionComponent={TransitionLeft} />;
}

what is the props property here?