search

mistic100 / jQCloud

jQuery plugin for drawing neat word clouds that actually look like clouds
mistic100.github.io/jQCloud
MIT License
268 stars 106 forks source link

Add template functionality to enable custom HTML formatting of words in ... #3

Closed thinkjones closed 9 years ago

thinkjones commented 9 years ago

...tag clouds. I needed to add icons with some of my tag words, using the post alter function ended up with some bad word wrapping, so I added a tempalte functionality where you can set the HTML directly.

mistic100 commented 9 years ago

Why don't just replace "text" by "html" on line 328 (and 325 too) and manage your template one level up ?

I would prefer this over adding an option on jQCloud (if I understand your usage, template is for example an underscore template, and text is an object)

mistic100 commented 9 years ago

and please don't push "dist" (amend your last commit and do a force push)

thinkjones commented 9 years ago

Changing text to html on lines 325, 328, imho would be a breaking change. Some people maybe relying on the text being encoded. I think it's safer and more customizable to offer a template function for people to amend the tag anyway they want. This is similar to how Kendo works - http://docs.telerik.com/kendo-ui/api/javascript/ui/combobox#configuration-template.

mistic100 commented 9 years ago

Some people maybe relying on the text being encoded.

very unlikely... who would want < and > showing up ? And honestly I don't mind breaking changes, I'll bump to 2.1 and it will be ok

mistic100 commented 9 years ago

Kendo uses a global template, it makes more sense that a template for each word, which is just like directly provide HTML

thinkjones commented 9 years ago

very unlikely... who would want < and > showing up ?

Anybody that would want that functionality? Why restrict them? There's probably a reason jQuery provides .text and .html because it doesn't want to dictate what method you should use.

Kendo uses a global template

Agree this would work better on the options object.

mistic100 commented 9 years ago

I'm not building a generic usage library, I'm building a plugin which I try to keep as simple as possible. Keeping a option (I talk about "text", allowing HTML is great) for 0.05% of users is a nonsense.

If you move the template option, un-commit "dist" and squash your commits I'll merge though.

mistic100 commented 9 years ago

Well sorry to bother you.

What is funny is that if someone had raised an issue saying "hey you use text instead of html and I can't put icons on my tags" I would have made the change straight away and nobody would have complained.

I'll reuse your idea though.

thinkjones commented 9 years ago

I am making the change at the moment. New pull request coming shortly.