adding the ability for notifications to auto hide when they close

[BSd3v]

keeps from showing up unexpectedly

[AnnMarieW]

how about making it {onClose, ...others} incase we get the functions as props working in a future version?

[BSd3v]

At that point, we will need to rework this anyways. As you wont be able to declare the name twice, plus this default behaviour should stay unless explicitly dont not to auto remove from the tree... At which time, this would trigger the event passed to the props while maintaining the original removal.

[AnnMarieW]

Closes #522

Thanks for the PR @BSd3v I ran it with the sample app in issue 522 and it solves the problem. What do you think about adding a test? Also need a changelog entry

[BSd3v]

@alexcjohnson,

What are your thoughts on adding a prop for a dev to be able to opt out of this? Where the prop would stay as they added without it auto hide for the action.

Not sure why the dev would want this behaviour though...

[alexcjohnson]

This does not need an opt-out. If it did, to be consistent we should in fact re-show the notification immediately when it's closed, because internally it still thinks its action is show even though it has been closed. The root of the issue is we're operating this halfway between a controlled and an uncontrolled component, but your fix makes it closer to a fully-controlled component (even if it's redundant control because the close has already happened).

The only thing I'm wondering is if hide is really the right new action, or if it should be none or something? I guess what this is pointing to is this component is written in a weird way - action isn't really a prop, it's, well, an action. It's a change of state, but given that React expects to be able to rerender to its heart's content without anything changing if it's given consistent props, it doesn't really work to have a prop that represents a change. Ideally the prop should just be "is this notification visible or not", then the component or the notification system figures out when it sees some props, does this represent a new component to show, a currently-visible component I need to hide, or an existing component I need to update.

Anyway that's probably a bigger task than you want to do right now... it must already work (ie be a no-op) to re-show an already-visible notification or you'd have seen that bug pop up way sooner, so this seems like it'll work if people use it in simple ways, but there are very likely other bugs hiding out in more complex use cases.

So I'd say go for it, merge this as a fix but down the line we can think about a cleaner pattern.

[BSd3v]

Believe this issue stems from treating an underlying api as a component which is then rerendered by dash.

I recommend that we shift to treating the notifications as new prop of the NotificationProvider and treat it as an api call similar to how rowTransactions work with dash-ag-grid. This should keep the rerendering of the components and the notification system the way that it was designed by Mantine.

---

I also agree that this should be fine the way it is for now. This is a little more work than what we'd like to do currently and this PR solves the immediate issue.

[AnnMarieW]

Thanks @BSd3v and @alexcjohnson
Agreed that this component could use a rework/refactor in the future. Thanks for addressing the bug in this PR! I'll open an issue and link this PR for reference so we can track the improvements separately.