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 custom HTML alert support #129

Closed
wants to merge 2 commits into from
Closed

Add custom HTML alert support #129

wants to merge 2 commits into from

Conversation

huytd
Copy link

@huytd huytd commented Oct 16, 2014

Add new alert type: HTML

EDIT: now using allowHtml option instead of type

This will allow creating HTML alert box, can be use to create custome dialog types, for example: Prompt, Date picker,...

Usage:

swal({   
    title: "Custom dialog!",   
    text: "Here's my <b>HTML</b> <u>message</u>!",   
    type: "success",   
    confirmButtonText: "Yay!",
    allowHtml: true
});

@samacs
Copy link

samacs commented Oct 16, 2014

Wouldn't it be better if there's another option, allowHtml for instance, and keep the type as the original array?

@pomartel
Copy link

I agree with @samacs that html is not a type of dialog, it is a different feature completely. I could want to make an error dialog with html within. Unfortunately I don't think that the project owner wants to allow HTML in the alerts at all.

@samacs
Copy link

samacs commented Oct 16, 2014

Looking at the code, seems very easy to support this feature, but the modal's markup is out of the scope of the parameters so we cannot decide how to define the markup if it will allow HTML. I just glanced the code so I cannot tell if this is completely true. I'll make a pr in my free time and push the branch.

@samacs
Copy link

samacs commented Oct 16, 2014

Well, if that's the case, @pomartel lets fork it and add the feature -.-

@pomartel
Copy link

Maybe there is something I don't understand but why would we want to escape the HTML at all? Why not give total freedom to the developer to put what he wants in the alert? In my opinion, this should not even be an option so I just removed escapeHtml call in my branch : https://github.com/pomartel/sweetalert/commit/5756a87d0a0726e947d89e3f8a6f675e4b005be5

@samacs
Copy link

samacs commented Oct 16, 2014

(y)

@huytd
Copy link
Author

huytd commented Oct 17, 2014

Yeah, you guys are right. It should be another option (allowHtml), I'm sooooo lazzzyyyyy 💃
I see @t4t5 put the escapeHtml everywhere, may be he wanted to prevent XSS attack or something.
How do I change my submited pull request? Or just make another pull request?

@pomartel
Copy link

@huytd that should probably apply to the title as well.

@huytd
Copy link
Author

huytd commented Oct 17, 2014

I added a commit to use allowHtml option instead of type :D

@huytd huytd changed the title Add custom HTML alert type support Add custom HTML alert support Oct 17, 2014
@lipis
Copy link
Contributor

lipis commented Oct 22, 2014

In that particular PR you shouldn't change/fix the icon thingy as it is totally separate subject and should be in a separete PR

@lipis lipis mentioned this pull request Oct 22, 2014
@vmitchell85
Copy link

With HTML support you're bound to put lots of other things in the alert. Which creates issues if the alert becomes bigger than the window height. Add scrolling with the HTML support or a separate issue?

@pomartel
Copy link

@vmitchell85 I think the main use case for html in the alert is to add links in the text or basic styling tags. I don't see what's the added value of stripping html tags. Why not let the developer choose what he wants to put in his alerts?

@vmitchell85
Copy link

@pomartel - I wasn't advocating not using HTML, I am 100% for HTML... my concern is that if you put enough items into the alert and the window is small you currently can't scroll (as I've downloaded this code change and tested)

@Ashwinning
Copy link

Hey,
I used the script and it works great - except for one thing : Input fields are not clickable on chrome.
I'm passing the escaped HTML with an input field, and the field shows up fine.
According to the inspector, -webkit-user-select is set to 'text' in the elements, so that's fine too.
Works fine on Firefox.
Thanks!

@kai23
Copy link

kai23 commented Dec 3, 2014

Same error as @Ashwinning : did you managed to fix it ?

@giroin
Copy link

giroin commented Dec 17, 2014

Hi,
I also confirm that allowhtml works however what happened to timer function? it seems to be removed

@limonte
Copy link

limonte commented Jan 8, 2015

@huytd I added this feature and some other additional options in sweetalert2.

@t4t5
Copy link
Owner

t4t5 commented Jan 18, 2015

This is now possible through the htmlattribute. Thanks for your contributions and opinions folks! :)

@t4t5 t4t5 closed this Jan 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants