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

minIndex/maxIndex not obeyed after adapter.reload() #191

Closed zacronos closed 6 years ago

zacronos commented 6 years ago

Here is a plunker for this issue. When you click the Do Reload button, the min and max items available from the data source are changed, and dataSource.minIndex and dataSource.maxIndex are changed accordingly, and then adapter.reload() is called, passing in the index of the top visible element (to attempt to keep the scroll in visually the same location).

As-is, the scroll position is kept close to the same visible position as intended, however the padding to keep the scrollbar at proper total height seems to get wiped and isn't updated to the new dataSource.minIndex / dataSource.maxIndex values. Alternatively, I've tried setting dataSource.minIndex and dataSource.maxIndex after calling adapter.reload() instead of before; in this case the scrollbar height is set properly, but the scroll position is lost and it winds up at the first available item instead of the one passed to adapter.reload().

dhilt commented 6 years ago

@zacronos I think, I've got the solution, please try branch code. It also includes the solution for "blank" issue (https://github.com/angular-ui/ui-scroll/issues/189), so setTimeout on start is no more needed.

zacronos commented 6 years ago

I've updated the plunker. It seems the first case is fixed, but setting dataSource.minIndex and dataSource.maxIndex after calling adapter.reload() still has some problems.

Also, if I comment out the setTimeout, I still see the blank issue.

dhilt commented 6 years ago

@zacronos I did some update covering that case (the test), please try latest branch code.

Regarding blank issue, I see that Plunker loads resources for too long. I gave 500ms on initialization and styles applying, but in that case it isn't enough. I would reduce it down to 250ms, which will make the success for Plunker less probable, so one of suggested workarounds should be applied...

zacronos commented 6 years ago

Plunker updated again. The 2nd case now works as well, except if I am scrolled to within the 1st 50 items or the last 50 items when I hit the reload button -- in that case, the scroll position after reload does not match the index value passed to adapter.reload(). (The 1st case does not have this problem.)

dhilt commented 6 years ago

@zacronos I made a fix that covers this use case. Unfortunately, I was not able to write appropriate test due to datasource.get async nature, which is not async in the test environment, so the new items processing occurs before min/maxIndex set. Please try the latest dist again... and thanks for participation!

zacronos commented 6 years ago

That did it! Everything seems correct to me.

dhilt commented 6 years ago

Closing due to v1.7.0-rc.6 has been released.