Expandable grid display issues when rows are expanded

[yurigenin]

When you expand several top rows in the grid and scroll all the way down the grid is not painted properly. Just try Expandable Grid tutorial and you will see the problem. Seems like a scrolling issue but currently this feature is not much usable. You can see even bigger problem when you expand all rows. Now scrolling to the bottom exposes an empty flickering grid.

Seems like it is related to the virtualizationThreshold feature. If you make it equal to the number of rows in the main grid the issue goes away but with big datasets it is impractical as grid really slows to a crawl in that case.

The issue is in the GridRenderContainer.prototype.adjustRows method (expanded rows heights are not used for calculations) as well as in the uiGridViewport directive's controller() function:

var hiddenRowWidth = (rowContainer.currentTopRow) * rowContainer.grid.options.rowHeight;

Again, expanded row heights are not taken into consideration at all.

[Mangesh-P]

+1 Also expandAll becomes extremely slow when using large no of rows

[FlavorScape]

Indeed, I can reproduce with a tall expandable row template.

Heres a plunkr of the tutorial demo with a tall expandable row: http://plnkr.co/edit/kLMMl74UjRXiuS3h0s8x?p=preview

[PaulL1]

The defect we're on here is with expandable, the code fragment you refer to is in infinite scroll. So I doubt the two are related - the features are quite independent of each other.

[vance]

Ok, so back to this today. Will post more soon. In my initial research, I see one value that jumps to a very invalid value, from a constant 14992 value, it jumps to -215 suddently (negative value) when scrolling past an expanded row. I'm graphing the values, and just by looking at the graph, I can tell when I've scrolled passed an expanded row.

GridRenderContainer.js

GridRenderContainer.prototype.adjustRows = function adjustRows(scrollTop, scrollPercentage, postDataLoaded) {
         ...
        self.getVerticalScrollLength()
        ...

I will graph other values and see what I get... EDIT: I confirmed that the negative value is the same as when you first expand the row. That is, expanding it and scrolling by it both cause a blip in the value.

[vance]

I have graphed some variables and added the ability to tag events. We can clearly see the the parts where the yellow line (canvasHeight) jumps to a weird value when scrolling past the expanded row. This may be removing and adding it to the rowCache... but as you can see, there's two values it jumps to, so i'm thinking maybe it's both hitting and not hitting the self.canvasHeightShouldUpdate

if you want to debug with the same tool, I made it a project: tiny-graph

[vance]

Getting closer. I think there's an outstanding task to actually take height into account when determining the minRowsToRender... When I log if the rows have a height over 30 (row default height) it never logs true. So, the list is wrong, or the values are incorrect and do not include the expanded row's height. This is only one place... this may be lacking in other places, too....

  // TODO(c0bra): calculate size?? Should this be in a stackable directive?

GridRenderContainer.prototype.minRowsToRender = function minRowsToRender() {
    var self = this;
    var minRows = 0;
    var rowAddedHeight = 0;
    var viewPortHeight = self.getViewportHeight();
    var hasTall = false;
    for (var i = self.visibleRowCache.length - 1; rowAddedHeight < viewPortHeight && i >= 0; i--) {
      rowAddedHeight += self.visibleRowCache[i].height;    
      if(self.visibleRowCache[i].height > 30){
        hasTall = true;
      }
      minRows++;
    }
    console.log('has a tall row? ', hasTall );

    return minRows;
  };

[vance]

I'm going nuts here... anyone know where the visibleRowCache is actually set? All I can find are references getting the value, but not setting it. Maybe i'm blind.

[vance]

Of course, I find it right after posting: Grid.prototype.setVisibleRows

[vance]

1) Does anyone think expanding a row should call redrawInPlace on the container? (it doesn't) Or should it just recalculate the canvas height only?

2) Any idea why GridRenderContainer.prototype.getCanvasHeight shows self.visibleRowCache to contain a row with height > 30 (the expanded row), while in the function minRowsToRender the cache does not contain the expanded row?

[vance]

My most recent finding It is removing the expanded row TOO EARLY. The currentTopRow index is incorrect and does not include the expanded row when scrolling down past it. That explains the "jumping" to 5-10 rows below the index where the canvas should start rendering.

I think this logic inside adjustRows should be inclusive of row height (including expanded rows). Currently, it seems to only decide the rendering range based on index/count and not height.

if (rowCache.length > self.grid.options.virtualizationThreshold) {
  if (!(typeof(scrollTop) === 'undefined' || scrollTop === null)) {
    // Have we hit the threshold going down?
    if ( !self.grid.suppressParentScrollDown && self.prevScrollTop < scrollTop && rowIndex < self.prevRowScrollIndex + self.grid.options.scrollThreshold && rowIndex < maxRowIndex) {
      return;
    }
    //Have we hit the threshold going up?
    if ( !self.grid.suppressParentScrollUp && self.prevScrollTop > scrollTop && rowIndex > self.prevRowScrollIndex - self.grid.options.scrollThreshold && rowIndex < maxRowIndex) {
      return;
    }
  }
  var rangeStart = {};
  var rangeEnd = {};

  rangeStart = Math.max(0, rowIndex - self.grid.options.excessRows);
  rangeEnd = Math.min(rowCache.length, rowIndex + minRows + self.grid.options.excessRows);

  newRange = [rangeStart, rangeEnd];
}

[vance]

ah yes! Whilst scrolling past an expandable with height 1500PX, the rowIndex continues to increment, even though we're still looking at the guts of the expandable row. it should be "locked" at the index of the expandable row until it has passed the top boundary of the canvas.

tldr; there's a mismatch. the rendered rows are selected based on % of visible row count based on the scrollPercentage, which takes no account of height.

[vance]

Good news and bad news. Good news is, for basic cases, I have a solution for the expandable row miscalculation. The bad news is, this may be a systemic problem, and does NOT fix it if you also use infinite scroll because it has the same flawed assumption that rows are the same height.

We need to compute the proper index based on scrollTop computed agains the actual row height, and not just the count:

in GridRenderContainer.prototype.adjustRows doing this:

var theoreticalRowIndex = 0;
    var currentHeight = 0;
    var i = 0;
    self.visibleRowCache.forEach(function(r){
      if( scrollTop > currentHeight && scrollTop < currentHeight + r.height){
        theoreticalRowIndex = i;
      }
      currentHeight+= r.height;
      i++;
    });

instead of

 maxRowIndex * scrollPercentage

gets rid of the "jumping" at the end of the expandable row.

I'm not sure how to adjust the end range, so just setting rangeEnd = rowCache.length; seems to work.

[vance]

this is fixed with the PR by the way =)

[cutterbl]

I notice this PR was entered back in 2016. I implemented this locally (dc183a1), and it seems to resolve the scrolling issue with expandable rows. What's required to get the changes into code? I see a 'help wanted' tag here...

[mportuga]

@cutterbl That PR id not valid, it is the equivalent of changing your grid options to have no virtual rows. See the final comment in the PR: https://github.com/angular-ui/ui-grid/pull/5395

[Agarwal-Pulkit]

I am also facing same issues while integrating expandable in our grid. Do we have any work around or solution that can eliminate scroll issue.