fkhadra / react-toastify

React notification made easy 🚀 !
https://fkhadra.github.io/react-toastify/introduction
MIT License
12.58k stars 692 forks source link

Toast Update: enableMultiContainer breaks it #855

Open shehi opened 1 year ago

shehi commented 1 year ago

When enableMultiContainer is enabled and we have multiple ToastContainers, toast.update() doesn't work:

https://codesandbox.io/s/react-toastify-update-bug-forked-1nkwyp?file=/src/index.js

shehi commented 1 year ago

Please note that, in the example, we are dealing with "default" container (the one without explicit ID set to it). If containerId is set, then it works.

shehi commented 1 year ago

EDIT: @fkhadra , did you delete your reply? This post is my response to your reply:

Hi, You forgot to add containerId to toast.update and toast.loading. Function should look like this:

MY REPLY

But why? What is the point of last bulletpoint here then:

Note: adding enableMultiContainer prop to the <ToastContainer/ > will:

  • ...
  • Render any toast if both the toast and <ToastContainer/ > does not include containerId and containerId respectively.

This way, a ToastContainer without explicit containerId defined becomes de-facto default. Toasts created with no containerId end up in this "default" container. And it works.

Now what one would expect here is for those "default" toasts to also properly update. Am I missing something here?

cgibin commented 1 year ago

I agree, toast.update() works not as expected, at least for me. We have a lot of uses of default container and occasional use of another one, with specific containerId. Now we have a need to update default one in some scenario and we cannot do it easily, because it wont work without containerId. To do that we need either somehow add it to every use of default toast, or make another container for that one case, which is not that great.

fkhadra commented 1 year ago

Hey @shehi, dunno what happened with this issue. That being said, I agree with you that the documentation is not accurate and misleading. I'll update it.

I did not foresee a case where one would use multiple containers, one with an id and one without.

When using multiple containers, you should provide a containerId for every container. Imagine the following case

<ToastContainer containerId="1" enableMultiContainer />
<ToastContainer  enableMultiContainer />
<ToastContainer enableMultiContainer />

I have no way of telling which one is the default one.

shehi commented 1 year ago

Well, the concept of "default" by default means one item right? So, you pick the first ToastContainer without Id and use it as default, regardless how many such containers exist. You can even emit a warning when more-than-one container without an id is detected. I believe this use case would add to the strength of this package.

fkhadra commented 1 year ago

Fair enough @shehi, if I do what you suggested, this is doable indeed. I'll add that as a feature enhancement

So, you pick the first ToastContainer without Id and use it as default, regardless how many such containers exist. You can even emit a warning when more-than-one container without an id is detected.

shehi commented 1 year ago

Hey @fkhadra , any idea how long would it take to have this feature ready? Roughly is good enough for me, as in - a month? two? more? Thanks a lot for your answer in advance! I understand time is quite expensive resource, so no rush is implied.

rus-yurchenko commented 11 months ago

I totally agree with the initial issue. I have two containers with the enableMultiContainer prop: one with the specified ID to display the toats of a particular kind and one - without, like the default for everything else.

Following the docs: Render any toast if both the toast and <ToastContainer/ > does not include containerId and containerId respectively. It worked as expected until I needed to update those "default" toasts.

Based on the answers above, it's true that you can render all toats under non-specified container, but you can't update those toasts. What is the reason for such different behavior?