Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

fast scrolling make grid disappearing (and fix) #309

Open
jsenellart opened this Issue Mar 27, 2012 · 12 comments

Comments

Projects
None yet
7 participants

I had a problem when handling a grid with quite small height (was showing only ~10rows), and scrolling vertically very fast through the list (I am using a touchpad): in some cases, the grid content was completely disappearing. I was not using pagination.

After some debugging, I found that the error is coming from the handleScroll method in slick.grid.js where page calculation could sometime get NaN - which was after propagating to offset, then to range, and was preventing the subsequent call to render to render anymore any row.

find below a patch for that issue (on slick.grid.js - rev 497):
*** /Users/js/DEV/SlickGrid.git/trunk/slick.grid.js 2012-03-27 23:43:31.000000000 +0200
--- slick.grid.js 2012-03-27 23:44:15.000000000 +0200


*** 1557,1563 ****
scrollTo(scrollTop + offset);
} else {
var oldOffset = offset;
! page = Math.min(n - 1, Math.floor(scrollTop * ((th - viewportH) / (h - viewportH)) * (1 / ph)));
offset = Math.round(page * cj);
if (oldOffset != offset) {
invalidateAllRows();
--- 1557,1566 ----
scrollTo(scrollTop + offset);
} else {
var oldOffset = offset;
! if (h == viewportH)
! page = n-1;
! else
! page = Math.min(n - 1, Math.floor(scrollTop * ((th - viewportH) / (h - viewportH)) * (1 / ph)));
offset = Math.round(page * cj);
if (oldOffset != offset) {
invalidateAllRows();

Owner

mleibman commented Mar 28, 2012

I'm having a hard time reading the patch.

On Tue, Mar 27, 2012 at 2:57 PM, Jean Senellart <
reply@reply.github.com

wrote:

I had a problem when handling a grid with quite small height (was showing
only ~10rows), and scrolling vertically very fast through the list (I am
using a touchpad): in some cases, the grid content was completely
disappearing. I was not using pagination.

After some debugging, I found that the error is coming from the
handleScroll method in slick.grid.js where page calculation could sometime
get NaN - which was after propagating to offset, then to range, and was
preventing the subsequent call to render to render anymore any row.

find attached a patch for that issue.


Reply to this email directly or view it on GitHub:
#309

Sorry - I didn't find how to attach it properly to github. Here is the
patch - it is just protecting the case where h==viewportH in
(slick.grid.js, handleScroll function):

page = Math.min(n - 1, Math.floor(scrollTop * ((th - viewportH) / (h -

viewportH)) * (1 / ph)));

And it works fine.

Regards,
Jean

Le 28/03/12 23:40, Michael Leibman
<reply+i-3841058-1f196390f555494f60462f0a0658a62cf81d0750-1520492@reply.git
hub.com> a crit :

I'm having a hard time reading the patch.

On Tue, Mar 27, 2012 at 2:57 PM, Jean Senellart <
reply@reply.github.com

wrote:

I had a problem when handling a grid with quite small height (was
showing
only ~10rows), and scrolling vertically very fast through the list (I am
using a touchpad): in some cases, the grid content was completely
disappearing. I was not using pagination.

After some debugging, I found that the error is coming from the
handleScroll method in slick.grid.js where page calculation could
sometime
get NaN - which was after propagating to offset, then to range, and was
preventing the subsequent call to render to render anymore any row.

find attached a patch for that issue.


Reply to this email directly or view it on GitHub:
#309


Reply to this email directly or view it on GitHub:
#309 (comment)

I have a table of about 25 elements using doT as a micro template per item. When using Apple's scroll inertia I am able to scroll so fast that SlickGrid can not keep up. I wonder if I am seeing this same bug? Also are there any plans to fix this?

tp commented Aug 26, 2015

@ktsuttlemyre Seeing the same here. But I can't see any difference between the latest code and the proposed fix by @jsenellart-systran so no idea whether this can easily be fixed.

6pac commented Aug 27, 2015

Have you tried my Alternate Master repo ? I think this might have been fixed there, along with a lot of other minor issues. Check out the wiki for more info.

boutils commented Sep 24, 2015

@6pac I tried your fork and I noticed the same issue. I'm very interested to find a good fix without forking if possible.

6pac commented Sep 24, 2015

Thanks for the feedback. Can any or all or you let me know the conditions to reproduce (ie. hardware, OS, browser) - I'm not clear on if this is tablet or desktop. Seems pretty clear that fast vertical scrolling is the issue.

BTW, my repo is simply a copy of the MLeibman master with updated jQuery versions, and only small bug fixes and enhancements applied. It is designed to be as much 'not a fork' as possible.

boutils commented Sep 25, 2015

@6pac
My conditions: Macbook pro 15" with Chrome v45. You can simply check this exemple: http://mleibman.github.io/SlickGrid/examples/example-optimizing-dataview.html

Try to quiclky scroll (bottom and top) and you should see the grid disappear for a few seconds.

Thanks!

6pac commented Sep 25, 2015

Thanks. Unfortunately my Macbook Pro died two days ago :-( I'll try to replicate it elsewhere, but otherwise I may have to wait a week or so for the replacement to arrive ...

6pac commented Sep 26, 2015

Hey, I've had a look. This is not an error, as in it's not triggering any javascript errors at this stage. It's a performance issue - simply, the grid momentarily can't keep up with the scrolling, but catches up shortly after.
This will be an artifact of how the Macbook sends scrolling information to the grid. To work around it, we'd have to set up logging to find out what's going on with the scrolling input, and then create some process to recognise and shortcut the fast scroll scenario.
Unfortunately, not simple, and there's also the possibility of introducing bugs into other unforeseen scrolling scenarios.
If anyone's keen enough to undertake that, I'd review it and possibly incorporate it into my branch, but I think the easiest way out is to recognise that you have to wait a second for fast scrolling.

danmo commented May 9, 2017

2017 - still needed.

6pac commented May 10, 2017

The original post cited an error that caused the grid to stop rendering altogether, I believe it's fixed in my repo, which you should be using rather than this no-longer-maintained one.

Further in the conversation, it was brought up that the grid disappears for a short time and then reappears once it 'catches up'. This is a normal performance issue for the browser, and is not fixable without performance monitoring and trying to bypass any bottlenecks, a quite serious task in itself.

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