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

Error with material-UI Alpha-v5 #295

Closed
misshu1 opened this issue Aug 12, 2020 · 25 comments
Closed

Error with material-UI Alpha-v5 #295

misshu1 opened this issue Aug 12, 2020 · 25 comments

Comments

@misshu1
Copy link

misshu1 commented Aug 12, 2020

I'm getting the following error when the notification is shown:

Material-UI: The key `container` provided to the classes prop is not implemented in ForwardRef(Collapse).
You can only override one of the following: root,horizontal,entered,hidden,wrapper,wrapperInner.

Link: https://codesandbox.io/s/notistack-simple-example-forked-1b301

Using the alpha v5 from material UI and react 17

@misshu1 misshu1 changed the title Error with material-UI Alphav5 Error with material-UI Alpha-v5 Aug 12, 2020
@michael-land
Copy link

Using the alpha v5 from material UI and react 16.13.1

@ghost
Copy link

ghost commented Aug 18, 2020

Same here using alpha 6, saw it on Stack Overflow too

@Lukas-Kullmann
Copy link
Contributor

In alpha 6, snackbars will actually never go close.
I think this is due to the onE events (onEntered, onExited, ...) being removed from the Snackbar component. But they are used in the SnackbarItem component: /~https://github.com/iamhosseindhv/notistack/blob/v0.9.17/src/SnackbarItem/SnackbarItem.tsx#L188-L194

@michael-land
Copy link

Yeah, it's unusable on alpha.6

@iamhosseindhv
Copy link
Owner

iamhosseindhv commented Aug 22, 2020

Yes in MUI alpha, these callbacks have been moved to TransitionProps of the snackbar component.

- <Snackbar onExit={() => {}} />
+ <Snackbar TransitionProps={{ onExit: () => {} }} />

At this moment there are no plans for notistack to be compatible with alpha releases (but only stable releases)

@iamhosseindhv iamhosseindhv added the wontfix This will not be worked on label Aug 22, 2020
@kikoanis
Copy link

Yes in MUI alpha, these callbacks have been moved to TransitionProps of the snackbar component.

- <Snackbar onExit={() => {}} />
+ <Snackbar TransitionProps={{ onExit: () => {} }} />

At this moment there are no plans for notistack to be compatible with alpha releases (but only stable releases)

Could you just start on a next branch so we know where is this going?

@michael-land
Copy link

I just made a quick fix for Material-UI V5 alpha since @iamhosseindhv won't fix it right now, use it at your own risk.

yarn add notistack-v5

@kikoanis
Copy link

kikoanis commented Aug 23, 2020

I've been working on switching to react-toastify today. Notistack has been great for me but since it depends on mateiral-ui, this won't be the last time. I'd rather not to deal with this anymore.

@iamhosseindhv
Copy link
Owner

Could you just start on a next branch so we know where is this going?

@kikoanis Branch future which is already 30 commits ahead of master, started after I received enough upvotes in #184.

Removing MUI dependency has been the plan since very long time ago, requires lots of thoughts and patience, as well as back and forth emails with the MUI team.

As the owner and maintainer of notistack, I'm responsible for making the transition as smooth as possible for everyone. I'm sure non of you want to spend 1h doing the migration. Looking at the changelog, you can see the trails of small, incremental progress. In the meantime, I'm committed to make sure notistack is production-ready, by being compatible with stable releases of MUI.

@kikoanis
Copy link

@iamhosseindhv totally understandable. I really appreciate your hard work on the library. I am still keeping my old code just in case. It's been my number one for toasts for a long time. I am in for MUI 5 for a good reasons related to date pickers.

@cansin
Copy link

cansin commented Aug 28, 2020

I have just tried using the future branch, but looks like this issue is not fixed there yet.

@oliviertassinari
Copy link
Contributor

oliviertassinari commented Sep 3, 2020

I think that the best way to avoid future occurrences of this problem in the alpha and beta lifespan of v5, is to include the features of notistack directly in Material-UI. From what I understand notistack brings 3 major features:

  • imperative API
  • stacking
  • bundled alert's variants

These features seem to be almost standard in the industry. They should likely come with Material-UI's Snackbar, as they do from many design systems:

If you look at their implementations, all* use a custom solution. It would make a lot of sense for Material-UI too as we are pushing in the unstyled direction for v5.

@iamhosseindhv It would be awesome to consider bringing these features, I think that we can wait a bit for the migration to emotion and unstyled support to be more mature. We could also consider exposing hooks for it.

@mnemanja
Copy link

mnemanja commented Sep 15, 2020

So, how do we progress now with this?
Is the general recommendation to switch away from notistack to something else?

@iamhosseindhv
Copy link
Owner

Options @mnemanja:

  1. Wait for future release of notistack,
  2. Wait for a material-ui release while @oliviertassinari (MUI-Team) and I work on it,
  3. Contribute. to either of the options above,
  4. Use alternative notification library.

@cansin
Copy link

cansin commented Sep 15, 2020

@mnemanja you could temporarily rely on notistack-next (5.0.0-alpha.7-2 or 5.0.0-alpha.9) which I forked from notistack in order to make it compatible with material-ui alpha. I will try to create a PR back to notistack once the alpha period is over. Here is the current changeset for reference: /~https://github.com/iamhosseindhv/notistack/compare/master...cansin:next?expand=1

@italodeandra
Copy link

italodeandra commented Oct 23, 2020

Thanks @cansin.

Just for reference, for people who wants to use your "next" package without breaking old code:

npm install notistack@npm:notistack-next
yarn add notistack@npm:npm:notistack-next

@vinkovsky
Copy link

Same. Has this problem been resolved?

@iamhosseindhv iamhosseindhv removed the wontfix This will not be worked on label Nov 19, 2020
@evasyuk
Copy link

evasyuk commented Nov 20, 2020

Same. Has this problem been resolved?

@xiaoyu-tamu created "hot-fix". worked for me. note: it was a pet-project, but not real production app

you can try it with:
yarn add notistack-v5

@iamhosseindhv
Copy link
Owner

Please update to v1.0.2. Fixed in #333

Let me know if the there're any issues.

@kevinwolfcr
Copy link

@iamhosseindhv still happening on v1.0.2

image

@iamhosseindhv
Copy link
Owner

Yup I mentioned that issue remains in the PR above.

You can now use npm i notistack@next to have a version compatible with MUI next.

@oliviertassinari
Copy link
Contributor

oliviertassinari commented Nov 27, 2020

@iamhosseindhv Great! I think that we should look into updating the dependency for the demo in https://next.material-ui.com/components/snackbars/#notistack.

We have recently completed the migration of the Slider to the unstyled approach. @mnajdova is working on a migration guide for the rest of the codebase, it would probably be good to experiment with the unstyled snackbar.

@iamhosseindhv
Copy link
Owner

Thanks @oliviertassinari. I think at this very moment we wouldn't achieve much by updating next.material-ui.com to use notistack@next. The main problems (transition callbacks, theme.palette.mode instead of type) are already fixed in notistack@1.0.2.

Although It certainly makes sense to do so when an unstyled version of snackbar/notistack is ready.

@artus9033
Copy link

This issue seems to still exist with notistack@next (resolved to 1.0.4-alpha.0) & material-ui@5.0.0-alpha.19

@artus9033
Copy link

@iamhosseindhv would you reopen, please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests