From 8cfaeea4b0b9bf0f10529a8d50d9afd06014b8a1 Mon Sep 17 00:00:00 2001
From: mmalerba <mmalerba@google.com>
Date: Mon, 15 Oct 2018 14:02:49 -0700
Subject: [PATCH] fix(virtual-scroll): fix several small bugs (#13597)

* fix(virtual-scroll): fix several small bugs

fixes include:
* ensure content wrapper is at least as wide as the viewport
* run change detection when data source emits but the data length remains the same
* improve error message when no virtual scroll strategy is provided
* add docs on using a custom virtual scroll strategy

* address comments

* address feedback
---
 src/cdk/scrolling/scrolling.md                | 12 ++++++++-
 .../scrolling/virtual-scroll-viewport.scss    |  2 ++
 .../scrolling/virtual-scroll-viewport.spec.ts | 25 +++++++++++++++++++
 src/cdk/scrolling/virtual-scroll-viewport.ts  |  8 +++++-
 ...virtual-scroll-custom-strategy-example.css |  9 +++++++
 ...irtual-scroll-custom-strategy-example.html |  3 +++
 ...-virtual-scroll-custom-strategy-example.ts | 20 +++++++++++++++
 7 files changed, 77 insertions(+), 2 deletions(-)
 create mode 100644 src/material-examples/cdk-virtual-scroll-custom-strategy/cdk-virtual-scroll-custom-strategy-example.css
 create mode 100644 src/material-examples/cdk-virtual-scroll-custom-strategy/cdk-virtual-scroll-custom-strategy-example.html
 create mode 100644 src/material-examples/cdk-virtual-scroll-custom-strategy/cdk-virtual-scroll-custom-strategy-example.ts

diff --git a/src/cdk/scrolling/scrolling.md b/src/cdk/scrolling/scrolling.md
index 3729a76ec90..1ddf16cc06c 100644
--- a/src/cdk/scrolling/scrolling.md
+++ b/src/cdk/scrolling/scrolling.md
@@ -25,7 +25,7 @@ rest.
 #### Creating items in the viewport
 `*cdkVirtualFor` replaces `*ngFor` inside of a `<cdk-virtual-scroll-viewport>`, supporting the exact
 same API as [`*ngFor`](https://angular.io/api/common/NgForOf). The simplest usage just specifies the
-list of items:
+list of items (note that the `itemSize` property on the viewport must be set):
 
 <!-- example(cdk-virtual-scroll-overview) -->
 
@@ -111,3 +111,13 @@ that the parent does not introduce additional space (e.g. via `margin` or `paddi
 interfere with the scrolling.
 
 <!-- example(cdk-virtual-scroll-dl) -->
+
+### Scrolling strategies
+In order to determine how large the overall content is and what portion of it actually needs to be
+rendered at any given time the viewport relies on a `VirtualScrollStrategy` being provided. The
+simplest way to provide it is to use the `itemSize` directive on the viewport
+(e.g. `<cdk-virtual-scroll-viewport itemSize="50">`). However it is also possible to provide a 
+custom strategy by creating a class that implements the `VirtualScrollStrategy` interface and
+providing it as the `VIRTUAL_SCROLL_STRATEGY` on the component containing your viewport.
+
+<!-- example(cdk-virtual-scroll-custom-strategy) -->
diff --git a/src/cdk/scrolling/virtual-scroll-viewport.scss b/src/cdk/scrolling/virtual-scroll-viewport.scss
index fb90cb7193e..8f127ce88a3 100644
--- a/src/cdk/scrolling/virtual-scroll-viewport.scss
+++ b/src/cdk/scrolling/virtual-scroll-viewport.scss
@@ -56,10 +56,12 @@ cdk-virtual-scroll-viewport {
 }
 
 .cdk-virtual-scroll-orientation-horizontal .cdk-virtual-scroll-content-wrapper {
+  min-height: 100%;
   @include _cdk-virtual-scroll-clear-container-space(horizontal);
 }
 
 .cdk-virtual-scroll-orientation-vertical .cdk-virtual-scroll-content-wrapper {
+  min-width: 100%;
   @include _cdk-virtual-scroll-clear-container-space(vertical);
 }
 
diff --git a/src/cdk/scrolling/virtual-scroll-viewport.spec.ts b/src/cdk/scrolling/virtual-scroll-viewport.spec.ts
index 4bb5796ab98..6ca88466344 100644
--- a/src/cdk/scrolling/virtual-scroll-viewport.spec.ts
+++ b/src/cdk/scrolling/virtual-scroll-viewport.spec.ts
@@ -717,6 +717,20 @@ describe('CdkVirtualScrollViewport', () => {
       expect(viewport.elementRef.nativeElement.scrollWidth).toBe(10000);
     }));
   });
+
+  describe('with no VirtualScrollStrategy', () => {
+    beforeEach(() => {
+      TestBed.configureTestingModule({
+        imports: [ScrollingModule],
+        declarations: [VirtualScrollWithNoStrategy],
+      }).compileComponents();
+    });
+
+    it('should fail on construction', fakeAsync(() => {
+      expect(() => TestBed.createComponent(VirtualScrollWithNoStrategy)).toThrowError(
+          'Error: cdk-virtual-scroll-viewport requires the "itemSize" property to be set.');
+    }));
+  });
 });
 
 
@@ -848,3 +862,14 @@ class FixedSizeVirtualScrollWithRtlDirection {
     return this.orientation == 'horizontal' ? this.viewportCrossSize : this.viewportSize;
   }
 }
+
+@Component({
+  template: `
+    <cdk-virtual-scroll-viewport>
+      <div *cdkVirtualFor="let item of items">{{item}}</div>
+    </cdk-virtual-scroll-viewport>
+  `
+})
+class VirtualScrollWithNoStrategy {
+  items = [];
+}
diff --git a/src/cdk/scrolling/virtual-scroll-viewport.ts b/src/cdk/scrolling/virtual-scroll-viewport.ts
index 4207ea2756b..d2c04c9c3b1 100644
--- a/src/cdk/scrolling/virtual-scroll-viewport.ts
+++ b/src/cdk/scrolling/virtual-scroll-viewport.ts
@@ -124,10 +124,15 @@ export class CdkVirtualScrollViewport extends CdkScrollable implements OnInit, O
   constructor(public elementRef: ElementRef<HTMLElement>,
               private _changeDetectorRef: ChangeDetectorRef,
               ngZone: NgZone,
-              @Inject(VIRTUAL_SCROLL_STRATEGY) private _scrollStrategy: VirtualScrollStrategy,
+              @Optional() @Inject(VIRTUAL_SCROLL_STRATEGY)
+                  private _scrollStrategy: VirtualScrollStrategy,
               @Optional() dir: Directionality,
               scrollDispatcher: ScrollDispatcher) {
     super(elementRef, scrollDispatcher, ngZone, dir);
+
+    if (!_scrollStrategy) {
+      throw Error('Error: cdk-virtual-scroll-viewport requires the "itemSize" property to be set.');
+    }
   }
 
   ngOnInit() {
@@ -183,6 +188,7 @@ export class CdkVirtualScrollViewport extends CdkScrollable implements OnInit, O
           this._dataLength = newLength;
           this._scrollStrategy.onDataLengthChanged();
         }
+        this._doChangeDetection();
       });
     });
   }
diff --git a/src/material-examples/cdk-virtual-scroll-custom-strategy/cdk-virtual-scroll-custom-strategy-example.css b/src/material-examples/cdk-virtual-scroll-custom-strategy/cdk-virtual-scroll-custom-strategy-example.css
new file mode 100644
index 00000000000..55eb0cd3c6e
--- /dev/null
+++ b/src/material-examples/cdk-virtual-scroll-custom-strategy/cdk-virtual-scroll-custom-strategy-example.css
@@ -0,0 +1,9 @@
+.example-viewport {
+  height: 200px;
+  width: 200px;
+  border: 1px solid black;
+}
+
+.example-item {
+  height: 50px;
+}
diff --git a/src/material-examples/cdk-virtual-scroll-custom-strategy/cdk-virtual-scroll-custom-strategy-example.html b/src/material-examples/cdk-virtual-scroll-custom-strategy/cdk-virtual-scroll-custom-strategy-example.html
new file mode 100644
index 00000000000..0f913aff587
--- /dev/null
+++ b/src/material-examples/cdk-virtual-scroll-custom-strategy/cdk-virtual-scroll-custom-strategy-example.html
@@ -0,0 +1,3 @@
+<cdk-virtual-scroll-viewport class="example-viewport">
+  <div *cdkVirtualFor="let item of items" class="example-item">{{item}}</div>
+</cdk-virtual-scroll-viewport>
diff --git a/src/material-examples/cdk-virtual-scroll-custom-strategy/cdk-virtual-scroll-custom-strategy-example.ts b/src/material-examples/cdk-virtual-scroll-custom-strategy/cdk-virtual-scroll-custom-strategy-example.ts
new file mode 100644
index 00000000000..0f7c4b1ae15
--- /dev/null
+++ b/src/material-examples/cdk-virtual-scroll-custom-strategy/cdk-virtual-scroll-custom-strategy-example.ts
@@ -0,0 +1,20 @@
+import {FixedSizeVirtualScrollStrategy, VIRTUAL_SCROLL_STRATEGY} from '@angular/cdk/scrolling';
+import {ChangeDetectionStrategy, Component} from '@angular/core';
+
+export class CustomVirtualScrollStrategy extends FixedSizeVirtualScrollStrategy {
+  constructor() {
+    super(50, 250, 500);
+  }
+}
+
+/** @title Virtual scroll with a custom strategy */
+@Component({
+  selector: 'cdk-virtual-scroll-custom-strategy-example',
+  styleUrls: ['cdk-virtual-scroll-custom-strategy-example.css'],
+  templateUrl: 'cdk-virtual-scroll-custom-strategy-example.html',
+  changeDetection: ChangeDetectionStrategy.OnPush,
+  providers: [{provide: VIRTUAL_SCROLL_STRATEGY, useClass: CustomVirtualScrollStrategy}]
+})
+export class CdkVirtualScrollCustomStrategyExample {
+  items = Array.from({length: 100000}).map((_, i) => `Item #${i}`);
+}