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

Return (event, reason) in onClose callback #46

Closed
zsh1313 opened this issue Jan 8, 2019 · 1 comment
Closed

Return (event, reason) in onClose callback #46

zsh1313 opened this issue Jan 8, 2019 · 1 comment
Labels
must have really? it's 2019 and still we don't have this feature?

Comments

@zsh1313
Copy link

zsh1313 commented Jan 8, 2019

onClose callback only return (key) at the moment, but it will be very useful if the original (event, reason) are also returned.
The benefits being that passing the (event, reason) in the custom onClose 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.

some discussion has gone through this in Allow "onClose" and "onExited" callbacks. The changes to the code as mentioned by @iamhosseindhv looks good

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

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

@iamhosseindhv
Copy link
Owner

Thanks for opening the issue. This will be in the next release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
must have really? it's 2019 and still we don't have this feature?
Projects
None yet
Development

No branches or pull requests

2 participants