Skip to content

Commit d3aa780

Browse files
committed
address some more comments
1 parent 6277388 commit d3aa780

File tree

5 files changed

+132
-70
lines changed

5 files changed

+132
-70
lines changed

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

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ export class CdkVirtualForOf<T> implements CollectionViewer, DoCheck, OnDestroy
148148
@SkipSelf() private _viewport: CdkVirtualScrollViewport) {
149149
this.dataStream.subscribe(data => this._data = data);
150150
this._viewport.renderedRangeStream.subscribe(range => this._onRenderedRangeChange(range));
151-
this._viewport.connect(this);
151+
this._viewport.attach(this);
152152
}
153153

154154
/**
@@ -197,6 +197,9 @@ export class CdkVirtualForOf<T> implements CollectionViewer, DoCheck, OnDestroy
197197

198198
ngDoCheck() {
199199
if (this._differ && this._needsUpdate) {
200+
// TODO(mmalerba): We should differentiate needs update due to scrolling and a new portion of
201+
// this list being rendered (can use simpler algorithm) vs needs update due to data actually
202+
// changing (need to do this diff).
200203
const changes = this._differ.diff(this._renderedItems);
201204
if (!changes) {
202205
this._updateContext();
@@ -208,7 +211,7 @@ export class CdkVirtualForOf<T> implements CollectionViewer, DoCheck, OnDestroy
208211
}
209212

210213
ngOnDestroy() {
211-
this._viewport.disconnect();
214+
this._viewport.detach();
212215

213216
this._dataSourceChanges.complete();
214217
this.viewChange.complete();
@@ -240,20 +243,27 @@ export class CdkVirtualForOf<T> implements CollectionViewer, DoCheck, OnDestroy
240243

241244
/** Update the `CdkVirtualForOfContext` for all views. */
242245
private _updateContext() {
243-
for (let i = 0, len = this._viewContainerRef.length; i < len; i++) {
246+
const count = this._data.length;
247+
let i = this._viewContainerRef.length;
248+
while(i--) {
244249
let view = this._viewContainerRef.get(i) as EmbeddedViewRef<CdkVirtualForOfContext<T>>;
245250
view.context.index = this._renderedRange.start + i;
246-
view.context.count = this._data.length;
251+
view.context.count = count;
247252
this._updateComputedContextProperties(view.context);
248253
view.detectChanges();
249254
}
250255
}
251256

252257
/** Apply changes to the DOM. */
253258
private _applyChanges(changes: IterableChanges<T>) {
259+
// TODO(mmalerba): Currently we remove every view and then re-insert it in the correct place.
260+
// It would be better to generate the minimal set of remove & inserts to get to the new list
261+
// instead.
262+
254263
// Detach all of the views and add them into an array to preserve their original order.
255264
const previousViews: (EmbeddedViewRef<CdkVirtualForOfContext<T>> | null)[] = [];
256-
for (let i = 0, len = this._viewContainerRef.length; i < len; i++) {
265+
let i = this._viewContainerRef.length;
266+
while (i--) {
257267
previousViews.unshift(
258268
this._viewContainerRef.detach()! as EmbeddedViewRef<CdkVirtualForOfContext<T>>);
259269
}
@@ -278,7 +288,8 @@ export class CdkVirtualForOf<T> implements CollectionViewer, DoCheck, OnDestroy
278288

279289
// We have nulled-out all of the views that were removed or moved from previousViews. What is
280290
// left is the unchanged items that we queue up to be re-inserted.
281-
for (let i = 0, len = previousViews.length; i < len; i++) {
291+
i = previousViews.length;
292+
while(i--) {
282293
if (previousViews[i]) {
283294
insertTuples[i] = {record: null, view: previousViews[i]!};
284295
}
@@ -290,8 +301,12 @@ export class CdkVirtualForOf<T> implements CollectionViewer, DoCheck, OnDestroy
290301

291302
/** Insert the RecordViewTuples into the container element. */
292303
private _insertViews(insertTuples: RecordViewTuple<T>[]) {
293-
for (let i = 0, len = insertTuples.length; i < len; i++) {
294-
let {view, record} = insertTuples[i];
304+
const count = this._data.length;
305+
let i = insertTuples.length;
306+
const lastIndex = i - 1;
307+
while (i--) {
308+
const index = lastIndex - i;
309+
let {view, record} = insertTuples[index];
295310
if (view) {
296311
this._viewContainerRef.insert(view);
297312
} else {
@@ -310,8 +325,8 @@ export class CdkVirtualForOf<T> implements CollectionViewer, DoCheck, OnDestroy
310325
if (record) {
311326
view.context.$implicit = record.item as T;
312327
}
313-
view.context.index = this._renderedRange.start + i;
314-
view.context.count = this._data.length;
328+
view.context.index = this._renderedRange.start + index;
329+
view.context.count = count;
315330
this._updateComputedContextProperties(view.context);
316331
view.detectChanges();
317332
}

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ export class VirtualScrollFixedSizeStrategy implements VirtualScrollStrategy {
6868
return;
6969
}
7070

71-
this._viewport.totalContentSize = this._viewport.dataLength * this._itemSize;
71+
this._viewport.setTotalContentSize(this._viewport.getDataLength() * this._itemSize);
7272
};
7373

7474
/** Update the viewport's rendered range. */
@@ -82,9 +82,9 @@ export class VirtualScrollFixedSizeStrategy implements VirtualScrollStrategy {
8282
const range = this._expandRange(
8383
{start: firstVisibleIndex, end: firstVisibleIndex},
8484
this._bufferSize,
85-
Math.ceil(this._viewport.viewportSize / this._itemSize) + this._bufferSize);
86-
this._viewport.renderedRange = range;
87-
this._viewport.renderedContentOffset = this._itemSize * range.start;
85+
Math.ceil(this._viewport.getViewportSize() / this._itemSize) + this._bufferSize);
86+
this._viewport.setRenderedRange(range);
87+
this._viewport.setRenderedContentOffset(this._itemSize * range.start);
8888
}
8989

9090
/**
@@ -100,7 +100,7 @@ export class VirtualScrollFixedSizeStrategy implements VirtualScrollStrategy {
100100
}
101101

102102
const start = Math.max(0, range.start - expandStart);
103-
const end = Math.min(this._viewport.dataLength, range.end + expandEnd);
103+
const end = Math.min(this._viewport.getDataLength(), range.end + expandEnd);
104104
return {start, end};
105105
}
106106
}
@@ -127,10 +127,10 @@ export function _virtualScrollFixedSizeStrategyFactory(fixedSizeDir: CdkVirtualS
127127
}],
128128
})
129129
export class CdkVirtualScrollFixedSize implements OnChanges {
130-
/** The size of the items in the list. */
130+
/** The size of the items in the list (in pixels). */
131131
@Input() itemSize = 20;
132132

133-
/** The number of extra elements to render on either side of the viewport. */
133+
/** The number of extra elements to render on either side of the scrolling viewport. */
134134
@Input() bufferSize = 5;
135135

136136
/** The scroll strategy used by this directive. */
Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,16 @@
1+
<!--
2+
Wrap the rendered content in an element that will be used to offset it based on the scroll
3+
position.
4+
-->
15
<div #contentWrapper class="cdk-virtual-scroll-content-wrapper"
26
[style.transform]="_renderedContentTransform">
37
<ng-content></ng-content>
48
</div>
9+
<!--
10+
Spacer used to force the scrolling container to the correct size for the *total* number of items
11+
so that the scrollbar captures the size of the entire data set.
12+
-->
513
<div class="cdk-virtual-scroll-spacer"
6-
[style.height.px]="orientation === 'horizontal' ? 1 : totalContentSize"
7-
[style.width.px]="orientation === 'horizontal' ? totalContentSize : 1">
14+
[style.height.px]="orientation === 'horizontal' ? 1 : _totalContentSize"
15+
[style.width.px]="orientation === 'horizontal' ? _totalContentSize : 1">
816
</div>
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,22 @@
1+
// Scrolling container.
12
cdk-virtual-scroll-viewport {
23
display: block;
34
position: relative;
45
overflow: auto;
56
}
67

8+
// Wrapper element for the rendered content. This element will be transformed to push the rendered
9+
// content to its correct offset in the data set as a whole.
710
.cdk-virtual-scroll-content-wrapper {
811
position: absolute;
912
top: 0;
1013
left: 0;
1114
will-change: contents, transform;
1215
}
1316

17+
// Spacer element that whose width or height will be adjusted to match the size of the entire data
18+
// set if it were rendered all at once. This ensures that the scrollable content region is the
19+
// correct size.
1420
.cdk-virtual-scroll-spacer {
1521
will-change: height, width;
1622
}

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

Lines changed: 84 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,10 @@ import {VIRTUAL_SCROLL_STRATEGY, VirtualScrollStrategy} from './virtual-scroll-s
4343
preserveWhitespaces: false,
4444
})
4545
export class CdkVirtualScrollViewport implements OnInit, DoCheck, OnDestroy {
46-
private _disconnectSubject = new Subject<void>();
46+
/** Emits when the viewport is detached from a CdkVirtualForOf. */
47+
private _detachedSubject = new Subject<void>();
4748

49+
/** Emits when the rendered range changes. */
4850
private _renderedRangeSubject = new Subject<Range>();
4951

5052
/** The direction the viewport scrolls. */
@@ -53,73 +55,101 @@ export class CdkVirtualScrollViewport implements OnInit, DoCheck, OnDestroy {
5355
/** The element that wraps the rendered content. */
5456
@ViewChild('contentWrapper') _contentWrapper: ElementRef;
5557

56-
/** The total size of all content, including content that is not currently rendered. */
57-
get totalContentSize() { return this._totalContentSize; }
58-
set totalContentSize(size: number) {
58+
/** A stream that emits whenever the rendered range changes. */
59+
renderedRangeStream: Observable<Range> = this._renderedRangeSubject.asObservable();
60+
61+
/**
62+
* The total size of all content (in pixels), including content that is not currently rendered.
63+
*/
64+
_totalContentSize = 0;
65+
66+
/** The transform used to offset the rendered content wrapper element. */
67+
_renderedContentTransform: string;
68+
69+
/** The currently rendered range of indices. */
70+
private _renderedRange: Range = {start: 0, end: 0};
71+
72+
/** The length of the data bound to this viewport (in number of items). */
73+
private _dataLength = 0;
74+
75+
/** The size of the viewport (in pixels). */
76+
private _viewportSize = 0;
77+
78+
/** Whether this viewport is attached to a CdkVirtualForOf. */
79+
private _isAttached = false;
80+
81+
/**
82+
* The scroll handling status.
83+
* needed - The scroll state needs to be updated, but a check hasn't yet been scheduled.
84+
* pending - The scroll state needs to be updated, and an update has already been scheduled.
85+
* done - The scroll state does not need to be updated.
86+
*/
87+
private _scrollHandledStatus: 'needed' | 'pending' | 'done' = 'done';
88+
89+
constructor(public elementRef: ElementRef, private _changeDetectorRef: ChangeDetectorRef,
90+
private _ngZone: NgZone,
91+
@Inject(VIRTUAL_SCROLL_STRATEGY) private _scrollStrategy: VirtualScrollStrategy) {}
92+
93+
/** Gets the length of the data bound to this viewport (in number of items). */
94+
getDataLength(): number {
95+
return this._dataLength;
96+
}
97+
98+
/** Gets the size of the viewport (in pixels). */
99+
getViewportSize(): number {
100+
return this._viewportSize;
101+
}
102+
103+
// TODO(mmalebra): Consider calling `detectChanges()` directly rather than the methods below.
104+
105+
/**
106+
* Sets the total size of all content (in pixels), including content that is not currently
107+
* rendered.
108+
*/
109+
setTotalContentSize(size: number) {
59110
if (this._totalContentSize != size) {
111+
// Re-enter the Angular zone so we can mark for change detection.
60112
this._ngZone.run(() => {
61113
this._totalContentSize = size;
62114
this._changeDetectorRef.markForCheck();
63115
});
64116
}
65117
}
66-
private _totalContentSize = 0;
67118

68-
/** The currently rendered range of indices. */
69-
get renderedRange() { return this._renderedRange; }
70-
set renderedRange(range: Range) {
119+
/** Sets the currently rendered range of indices. */
120+
setRenderedRange(range: Range) {
71121
if (!this._rangesEqual(this._renderedRange, range)) {
122+
// Re-enter the Angular zone so we can mark for change detection.
72123
this._ngZone.run(() => {
73124
this._renderedRangeSubject.next(this._renderedRange = range);
74125
this._changeDetectorRef.markForCheck();
75126
});
76127
}
77128
}
78-
private _renderedRange: Range = {start: 0, end: 0};
79129

80-
/** The offset of the rendered portion of the data from the start. */
81-
get renderedContentOffset(): number { return this._renderedContentOffset; }
82-
set renderedContentOffset(offset: number) {
83-
if (this._renderedContentOffset != offset) {
130+
/** Sets the offset of the rendered portion of the data from the start (in pixels). */
131+
setRenderedContentOffset(offset: number) {
132+
const transform =
133+
this.orientation === 'horizontal' ? `translateX(${offset}px)`: `translateY(${offset}px)`;
134+
if (this._renderedContentTransform != transform) {
135+
// Re-enter the Angular zone so we can mark for change detection.
84136
this._ngZone.run(() => {
85-
this._renderedContentOffset = offset;
86-
this._renderedContentTransform = this.orientation === 'horizontal' ?
87-
`translateX(${offset}px)`: `translateY(${offset}px)`;
137+
this._renderedContentTransform = transform;
88138
this._changeDetectorRef.markForCheck();
89139
});
90140
}
91141
}
92-
private _renderedContentOffset = 0;
93-
94-
/** The length of the data connected to this viewport. */
95-
get dataLength() { return this._dataLength; }
96-
private _dataLength = 0;
97-
98-
/** The size of the viewport. */
99-
get viewportSize() { return this._viewportSize; }
100-
private _viewportSize = 0;
101-
102-
/** A stream that emits whenever the rendered range changes. */
103-
renderedRangeStream: Observable<Range> = this._renderedRangeSubject.asObservable();
104-
105-
_renderedContentTransform: string;
106-
107-
private _connected = false;
108-
109-
private _scrollHandledStatus: 'needed' | 'pending' | 'done' = 'done';
110-
111-
constructor(public elementRef: ElementRef, private _changeDetectorRef: ChangeDetectorRef,
112-
private _ngZone: NgZone,
113-
@Inject(VIRTUAL_SCROLL_STRATEGY) private _scrollStrategy: VirtualScrollStrategy) {}
114142

115-
/** Connect a `CdkVirtualForOf` to this viewport. */
116-
connect(forOf: CdkVirtualForOf<any>) {
117-
if (this._connected) {
118-
throw Error('CdkVirtualScrollViewport is already connected.');
143+
/** Attaches a `CdkVirtualForOf` to this viewport. */
144+
attach(forOf: CdkVirtualForOf<any>) {
145+
if (this._isAttached) {
146+
throw Error('CdkVirtualScrollViewport is already attached.');
119147
}
120148

121-
this._connected = true;
122-
forOf.dataStream.pipe(takeUntil(this._disconnectSubject)).subscribe(data => {
149+
this._isAttached = true;
150+
// Subscribe to the data stream of the CdkVirtualForOf to keep track of when the data length
151+
// changes.
152+
forOf.dataStream.pipe(takeUntil(this._detachedSubject)).subscribe(data => {
123153
const len = data.length;
124154
if (len != this._dataLength) {
125155
this._dataLength = len;
@@ -128,13 +158,13 @@ export class CdkVirtualScrollViewport implements OnInit, DoCheck, OnDestroy {
128158
});
129159
}
130160

131-
/** Disconnect the current `CdkVirtualForOf`. */
132-
disconnect() {
133-
this._connected = false;
134-
this._disconnectSubject.next();
161+
/** Detaches the current `CdkVirtualForOf`. */
162+
detach() {
163+
this._isAttached = false;
164+
this._detachedSubject.next();
135165
}
136166

137-
/** Gets the current scroll offset of the viewport. */
167+
/** Gets the current scroll offset of the viewport (in pixels). */
138168
measureScrollOffset() {
139169
return this.orientation === 'horizontal' ?
140170
this.elementRef.nativeElement.scrollLeft : this.elementRef.nativeElement.scrollTop;
@@ -164,23 +194,26 @@ export class CdkVirtualScrollViewport implements OnInit, DoCheck, OnDestroy {
164194
}
165195

166196
ngOnDestroy() {
167-
this.disconnect();
197+
this.detach();
168198
this._scrollStrategy.detach();
169199

170200
// Complete all subjects
171-
this._disconnectSubject.complete();
201+
this._detachedSubject.complete();
172202
this._renderedRangeSubject.complete();
173203
}
174204

205+
/** Marks that a scroll event happened and that the scroll state should be checked. */
175206
private _markScrolled() {
176207
if (this._scrollHandledStatus === 'done') {
208+
// Re-enter the Angular zone so we can mark for change detection.
177209
this._ngZone.run(() => {
178210
this._scrollHandledStatus = 'needed';
179211
this._changeDetectorRef.markForCheck();
180212
});
181213
}
182214
}
183215

216+
/** Checks if the given ranges are equal. */
184217
private _rangesEqual(r1: Range, r2: Range): boolean {
185218
return r1.start == r2.start && r1.end == r2.end;
186219
}

0 commit comments

Comments
 (0)