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

Add an option to bind html content in message #12

Closed
NetanelBasal opened this issue May 12, 2015 · 6 comments
Closed

Add an option to bind html content in message #12

NetanelBasal opened this issue May 12, 2015 · 6 comments

Comments

@NetanelBasal
Copy link

Please add option that we be able to add html content in the message.

@matowens
Copy link
Owner

Hey there @NetanelBasal, as far as I understand, I believe binding html content here would require an additional dependency - ng-sanitize if I remember correctly. My initial thoughts have been to keep this super lean and simple, and allow others to modify when needed to fit their particular needs. I think it might be best to leave that bit out of this module and allow those that would like to add it to their specific projects do so via a fork.

Just my thoughts, how does that sound? Are you aware of another way to do something like this without requiring an additional module be included?

Would others prefer to bind html and require the sanitize module here or would most prefer to keep it lean and simple with the ability to make any tweaks they'd like on their own?

@NetanelBasal
Copy link
Author

Most of the cases the developer will set the message, not the client, so i does not think its really danger ,You can do this is with innerHTML and let the user consider if he want to use it, just put an option like html:true or something, cause this is important than we be able to insert there things like links and extra..

@gaetancollaud
Copy link

Hi,

I think html content would be a great idea too.

ngSanitize is indeed the best solution to implement this.

Like @NetanelBasal suggested, I think the best way to display HTML messages is to have an option when we use set(). Lke this, we don't alter your current API, we just add something.

ngNotify.set('<b>Error</b> : you are wrong', {
    type: 'error',
    html:true
});

@matowens
Copy link
Owner

Hey there @gaetancollaud, my hesitation with adding ngSantize directly to the ngNotify module is that anyone who currently uses this and pulled down the latest update would have breaking changes introduced if they didn't also include ngSanitize.

I played around a little bit with this, too, and have added in a check for the ngSanitize script. If it exists, we dynamically inject it, and if it doesn't exist, we gracefully degrade back to using HTML escaped messages. The use would be the same as in your example, setting the 'html' attribute to true. If you set 'html' to true and ngSanitize doesn't exist, we shoot out a debug message in the console reminding the developer and continue using escaped HTML messages.

This should continue to work the same way for those that are already using it as well as give the added option for HTML notifications for those who decide to include ngSanitize in their app. Let me know what you guys think.

I'm going to run it through a few more tests when I can dig up some free time but it looks like it's going to work out.

@matowens
Copy link
Owner

Just ran through tests and pushed an update, v0.6.0.

https://github.com/matowens/ng-notify/blob/master/CHANGELOG.md

Thanks guys!

@gaetancollaud
Copy link

Sounds good for me. Thank you !

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

No branches or pull requests

3 participants