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

Added a onClosed and my suggestion for a fix on #105 #136

Closed
wants to merge 1 commit into from
Closed

Added a onClosed and my suggestion for a fix on #105 #136

wants to merge 1 commit into from

Conversation

beardedvikingdev
Copy link

Hi,

Please check / accept my pull request.

I've added a option called 'onClosed' to use if the close button is pressed. Also I saw a filed bug (issue #105) which I thought you could fix it by removing the handler for the mouseleave event, after clicking the remove button.

I tested this on the latest version of Chrome and Firefox, also on IE10 and IE11. Not sure how you expect contributions because I didn't find them on the docs, cause after running grunt your comments from css where removed and some changes where made.

Regards Dave

@johnpapa
Copy link
Member

I'll look into this.

@beardedvikingdev
Copy link
Author

Okay, thanks

@beardedvikingdev
Copy link
Author

Hi John,

Did you get a chance to look into this?

Kind regards Dave

@beardedvikingdev
Copy link
Author

Hi @johnpapa, I'm not trying to push you, but did you get a chance to take a look at this :)

Kind regards Dave

@johnpapa
Copy link
Member

Not deeply yet. Though I am not sure why you need this at all since you can just hook up any event handler to any element in the toast. So it seems this is sugar, and I would like to keep the API trim as can be. In other words, you can do this without adding code to toastr and instead just hooking up an event handler. Or am I missing what you want?

@beardedvikingdev
Copy link
Author

Hi @johnpapa, It is related to issue #105 and indeed you can hook up an extra event handler, but I think that if the event triggers a callback more than once this could be sometimes annoying. About the callback on closing that could be sugar, but handling both of these via the API would make more sense to me.

@johnpapa
Copy link
Member

I get it. But I think since this can be done through the existing API I'll close it for now. I dont want to extend the core unless needed

@johnpapa johnpapa closed this Apr 29, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants