Allow "onClose" and "onExited" callbacks

[nocksapp]

As global for all snacks on the SnackbarProvider and for single snacks created with enqueueSnackbar

[iamhosseindhv]

@nocksapp Could you give an example of a scenario where having access to onClose and onExit callbacks is useful.

[nocksapp]

@nocksapp Could you give an example of a scenario where having access to onClose and onExit callbacks is useful.

Well, I needed the onClose callback to manipulate state after a snackbar is closed. There are definitely other ways to do that but I think the onClose approach is a clean way.

I created the PR more because I think that onClose should "just" work. And the docs say: "All material-ui Snackbar props will get passed down to a Snackbar component". But that is not true for the onClose and onExited props. With this PR you can add the onClose and onExited props like you would do in the material-ui Snackbar.

[iamhosseindhv]

This will be merged for the next version of notistack. Thanks for the contribution @nocksapp ❤️

[iamhosseindhv]

@nocksapp Functionality has been added to master branch, and will be included in the next release. Thanks for reporting this and the PR. ❤️

[zsh1313]

I have latest version 0.4.1 installed through npm, I don't see this PR is merged.
Which version is it going to be merged to

[iamhosseindhv]

@zsh1313 It has already been merged. Check this commit out 728e004

[zsh1313]

@iamhosseindhv In commit a53a91d, the parameters in onClose callback is (event, reason, key), but in 728e004, the parameter is only (key).
The reason I prefer the changes in a53a91d is that I need to know the onClose reason, so that I can do something programmably if reason is "timeout", otherwise do nothing.
Also the current implementation with commit 728e004 is against the docs as it says: "All material-ui Snackbar props will get passed down to a Snackbar component" as mentioned by @nocksapp .
Anyway, is there way to know the onClose reason in my custom onClose callback func with the current implementation?

[iamhosseindhv]

I see your point @zsh1313. However, there're only two possible reasons: timeout and clickaway. notistack returns on clickaway event.

The reason I prefer the changes in a53a91d is that I need to know the onClose reason, so that I can do something programmatically if reason is "timeout", otherwise do nothing.

so basically notistack does the "otherwise do nothing" part of your work, and the function you pass only gets called if the reason is timeout.

@nocksapp was right about what he mentioned previously, as at the time there was no support for onClose and onExited props.

with the current implementation there isn't a way of accessing reason. lmk if you can think of a use case where having access to reason is beneficial.

[zsh1313]

@iamhosseindhv

so basically notistack does the "otherwise do nothing" part of your work, and the function you pass only gets called if the reason is timeout.

It is not quite true that the custom callback func gets called ONLY if the reason is "timeout". I debugged the notistack code and found if there is action button and when clicking it, the reason is "undefined" and the passed onClose func gets called too. So basically there are two cases that the callback gets called: timeout & clicking action button.

What I can think the benefits if pass the (event, reason) in the custom callback is like below

1. knows the exact reason about the closing.
2. The material-ui only has two reasons at the moment, but it could have more reasons in the future
3. With event parameter, we can identify where the notification is from, therefore knows which case is this closing about. Say I have a single global (centra) notification component (say named NOTIFIER) with notistack, any component (say 30 components) in the application may fire a notification through NOTIFIER with their own action. And I may have another global component to post-process when user clicks the notification action. With the event parameter, we can identify the case (or another word which origin component) where this notification was fired.

Hope that makes sense

[iamhosseindhv]

Fair enough @zsh1313. didn't know reason can be undefined. I based my argument on material docs.

Anyhow, so line 21-23 would be?:

if (reason === 'clickaway') return;
if (singleOnClose) singleOnClose(event, reason, key);
onClose(event, reason, key);

(and event will get passed to onExited callback as well)

[zsh1313]

That looks good @iamhosseindhv .
Better update the document as well so that we know there is an extra parameter (key) in callback

[iamhosseindhv]

👍🏼Appreciate if you could open a new issue for this. The fix will be published in the next version. @zsh1313

[zsh1313]

@iamhosseindhv Sorry for late reply. I have created a new issue about what we have discussed

[iamhosseindhv]

@zsh1313 Thanks 👍🏼