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

Code change to decrease population time of large tables when resizable is used #662

Closed
rolinex opened this issue Jun 30, 2014 · 5 comments
Closed

Comments

@rolinex
Copy link

rolinex commented Jun 30, 2014

I changed the code (version 2.17.2) of resizable widget to decrease population time of large tables during my testing. I would like to know if the code change is ok and if so if it can be incorporated into the source.

I moved calculation below from format()

fullWidthInit = false,
fullWidth = 0,
// fullWidth = Math.abs($table.parent().width() - $table.width()) < 20,

to .bind('mousemove.tsresize', function(event)

if (fullWidthInit === false) {
fullWidth = Math.abs($table.parent().width() - $table.width()) < 20;
fullWidthInit = true;
}

@Mottie
Copy link
Owner

Mottie commented Jun 30, 2014

Hi @rolinex!

The widget uses a format function to keep it compatible with older versions of tablesorter; otherwise I would have put all of that code into the init widget function. Although I haven't tested the widgets in the original version of tablesorter in quite some time... so maybe it isn't worth trying to maintain that compatibility.

Anyway, if you look at the first two lines in the format function you'll see this:

if (c.$table.hasClass('hasResizable')) { return; }
c.$table.addClass('hasResizable');

So, the code change you propose would actually add more time because the mousemove event gets triggered quite often in some browsers while the browser window is being resized.

@rolinex
Copy link
Author

rolinex commented Jun 30, 2014

I thought it would be calculated only once since the changed code checks if fullWidth was initialised. I only want to delay the calculation until resize is used. The change seems to cut down few seconds from the initial population time when the table has eg 1000rows on IE9.

@Mottie
Copy link
Owner

Mottie commented Jun 30, 2014

This calculation only happens once

fullWidth = Math.abs($table.parent().width() - $table.width()) < 20

That translates into, take the absolute value of the element wrapping the table minus the table width, then see if that is less than 20 (basically detecting if the table takes up the full width of its container).... there is no way that takes a "few seconds" to calculate, even in IE.

What might help improve the speed in IE is throttling the mousemove event, but then it might not react as smoothly. I'll do some testing and see how that works out; I may even add an option to enable/disable throttling.

@thezoggy
Copy link
Collaborator

thezoggy commented Jul 1, 2014

there are libs for that..
https://github.com/briancherne/jquery-hoverIntent

@Mottie
Copy link
Owner

Mottie commented Jul 1, 2014

I plan to incorporate the method into the code. I don't want to add a dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants