Fix: redraw top and bottom should acknowledge edge conditions

[daveduchene]

Since nobody else is complaining about this, my guess that this is happening to us due to the various ways that we're abusing/misusing DataTables (simulating infinite scroll, using fixed columns, sizing through CSS).

This changeset ensures that a redraw is triggered when we scroll to the bottom of the virtual area.

[DataTables]

Thanks for the PR!

Could you link to a test case which shows the problem please so I can confirm the issue and the fix.

Also, if you could confirm that you are happy for your changes to be included under the current MIT license, that would be great.

[daveduchene]

Thanks for making DataTables available! It's a pleasure to read through and work with.

I am happy for the changes to be included under terms of the MIT license, and will try to isolate a test case. However I think it's going to be hard to do so without significant effort or violating my employers IP rules. One way or another, I'll let you know.

[DataTables]

Yup - I understand. I fairness the change looks fairly trivial - certainly for redrawTop, I'd be willing to just take that one. But I'd like to understand the redrawBottom one a bit more.

Also it looks like you added iScrollHeight but it isn't used. Is that correct?

[daveduchene]

Argh, that's what I get for "cleaning up" before I submit. Line 799 should read:

this.s.redrawBottom = Math.min(iScrollTop + boundaryPx, iScrollHeight - iScrollTop);

As you can probably tell, I'm not used to Githib's UI at all, but can try to update the PR.

[daveduchene]

Looking deeper, that isn't right either - it will draw too often.

A little more about my reasoning:

My understanding is that a redraw is supposed to be triggered when this.dom.scroller.scrollTop is larger than this.s.redrawBottom (line 597). Given that, I think that this.s.redrawBottom should be, at most, the height of the scroller minus the height of the visible area, which is not iScrollHeight - iScrollTop. My PR "Worked" because is always set this.s.redrawBottom to a value that is too small. I'll see if I can get it right, and come back with that.

[daveduchene]

I believe this actually captures my intent:

this.s.redrawBottom = Math.min(iScrollTop + boundaryPx, iScrollHeight - heights.viewport);

[DataTables]

Yeah - these calculations are darn tricky, which is why I was wondering if you might be able to bash together a test case so I can try it out. Thanks for following up on this - looks like it could be a good fix to have in Scroller!

[daveduchene]

Ok, I think this should do it:

http://jsbin.com/jademecuqe/1/edit?html,css,js,console,output

Sorry for the messiness. Things to note:

• The problem seems to be sensitive to the height of the container, and also to the any displayBuffer setting. Rapidly scrolling to the bottom of the buffer a couple of times in Chrome / OS X will reproduce it with for me.
• usePatchedCalculations will toggle between the proposed calculation and the one currently in master.

[daveduchene]

As a side benefit, there's a mostly working proof of concept for enabling fixed columns and scroller together there. I'd appreciate any advice you could offer for making sure that the vertical offsets between the two tables stay in sync a little more robustly.