Skip to content
This repository has been archived by the owner on Dec 24, 2019. It is now read-only.

Allow action and onClick for Stacked Notifications #62

Closed
wants to merge 6 commits into from
Closed

Allow action and onClick for Stacked Notifications #62

wants to merge 6 commits into from

Conversation

elrumordelaluz
Copy link
Contributor

Resolves #46.

Add references to timeouts that allows to clear them before unmount each notification.
So, let available to add action and onClick props in Notifications Components inside a Stack. #46

I tryied to let the example as it is (using OrderedSetfrom Immutable.js) but didn't find a way to reference each generated Notification Component inside its own onClick prop, so I changed the example with a raw Object, referenced with an unique id based on Date.now() (with the same advice comment).

stacnotif

Since the Notification Component fires the timer when receive props and verify the (if) condition, it is necessary a `clearTimeout` to avoid not desired behaviour with different timers fired.

Add an extra condition to only apply if has same `onDismiss` so don't brake the Notification Stack behaviour.
- Leave the `action` and `onClick` props to be defines by the Notification component
- Add references to timer to `clearTimeout` when will unmount
- Use an Object to control the Notifications State
- Add a method to handle Remove Notification, invoked from `onDismiss` and `onClick`
@pburtchaell
Copy link
Owner

@elrumordelaluz I'm busy today and tomorrow, but I'll take a look at this on Friday evening.

@pburtchaell
Copy link
Owner

@elrumordelaluz This looks good. Can you rebase so the older commits aren't on the branch? Once you do that I will merge and release!

@pburtchaell pburtchaell added the documentation Documentation and examples label Apr 8, 2016
@pburtchaell
Copy link
Owner

Note, this resolves #46.

@elrumordelaluz
Copy link
Contributor Author

Can you rebase so the older commits aren't on the branch?

I realised that there are duplicated commits in the PR, I'm sorry.

Due to my lack of deep knowledge in Git area I'm trying different ways to solve it asap, then will send to you.

@cliftonc
Copy link

@elrumordelaluz need any further assistance? This is a really useful PR for me, so happy to help if I can.

@elrumordelaluz
Copy link
Contributor Author

@cliftonc oh thank you! I thought there were a pretty simple git thing to solve, but then I found my fork in a little mess. Probably is a simple thing to solve, if you have 5 mins I'll appreciate your support. In that case we could chat.

@elrumordelaluz
Copy link
Contributor Author

Closed because of state problems on my fork.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Documentation and examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants