Allow onClick to be passed from a NotificationStack prop #97
Conversation
|
Hi @Gargron, thanks for the PR. CI is failing. Can you update this PR to pass tests? |
|
Sorry about this, submitted a quick fix via the GitHub website and turns out my syntax wasn't on point. I'll need to properly check out the repo so it'll have to wait until tomorrow... |
|
Okay, ready. |
|
@Gargron Can you rebase this? Once it's rebased, I can merge. Thanks! |
|
@Gargron Following up: can you rebase this? Once it's rebased, I can merge. Thanks! |
|
@pburtchaell (It's rebased) |
| @@ -22,6 +24,7 @@ const NotificationStack = props => ( | |||
| const isLast = index === 0 && props.notifications.length === 1; | |||
| const barStyle = props.barStyleFactory(index, notification.barStyle); | |||
| const activeBarStyle = props.activeBarStyleFactory(index, notification.activeBarStyle); | |||
| const onClick = (notification.onClick || props.onClick) || defaultClick; | |||
There was a problem hiding this comment.
Could you change this to just notification.onClick || props.onClick? Instead of using defaultClick you can use a default prop value.
| @@ -10,6 +10,8 @@ function defaultStyleFactory(index, style) { | |||
| ); | |||
| } | |||
|
|
|||
| const defaultClick = () => {}; | |||
There was a problem hiding this comment.
This can be replaced with a default prop value.
|
@Gargron Following up: I requested one change. Once that is completed, I can marge the PR. Thanks! |
|
Resolved 041dfe6. |
This permits a Redux container approach to NotificationStack where you get access to the dispatch function from a mapDispatchToProps object rather than mapStateToProps where the notifications themselves would be coming from.