Skip to content

Commit 644428f

Browse files
committed
address comments
1 parent 1932c53 commit 644428f

File tree

5 files changed

+28
-18
lines changed

5 files changed

+28
-18
lines changed

src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,29 +122,29 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
122122
this._viewport = null;
123123
}
124124

125-
/** Implemented as part of VirtualScrollStrategy. */
125+
/** @docs-private Implemented as part of VirtualScrollStrategy. */
126126
onContentScrolled() {
127127
if (this._viewport) {
128128
this._updateRenderedContentAfterScroll();
129129
}
130130
}
131131

132-
/** Implemented as part of VirtualScrollStrategy. */
132+
/** @docs-private Implemented as part of VirtualScrollStrategy. */
133133
onDataLengthChanged() {
134134
if (this._viewport) {
135135
// TODO(mmalebra): Do something smarter here.
136136
this._setScrollOffset();
137137
}
138138
}
139139

140-
/** Implemented as part of VirtualScrollStrategy. */
140+
/** @docs-private Implemented as part of VirtualScrollStrategy. */
141141
onContentRendered() {
142142
if (this._viewport) {
143143
this._checkRenderedContentSize();
144144
}
145145
}
146146

147-
/** Implemented as part of VirtualScrollStrategy. */
147+
/** @docs-private Implemented as part of VirtualScrollStrategy. */
148148
onRenderedOffsetChanged() {
149149
if (this._viewport) {
150150
this._checkRenderedContentOffset();
@@ -318,6 +318,7 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
318318
this._updateTotalContentSize(this._lastRenderedContentSize);
319319
}
320320

321+
/** Checks the currently rendered content offset and saves the value for later use. */
321322
private _checkRenderedContentOffset() {
322323
const viewport = this._viewport!;
323324
this._lastRenderedContentOffset = viewport.getOffsetToRenderedContentStart()!;

src/cdk-experimental/scrolling/fixed-size-virtual-scroll.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,21 +60,21 @@ export class FixedSizeVirtualScrollStrategy implements VirtualScrollStrategy {
6060
this._updateRenderedRange();
6161
}
6262

63-
/** Implemented as part of VirtualScrollStrategy. */
63+
/** @docs-private Implemented as part of VirtualScrollStrategy. */
6464
onContentScrolled() {
6565
this._updateRenderedRange();
6666
}
6767

68-
/** Implemented as part of VirtualScrollStrategy. */
68+
/** @docs-private Implemented as part of VirtualScrollStrategy. */
6969
onDataLengthChanged() {
7070
this._updateTotalContentSize();
7171
this._updateRenderedRange();
7272
}
7373

74-
/** Implemented as part of VirtualScrollStrategy. */
74+
/** @docs-private Implemented as part of VirtualScrollStrategy. */
7575
onContentRendered() { /* no-op */ }
7676

77-
/** Implemented as part of VirtualScrollStrategy. */
77+
/** @docs-private Implemented as part of VirtualScrollStrategy. */
7878
onRenderedOffsetChanged() { /* no-op */ }
7979

8080
/** Update the viewport's total content size. */

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import {
2424
ViewContainerRef,
2525
} from '@angular/core';
2626
import {Observable, Subject} from 'rxjs';
27-
import {pairwise, shareReplay, startWith, switchMap} from 'rxjs/operators';
27+
import {pairwise, shareReplay, startWith, switchMap, takeUntil} from 'rxjs/operators';
2828
import {CdkVirtualScrollViewport} from './virtual-scroll-viewport';
2929

3030

@@ -138,6 +138,8 @@ export class CdkVirtualForOf<T> implements CollectionViewer, DoCheck, OnDestroy
138138
/** Whether the rendered data should be updated during the next ngDoCheck cycle. */
139139
private _needsUpdate = false;
140140

141+
private _destroyed = new Subject<void>();
142+
141143
constructor(
142144
/** The view container to add items to. */
143145
private _viewContainerRef: ViewContainerRef,
@@ -151,7 +153,7 @@ export class CdkVirtualForOf<T> implements CollectionViewer, DoCheck, OnDestroy
151153
this._data = data;
152154
this._onRenderedDataChange();
153155
});
154-
this._viewport.renderedRangeStream.subscribe(range => {
156+
this._viewport.renderedRangeStream.pipe(takeUntil(this._destroyed)).subscribe(range => {
155157
this._renderedRange = range;
156158
this.viewChange.next(this._renderedRange);
157159
this._onRenderedDataChange();
@@ -214,6 +216,9 @@ export class CdkVirtualForOf<T> implements CollectionViewer, DoCheck, OnDestroy
214216
this._dataSourceChanges.complete();
215217
this.viewChange.complete();
216218

219+
this._destroyed.next();
220+
this._destroyed.complete();
221+
217222
for (let view of this._templateCache) {
218223
view.destroy();
219224
}

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -257,16 +257,15 @@ export class CdkVirtualScrollViewport implements DoCheck, OnInit, OnDestroy {
257257

258258
// If the rendered content offset was specified as an offset to the end of the content,
259259
// rewrite it as an offset to the start of the content.
260-
if (this._renderedContentOffsetNeedsRewrite) {
261-
this._ngZone.onStable.pipe(take(1)).subscribe(() => {
260+
this._ngZone.onStable.pipe(take(1)).subscribe(() => {
261+
if (this._renderedContentOffsetNeedsRewrite) {
262262
this._renderedContentOffset -= this.measureRenderedContentSize();
263263
this._renderedContentOffsetNeedsRewrite = false;
264264
this.setRenderedContentOffset(this._renderedContentOffset);
265-
});
266-
} else {
267-
this._ngZone.onStable.pipe(take(1))
268-
.subscribe(() => this._scrollStrategy.onRenderedOffsetChanged());
269-
}
265+
} else {
266+
this._scrollStrategy.onRenderedOffsetChanged();
267+
}
268+
});
270269
});
271270
}
272271
}

src/demo-app/virtual-scroll/virtual-scroll-demo.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,12 @@ export class VirtualScrollDemo {
9898
sortBy(prop: 'name' | 'capital') {
9999
this.statesObservable.next(this.states.map(s => ({...s})).sort((a, b) => {
100100
const aProp = a[prop], bProp = b[prop];
101-
return aProp < bProp ? -1 : (aProp > bProp ? 1 : 0);
101+
if (aProp < bProp) {
102+
return -1;
103+
} else if (aProp > bProp) {
104+
return 1;
105+
}
106+
return 0;
102107
}));
103108
}
104109
}

0 commit comments

Comments
 (0)