Stop setting container scrollTop to zero on render if collection has offset

[gwak]

Add scroll test where a collection that is in the middle of the scroll container, offset by some amount, and when rendering it, it should not reset the existing container scroll position. See comment

[pzuraq]

Ok, I will be out for the next two weeks but will take a look at it as soon as I’m back. Thank you!

[gwak]

Hi, do you need anything else for the PR to be merged ?

[pzuraq]

I don't think this is quite the right fix unfortunately, and I haven't had the time to look at the issue and fix it myself. If you can rebase and dive in a bit deeper to try to get the right fix I can try to get it merged asap, I'm also available for guidance if needed.

scrollDiff is supposed to represent the delta between where VC thinks it is and what reality is. This is particularly important in dynamic collections - for instance, let's say you're scrolling upward and VC inserts an element that it recorded previously was 50px high. We insert the element and remeasure it, and it turns out that the content was actually 100px high. If we do nothing, it will appear to the user that the content jumped 50px downward, which will look really bad. In that case, the scrollDiff should be 50, and we should move the scroll position to fix it.

This PR would disable all of that logic if the VC had an offset in its container. It's surprising to me that no tests fail, definitely points to a lack of coverage in this area. The correct fix would account for this._collectionOffset when calculating the scrollDiff.

Feel free to reach out in the #e-smoke-and-mirrors on the Ember Community slack, or on here!

[runspired]

I originally setup vertical-collection to allow for arbitrary padding on either side of the scrollable content in the scroll container to support loading states, table headers/footers and the like. It also helps for when the document body is the scroll container. Seems bad if this is broken :D

I read through the comments on the other thread and peeked at this. I agree this solution here isn't quite right, but if it gets rebased I'm sure we can massage it to a good place.