Allow onClick to be passed from a NotificationStack prop#97
Allow onClick to be passed from a NotificationStack prop#97Gargron wants to merge 1 commit intopburtchaell:masterfrom
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) |
pburtchaell
left a comment
pburtchaell
left a comment
There was a problem hiding this comment.
Thanks for the rebase! I have a requested change and then I can get this merged. 😄
| 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.
| ); | ||
| } | ||
|
|
||
| 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. |
2 participants
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.