search

angular-ui / ui-scroll

Unlimited bidirectional scrolling over a limited element buffer for AngularJS applications
http://angular-ui.github.io/ui-scroll/demo/
MIT License
327 stars 107 forks source link

Index with decimal... #218

Closed priandsf closed 5 years ago

priandsf commented 5 years ago

Not sure if this is a bug or not, but it sometimes calls the datasource with a non integer number, like 19.4 (and count=10). In that case, should we round it down to 19 but then, should we increment the count as well?

dhilt commented 5 years ago

Thanks for the issue! Having analysed it shallowly, I would say following. This https://github.com/angular-ui/ui-scroll/blob/05896d8950bd8cb444719c0e1d0d0342f2a87f3f/src/ui-scroll.js#L154 and that https://github.com/angular-ui/ui-scroll/blob/05896d8950bd8cb444719c0e1d0d0342f2a87f3f/src/ui-scroll.js#L164 are the only places where index are being passed to the datasource.get. The buffer.next and buffer.first are being maintained by src/modules/buffer.js and there are only ++, --, =1 and =startIndex operations. So I'm wondering how did you get this non-decimal index value...

Maybe I'm missing something, so it needs to be debugged, what code is responsible for breaking index integrity. If you want me to debug, the best option would be to make a plunkr/jsfiddle/stackblitz demo reproducing the issue.

priandsf commented 5 years ago

Thank Denis. I need to debug through it which I'll do this week-end. It seems that the issue happens after a reload, where we changed the data source and the #of rows. The debugger shows, for example. buffer.first, maxIndex, minIndex all set a the value like "22.6". I'll try to find out how this happens image

dhilt commented 5 years ago

Your screenshot makes me think about

https://github.com/angular-ui/ui-scroll/blob/05896d8950bd8cb444719c0e1d0d0342f2a87f3f/src/modules/buffer.js#L7-L17

So, speaking of reload, I would recommend to investigate startIndex argument value...

Anyway, I think it is a good idea to protect "start-index" attribute and Adapter.reload(startIndex) argument from non-decimal values right in the ui-scroll code. I see it's not safe now, it is fully under user responsibility, which could be improved. Putting enhancement label.

priandsf commented 5 years ago

Yep this is I actually did and this is the culprit. So nothing on your side, unless you might want to make it robust calling Math.floor(). Thanks for your help, this is an awesone library!

dhilt commented 5 years ago

angular-ui-scroll v1.7.3 has been released, it includes "start-index" attribute and Adapter.reload method argument pre-validation