adapter reload() re-render the DOM even items are the same.

[limingli0707]

I'm not sure if it's an issue, maybe more like an enhancement.

We notice a page flicker once the adapter.reload() gets called. You can reproduce it in the example(https://rawgit.com/angular-ui/ui-scroll/master/demo/reload100/reload100.html). Ideally, it would be great if we can avoid this in some cases. For example, in that reload100.html, we enter 1 as the index, the reloaded data and the previous buffer data are the same, maybe we don't have to re-render them at all.

It seems that, based on the implementation of reload(), reload() calls buffer.remove() which will destroy all the elements by calling removeElement(). Which seems like the reload function cleans up the DOM first and then paint new DOM. I feel like this is why reload() always re-rerender the DOM by design.

Can we enhance the logic for reload() to avoid the re-rendering? One interesting thing I found is that applyUpdates() doesn't have the flickering issue since doesn't destroy the dom I guess.

[dhilt]

The reload method is designed to provide complete re-initialization of the ui-scroll contents, the doc is clear in this regard. Making improvements you asked would be equivalent to implementing a new method, say reloadSafe, from scratch. That's not easy.

Entering "1" without scrolling in reload100.html demo seems an artificial use-case. You might protect reload on the app layer by checking topVisible and reload argument:

$scope.doReload = index => {
  if ($scope.myAdapter.topVisibleScope.$index !== index) {
    $scope.myAdapter.reload(index);
  }
}

This will prevent reload if the topmost item index is the reload index.

Then we may discuss more natural use-cases, but generally I would say that applyUpdates(updater) method is our friend here. The updater body could be very smart and complex, you may silently remove all items from current scroller buffer, insert any items instead (replace instead of remove + insert also works). May I not create a sample? Anyway, a specific case should be considered individually and precisely.

[joeattardi]

A little more about our use case:

We are showing a list of items that can be reordered via drag and drop.

We have tried using applyUpdates to update the visible items in the viewport, but the problem now is that there is a quite noticeable delay (we have seen delays up to 1 second) after drag and drop. applyUpdates runs a digest cycle which is what seems to be taking the most time in this delay.

[joeattardi]

Here's a flame chart showing the performance. processUpdates triggers multiple long-running $digest calls:

[dhilt]

@joeattardi The $digest calls amount seems not normal for your case and it needs to be investigated.

For example, for applyUpdates demo, clicking on remove even items button leads to following $digest calls:

• 1 from processUpdates() call coming from initial Adapter.applyUpdates() call;
• 1 from $animate.leave() call coming from removing at the DOM layer and updating the padding elements;
• 1 from another processUpdates() call initiated by new 5 items fetching in forward direction;
• 1 from another processUpdates() call initiated by next new 5 items fetching in forward direction;
• 1 from another processUpdates() call initiated by forward fetch stop (this one could be optimized I think);
• 1 from another processUpdates() call initiated by new 5 items fetching in backward direction;
• 1 from another processUpdates() call initiated by backward fetch stop (also might be optimized I think).

7 $digest calls which satisfy the design, which I think might be reduced to 5 calls.

If your drag-and-drop case means just two items exchange, than I would say it should lead to 2 $digest calls, not 6 as we see on your screenshot. Because no fetch should be involved. If you want me to participate in the investigation, a runnable repro would be appreciated. If it's not possible to create a demo, we may try to look at your datasource implementation and applyUpdates method usage.

[joeattardi]

Unfortunately I'm not able to provide a runnable repro as this is a pretty complex company-proprietary application.

I can share how we're using applyUpdates - we might be doing it wrong.

For our use case, we actually made a minor change to the library that exposes the buffer property of the adapter. This is so that we can access the buffer's first and length properties.

A little more detail:

• We are showing a grid of items.
• The data passed to the buffer is a two-dimensional array - each row of the grid is a single "item" in the buffer, and each element of that array is a value in each column.
• We store our data from the server as a flat array of items, then we call a function that takes the visible portion of the item list and builds the two-dimensional array.

Here is a code snippet. productList.makeRequest is the code that builds the two-dimensional array. We use the buffer.first and buffer.length properties to recalculate the displayed data for the viewport.

Then we call applyUpdates to update the data with the newly recalculated data.

var buffer = $scope.feedAdapter.adapter.buffer;
var updatedBuffer = productList.makeRequest(buffer.first, buffer.length);
$scope.feedAdapter.adapter.applyUpdates(function updater(item, scope) {
  return [updatedBuffer[scope.$index]];
});

Thank you for your continued support!

[dhilt]

@joeattardi Before we proceed, may I ask you to try this branch where I introduced new Adapter read-only props bufferFirst, bufferLast and bufferLength? You need to build that branch locally via npm run build and place the result ./dist sources into your project (./node_modules/angular-ui-scroll/dist). This update should allow to avoid passing the Buffer through the Adapter in your case. If this update is applicable, I will release it as v1.7.5 after some infrastructure work (tests for example). If not, please tell me what is missed.

[joeattardi]

It felt like a bit of a hack to be accessing those buffer properties, so we took a slightly different approach. We now calculate the contents of the row on the fly in the updater function, so our code now looks like this:

productList.recalculate();
$scope.feedAdapter.adapter.applyUpdates(function updater(item, scope) {
  return [productList.getRow(scope.$index)];
});

This is working the same as our previous approach, with the benefit of not having to access buffer properties.

We still have the long digest calls though.

Another issue I just noticed is that if I scroll down a bit so that the top row is not visible, and I do a drag and drop, which calls applyUpdates, the viewport scrolls up to the top. Do you know why it is doing that and how we might stop that from happening? We want the viewport to stay scrolled to the same point so that the result of the drag and drop operation is visible on screen.

[joeattardi]

It turns out that the second issue I mentioned - scrolling up to the top - was something we were doing, and I have fixed it now. So our only remaining issue is the slowness/delay during applyUpdates.

[dhilt]

The second problem might arise from the ui-scroll inner processes concurrency, and I would recommend to make your complicated changes via Adapter only if there are no pending processes (isLoading == false).

Regarding $digest calls issue. I created a demo (https://stackblitz.com/edit/angular-ui-scroll-v1-7-4?file=app.js) with completely another approach for 2 items exchange task. Two approaches I would say. They are based on updating item object data by reference. The ui-scroll does not clone items data, so we can change item contents outside the ui-scroll, and the ui-scroll immediately will have these changes with no additional efforts. There are two methods to choose.

1. No Adapter involved.

    $scope.exchange = function () {
      var contentA = $scope.getItemById($scope.indexA).content;
      var contentB = $scope.getItemById($scope.indexB).content;
      $scope.updateItemById($scope.indexA, contentB);
      $scope.updateItemById($scope.indexB, contentA);
    };

Here and below we assume that our dataset is not generated runtime and could be accessed outside the datasource.get. We can get dataset's item by index (getItemById), and we can update item's object keeping it's reference unchanged (updateItemById). Just rewrite content fields. This method does not take into account current ui-scroll buffer state. It also requires 2 additional dataset API methods.

2. Adapter involved.

    $scope.exchangeAdapter = function () {
      var contentA, contentB;
      var itemA, itemB;
      return $scope.adapter.applyUpdates(function (item, scope) {
        if (scope.$index === $scope.indexA) {
          itemA = item;
          contentA = item.content;
        }
        if (scope.$index === $scope.indexB) {
          itemB = item;
          contentB = item.content;
        }
        if (itemA && itemB) {
          itemA.content = contentB;
          itemB.content = contentA;
          itemA = itemB = null; // exchange should happen only once
          // if the exchange should lead to some external effects,
          // then this is the place where these effects might run
          // for example API call for remote dataset update
        }
      });
    };

The applyUpdates method iterates through all the buffer items, so we can get all necessary information in its callback without special dataset API methods. We do save this information (items that we want to exchange and their' contents) in local vars out of applyUpdates. When all the data needed for exchange is received, we do exchange (once!) and provide side-effects if needed. We return undefined for not to apply updates in terms of the Adapter workflow. As in var.1, we are changing only content fields of items objects while the objects itself remain the same (the references I mean). This method will not exchange items if they are not in current ui-scroll buffer.

As you might see, one of the main thing in these reference-based approaches is that we need to sync or dataset with what we want to be in the ui-scroll viewport after exchange. This synchronization may include remote API calls if the datasource deals with remote data. If all the data is on the client, and datasource.get consumes it within a single namespace, then changing that data by refs accurately should be enough.

Please, try to play with items objects without cloning them. This might conflict with immutable way and unidirectional data flow if it could be important for your project, but this might increase the performance drastically.

[joeattardi]

After experimenting for a few days, we have had really good results so far with updating the items in-place in applyUpdates. That is, we calculate the new row information, then call applyUpdates. In the updater function, instead of returning the new row, we copy the new row information over into the existing object.

This has improved the performance quite a bit!

[dhilt]

@joeattardi I'm glad to know it helps. Fortunately, we still have pure javascript, and in case we can't interfere in the internal framework's magic (digest cycling I mean), it is possible to use common patterns.

I guess we can close this issue, the output is that a) we can use applyUpdates to customize ui-scroll behaviour run-time; b) use mutable patterns (such as objects updates in-place) in the struggle for a better performance.