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

Problem with Dynamic Width #40

Closed ibmjanneme closed 4 years ago

ibmjanneme commented 4 years ago

Hello.

First of all thank you for this great extension. We are using jqcloud2 in our AngularJS project. It is invoked in the following way: <jqcloud words="..." height="..." autoResize="true"/> i. e. with autoResize and without the width specified. According to the documentation the width should be driven by the width of the container.

The problem is that when the container width is being set by jqcloud2 in jQCloud.prototype.initialize() the container still doesn't have it's final width (even though we are not setting its width dynamically - it is based only on media queries in the boostrap CSS). This results in an incorrectly formatted wordcloud.

I propose the following solution. In the module code I can see that the autoResize is solved as the window resize event. However nowadays with AJAX driven web pages the wordcloud size can easily change even without window resize (open/close a side pane, resize a modal window where the cloud is displayed etc.). So it would be more convenient for the module user to have the autoResize functionality bound to the container size change rather than to the window size change. Unfortunately such event is not natively supported by JavaScript. It can be managed by setTimeout() like this. In jqcloud.js


- replace

if (this.options.autoResize) { $(window).off('resize.' + this.data.namespace); }

by

if (this.options.autoResize) { clearInterval(this.resizeCheckInterval); }


I have tested this approach and it works fine for me. However I am aware that using `setInterval()` is not the best approach. There exist truly event driven solutions for capturing the element resize event (based on generating some hidden elements inside the container and binding events to it) but it is rather complex.

Please let me know what do you think about the fix proposed and whether you are about to include this or a different kind of fix to your distribution or we should rather use our "hacked" version.
mistic100 commented 4 years ago

Hello, I don't want this kind of solution in the lib, it's ugly and might have horrible performances for sure users.

Until JS offers a size detector, each user will have to handle it the best way suited for its usage.

Sent from MailDroid

-----Original Message----- From: ibmjanneme notifications@github.com To: mistic100/jQCloud jQCloud@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Sent: sam., 28 sept. 2019 9:18 Subject: [mistic100/jQCloud] Problem with Dynamic Width (#40)

Hello.

First of all thank you for this great extension. We are using jqcloud2 in our AngularJS project. It is invoked in the following way: <jqcloud words="..." height="..." autoResize="true"/> i. e. with autoResize and without the width specified. According to the documentation the width should be driven by the width of the container.

The problem is that when the container width is being set by jqcloud2 in jQCloud.prototype.initialize() the container still doesn't have it's final width (even though we are not setting its width dynamically - it is based only on media queries in the boostrap CSS). This results in an incorrectly formatted wordcloud.

I propose the following solution. In the module code I can see that the autoResize is solved as the window resize event. However nowadays with AJAX driven web pages the wordcloud size can easily change even without window resize (open/close a side pane, resize a modal window where the cloud is displayed etc.). So it would be more convenient for the module user to have the autoResize functionality bound to the container size change rather than to the window size change. Unfortunately such event is not natively supported by JavaScript. It can be managed by setTimeout() like this. In jqcloud.js


- replace

if (this.options.autoResize) { $(window).off('resize.' + this.data.namespace); }

by

if (this.options.autoResize) { clearInterval(this.resizeCheckInterval); }


I have tested this approach and it works fine for me. However I am aware that using `setInterval()` is not the best approach. There exist truly event driven solutions for capturing the element resize event (based on generating some hidden elements inside the container and binding events to it) but it is rather complex.

Please let me know what do you think about the fix proposed and whether you are about to include this or a different kind of fix to your distribution or we should rather use our "hacked" version.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/mistic100/jQCloud/issues/40
ibmjanneme commented 4 years ago

Hello.

Thanks a lot for a prompt reply. I fully understand that you don't want to have this kind of code in the library.

Regards.

Jan