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

Buffer index when deleting an item #232

Closed priandsf closed 4 years ago

priandsf commented 5 years ago

buffer.remove() resets its first index when it gets empty with the code bellow:

      if(!buffer.length) {
        buffer.first = 1;
        buffer.next = 1;
      }

This actually has a few issues:

I suggest the following fix:

      if(!buffer.length) {
        buffer.first = buffer.minIndex!==undefined ? buffer.minIndex : startIndex;
        buffer.next = buffer.first;
        buffer.eof = buffer.eof && buffer.first==buffer.maxIndex;
      }
dhilt commented 5 years ago

@priandsf I opened PR #233. There a lot of changes, but they are mostly refactoring. Three files relate to our problem:

adapter.js buffer.js BufferCleanupSpec.js

The spec file contains scenarios, you may see how the indexes are evolving during cleaning the Buffer with and without immutableTop option in BOF/EOF and non-BOF/EOF cases.

Looks like it works without changes you suggested above. Moreover, my buffer.js change is also unnecessary. It is here for a consistency purpose that doesn't impact the behaviour.

May I ask you to look at this and say what do you think? does it satisfy your needs? do you have some scenarios which would be broken with this approach? can you introduce these scenarios in BufferCleanupSpec?

dhilt commented 4 years ago

Closing per v1.8.2 containing the fix is released.