fix(scrolling): virtual scroll not accounting for margin when measuring range (#19852)

Currently the `CdkVirtualForOf` determines the size of a rendered range by adding up the heights of all the elements, however this doesn't account for margins between them. These changes switch to doing it by taking the difference between the bottom of the last element and the top of the first. This should be a minor performance improvement as well, because we don't have to measure as many elements anymore.

Fixes #19851.

Author: crisbeto

src/cdk/scrolling/virtual-for-of.ts

@@ -57,16 +57,20 @@ export type CdkVirtualForOfContext<T> = {
 };
 
 
-/** Helper to extract size from a DOM Node. */
-function getSize(orientation: 'horizontal' | 'vertical', node: Node): number {
+/** Helper to extract the offset of a DOM Node in a certain direction. */
+function getOffset(orientation: 'horizontal' | 'vertical', direction: 'start' | 'end', node: Node) {
   const el = node as Element;
   if (!el.getBoundingClientRect) {
     return 0;
   }
   const rect = el.getBoundingClientRect();
-  return orientation == 'horizontal' ? rect.width : rect.height;
-}
 
+  if (orientation === 'horizontal') {
+    return direction === 'start' ? rect.left : rect.right;
+  }
+
+  return direction === 'start' ? rect.top : rect.bottom;
+}
 
 /**
  * A directive similar to `ngForOf` to be used for rendering data inside a virtual scrolling
@@ -209,19 +213,33 @@ export class CdkVirtualForOf<T> implements
     // The length of the range we're measuring.
     const rangeLen = range.end - range.start;
 
-    // Loop over all root nodes for all items in the range and sum up their size.
-    let totalSize = 0;
-    let i = rangeLen;
-    while (i--) {
+    // Loop over all the views, find the first and land node and compute the size by subtracting
+    // the top of the first node from the bottom of the last one.
+    let firstNode: HTMLElement | undefined;
+    let lastNode: HTMLElement | undefined;
+
+    // Find the first node by starting from the beginning and going forwards.
+    for (let i = 0; i < rangeLen; i++) {
+      const view = this._viewContainerRef.get(i + renderedStartIndex) as
+          EmbeddedViewRef<CdkVirtualForOfContext<T>> | null;
+      if (view && view.rootNodes.length) {
+        firstNode = lastNode = view.rootNodes[0];
+        break;
+      }
+    }
+
+    // Find the last node by starting from the end and going backwards.
+    for (let i = rangeLen - 1; i > -1; i--) {
       const view = this._viewContainerRef.get(i + renderedStartIndex) as
           EmbeddedViewRef<CdkVirtualForOfContext<T>> | null;
-      let j = view ? view.rootNodes.length : 0;
-      while (j--) {
-        totalSize += getSize(orientation, view!.rootNodes[j]);
+      if (view && view.rootNodes.length) {
+        lastNode = view.rootNodes[view.rootNodes.length - 1];
+        break;
       }
     }
 
-    return totalSize;
+    return firstNode && lastNode ?
+        getOffset(orientation, 'end', lastNode) - getOffset(orientation, 'start', firstNode) : 0;
   }
 
   ngDoCheck() {

src/cdk/scrolling/virtual-scroll-viewport.spec.ts

@@ -134,6 +134,14 @@ describe('CdkVirtualScrollViewport', () => {
           .toBe(testComponent.itemSize * 2, 'combined size of 2 50px items should be 100px');
     }));
 
+    it('should measure range size when items has a margin', fakeAsync(() => {
+      fixture.componentInstance.hasMargin = true;
+      finishInit(fixture);
+
+      expect(viewport.measureRangeSize({start: 1, end: 3})).toBe(testComponent.itemSize * 2 + 10,
+            'combined size of 2 50px items with a 10px margin should be 110px');
+    }));
+
     it('should set total content size', fakeAsync(() => {
       finishInit(fixture);
 
@@ -916,7 +924,8 @@ function triggerScroll(viewport: CdkVirtualScrollViewport, offset?: number) {
     <cdk-virtual-scroll-viewport
         [itemSize]="itemSize" [minBufferPx]="minBufferPx" [maxBufferPx]="maxBufferPx"
         [orientation]="orientation" [style.height.px]="viewportHeight"
-        [style.width.px]="viewportWidth" (scrolledIndexChange)="scrolledToIndex = $event">
+        [style.width.px]="viewportWidth" (scrolledIndexChange)="scrolledToIndex = $event"
+        [class.has-margin]="hasMargin">
       <div class="item"
            *cdkVirtualFor="let item of items; let i = index; trackBy: trackBy; \
                            templateCacheSize: templateCacheSize"
@@ -943,6 +952,10 @@ function triggerScroll(viewport: CdkVirtualScrollViewport, offset?: number) {
       box-sizing: border-box;
       border: 1px dashed #ccc;
     }
+
+    .has-margin .item {
+      margin-bottom: 10px;
+    }
   `],
   encapsulation: ViewEncapsulation.None,
 })
@@ -962,6 +975,7 @@ class FixedSizeVirtualScroll {
   @Input() templateCacheSize = 20;
 
   scrolledToIndex = 0;
+  hasMargin = false;
 
   get viewportWidth() {
     return this.orientation == 'horizontal' ? this.viewportSize : this.viewportCrossSize;