Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

All new items added via adapter operations end up in DOM #104

Open
arudnev opened this issue Jun 15, 2016 · 4 comments
Open

All new items added via adapter operations end up in DOM #104

arudnev opened this issue Jun 15, 2016 · 4 comments

Comments

@arudnev
Copy link

arudnev commented Jun 15, 2016

I believe in current implementation if you add items via append / prepend / applyUpdates operations on adapter all those new rows would end up being added to buffer / inserted to DOM / get all the watchers created for, etc, which basically leads to sluggish / unresponsive UI when added set of data is relatively big.

It's easy to change some of the demos to add bunch of items (i.e. 1000) instead of one to reproduce (i.e. http://rawgit.com/armortext/ui-scroll/master/demo/adapter/adapter.html), even though it might require higher number of watchers per row to see the effect visually, but you can inspect it in dev tools.

I'm wondering if those adapter operations can be changed to process new data in micro-batches where buffer is adjusted after smaller number of buffer modifications and addition of new elements to the buffer is avoided if they are clearly going to be out of bounds for items that should stay in the buffer?

It can be partially addressed / avoided for items that would be prepended / appended if the datasource is bounded and we can simply update datasource minIndex / maxIndex instead of calling prepend / append. Unfortunately it seems there are some issues with maintaining scroll position and top / bottom paddings when items are of variable height that becomes known only after item template is bound to the data. Also, it does not solve it for case when bunch of items are being inserted via applyUpdates.

The best workaround in current implementation that I see so far is to capture topVisible index and simply reload data if there is a chance that it's relatively big number of newly added items, but it's quite suboptimal from visual perspective, and also complicates code quite a bit if you try to be minimize number of reloads like those.

Any thoughts?

@mfeingold
Copy link
Contributor

Well, the applyUpdates is not intended for adding big number of items in one call. No matter what tricks can be played to optimize the process, in the end of the day out of 1000 items only a handful will survive the rest will be removed from DOM. I do not see any reasonable way to prevent the waste of effort of creating and then destroying them. Therefore I think this is the responsibility of the user of the component to only use applyUpdates to insert items which may be visible.

I understand that in some use cases it will make for a trickier logic in datasource, but it looks like that the user will have to deal with this logic anyway.

Now the issue of items with variable height this is a different story. We have long standing problems in this area. My latest hope is to refactor all related calculations to relay on element positions rather than element height/outerHeight. We started the refactoring, but it will take some more time

@arudnev
Copy link
Author

arudnev commented Jun 16, 2016

I was trying to figure out a way to spoon-feed those changes through adapter api in the application code rather than sending them in one call to applyUpdates or prepend / append, but there are a few issues with that.

Biggest issue is that once I add items to the buffer, I can not force them to be "clipped" from it programmatically.
I might be able to get access to viewport somehow and call clipTop / clipBottom on in from application code, but it's seems to be somewhat internal / not something that is documented, so I probably should stay away from it.
Let me give you an example to clarify the issue - let's say I'm at eof, new items are appended to the datasource and I call append on the adapter.
When I call append on the adapter clipBottom is not going to be invoked, so all the appended items are going to be added to the buffer, maxIndex is going to be incremented, and the buffer always will be in eof state.
So, even if I append items in small chunks (i.e. one by one, alternating it with $digest / $timeout somehow) I'll end up with all of them in the buffer / DOM.
If I only add a few of them then only those few are going to be visible once user scrolls down as no more data is going to be requested from the datasource since the buffer is at eof and has all the items up to maxIndex already.
Of course I could monitor scroll events, try to derive bottomVisible item / element somehow and determine that I need to dynamically append more items that have been added to the datasource, but have not been appended via adapter api as the user scrolls closer to the last item that was appended via adapter api.
It's far from ideal in terms of code complexity since it's largely duplicates internals of ui-scroll at that point, but bigger issue is that scroll bar keeps jumping as new items are being appended.
It all probably could be substantially simplified if clipTop / clipBottom is invoked internally after append / prepend / applyUpdates operations, then at least it becomes possible to break all the changes into multiple small batches of updates / intermediate datasource states.

Another issue with skipping propagation of some of the updates (i.e. we only prepend / append small number of items instead of all of them or only insert 20 items via adapter api out of 1000 that have been added into the datasource until user scrolls closer to the position where those items have been added) is that it's not clear how to determine when those items that have been added to the datasource but have not been propagated to the buffer are about to become visible. Basically, if minIndex / maxIndex and first / next attributes of the buffer and something like topVisibleIndex / bottomVisibleIndex are exposed then coding partial / incremental update propagation becomes much simpler task than it is right now.

I thought that one possible approach in which there is almost no extra logic in the application code is when ui-scroll checks internally if new elements need to be added to the DOM before actually doing that.
For example, when processing append operation only up to bufferSize number of items is added to the buffer / DOM, maxIndex / eof / next are updated accordingly, then it goes into normal mode of loading more items as needed.
Similarly, when processing applyUpdates, once it's known that new items are most likely going to be clipped anyway (before inserting an element check that its bounds are not going to cause it be be clipped), stop adding them to DOM / remove them from buffer at the end of applyUpdates, making sure that minIndex / maxIndex, bof / eof, first / next are updated correctly.
In all cases clipping extra elements needs to happen after those operations, but as soon as it's clear (simple logic within component, much more complex if done in the application code) that certain elements are going to be clipped they should not even be inserted at all.

Relying on element position sounds very interesting. If I understand correctly what you mean by that it could potentially solve issue of smooth scrolling through large dataset with known bounds when items have unknown variable height as well as issue of user dragging the scroller on a large dataset instead of using wheel (i.e. scrolling from item 1 to item 500 on a dataset of 1000 items - right now it would result into 50 calls to get operation on the datasource, insert all those intermediate elements into DOM with all the related visual delays).
I'm wondering if it makes sense to wait for it or try to figure out how to fix some of the flaws in current implementation.
From my perspective (for our use cases) the biggest source of issues is that template-based elements for new items are inserted into DOM, their default heights are used to adjust padding and then correction of padding adjustment happens after bindings are processed, which causes visual jumps of scroller position and size, brief shift in scroll view when binding are processed and until paddings are re-adjusted. Plus in some cases it results into incorrect combination of topPadding.height and viewport.scrollTop in the end.
All of that seems to be resolvable, but I'm wondering if I should report more defects and suggest some fixes for current codebase or wait some time for new refactored version, is it something that can potentially be prototyped and stabilized quickly enough?

@dhilt
Copy link
Member

dhilt commented Jun 16, 2016

I can't imagine when it would be necessary to add 1000 items via adapter. Is it realy impossible to make it via datasource.get? Adapter append/prepend is very specific mechanism. Using it means a big responsibility for a programmer and I beleive that we can't cover all use cases of adapter append/prepend usage inside the scroller core. And this can't be a goal cause each non-standart use case complecates the source code... Btw, did you see append/prepend sync demo? Look, during item append I don't add it into a DOM if I'm not at the bottom of current dataset, but this "appended" item will appear at the right time by the datasource.get call...

@arudnev
Copy link
Author

arudnev commented Jun 17, 2016

We are working on an instant messenger (chat), so our major use case is to open a scroll view positioned at the bottom of the list and keep appending new items as they come into the thread. If the app is in focus and you are the bottom you typically keep auto-scrolling, in which case items are appended to the DOM, but then are clipped at the top as it auto-scrolls, which is all fine. If user scrolls up just a bit (but the buffer is still in eof state) or app is simply not in focus, you typically keep appending items, but avoid auto-scrolling, so after some period of time you end up with hundreds (scroll up and leave it for a few hours and on active group chats you easily end up with 1000+) items appended via adapter and in the DOM.
There are similar use cases when items keep being prepended when you start at the top of the list.
There are use cases when view is sorted, so new items keep being inserted, but since there is no clip of top / bottom after insert you end up with all those extra items in DOM, even though they are being inserted in small numbers.
As I mentioned before, some of that can actually be addressed better by setting minIndex / maxIndex on the datasource, but there we run into various scrolling / jumping issues in the current version (especially if it items get prepended).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants