virtual-scroll: merge into master under cdk-experimental #11354

mmalerba · 2018-05-15T22:49:22Z

Current state:

Virtual scroll with fixed size items works pretty well
Virtual scroll with auto-size items works decently well, but has a few known bugs

Next steps:

Fix remaining bugs with auto-size mode
Continue working on performance improvements for both modes
Add missing features (e.g. responding to size changes, scroll to particular index). See the virtual scrolling project for a list of open bugs and feature requests
Integrate with existing material components

* feat(virtual-scroll): fixed size virtual scroll * address some comments * change VirtualScrollStrategy interface a bit * address some more comments * fix lint & build

* virtual-scroll: only move views that need to be moved * address comments

* virtual-scroll: switch throttleTime to sampleTime * add comment

… native `Range` (angular#10220)

* rename fixed size virtual scroll directive * add autosize virtual scroll strategy * add item size estimator class * add logic for jumping rendered content based on scroll position * address comments

…y` (angular#10290) * virtual-scroll: add `onContentRendered` hook to `VirtualScrollStrategy` * address comemnts

…lStrategy` (angular#10504) * virtual-scroll: add incremental scroll logic in `AutosizeVirtualScrollStrategy`. This still has a couple issues that need to be ironed out and it doesn't have the code for correcting the error between the predicted and actual scroll position. (See various TODOs for additional things that need work). * fix lint * address comments * address comments

…re items were removed too soon (angular#10986) * rewrite offsets to the end of the rendered content as offsets to the start * add some more autosize demos for testing * make sure not to remove too many items * address comments

* rewrite offsets to the end of the rendered content as offsets to the start * add some more autosize demos for testing * make sure not to remove too many items * virtual-scroll: address amcdnl's feedback

…ngular#11085)

angular#11137) * virtual-scroll: add logic to correct the scroll position as user moves toward the top * address comments

* fix(scrolling): adds right to fix pushed content * chore: address feedback * chore: address pr feedback

* virtual-scroll: add tests for fixed size items * address comments * fix bug in fixed size strategy * fix bazel build

* merge fixed size test components into one * add tests for cdkVirtualFor logic * allow undefined to be explicitly passed as trackBy * fix bazel build * address comments

…1345) * set up virtual scroll page in e2e app * add gulp task for e2e:watch * add e2e tests for autosize * address comments

amcdnl · 2018-05-16T11:52:54Z

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

+    // changes.
+    this._forOf.dataStream.pipe(takeUntil(this._detachedSubject)).subscribe(data => {
+      const len = data.length;
+      if (len != this._dataLength) {


Wouldn't you want !== ?

amcdnl · 2018-05-16T11:53:14Z

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

+   * rendered.
+   */
+  setTotalContentSize(size: number) {
+    if (this._totalContentSize != size) {


Wouldn't you want !== ?

amcdnl

Looks good overall, few comments.

amcdnl · 2018-05-16T11:59:14Z

What are your thoughts on adding these 3 pending PRs I have to this?

googlebot · 2018-05-16T17:05:13Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

googlebot · 2018-05-16T17:05:41Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

googlebot · 2018-05-16T17:45:30Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

googlebot · 2018-05-16T17:50:36Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

) * fix(scrolling): data change not updating size * chore: remove todo * chore: more performant fix for jump

googlebot · 2018-05-16T18:30:48Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

googlebot · 2018-05-16T18:31:20Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

angular-automatic-lock-bot · 2019-09-09T00:02:22Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

mmalerba and others added 27 commits April 5, 2018 12:26

feat(virtual-scroll): fixed size virtual scroll (angular#9316)

3967a64

* feat(virtual-scroll): fixed size virtual scroll * address some comments * change VirtualScrollStrategy interface a bit * address some more comments * fix lint & build

chore: move virtual-scroll to cdk-experimental (angular#9974)

1c19ca4

virtual-scroll: simplify scroll listener logic (angular#10102)

7495ee7

virtual-scroll: only move views that need to be moved (angular#10099)

e0591b3

* virtual-scroll: only move views that need to be moved * address comments

virtual-scroll: switch throttleTime to sampleTime (angular#10179)

55da6b2

* virtual-scroll: switch throttleTime to sampleTime * add comment

virtual-scroll: allow user to pass Observable<T[]> (angular#10158)

1471964

virtual-scroll: rename Range to ListRange to avoid confusion with…

ece59ba

… native `Range` (angular#10220)

virtual-scroll: add autosize scroll strategy (angular#10219)

b105916

* rename fixed size virtual scroll directive * add autosize virtual scroll strategy * add item size estimator class * add logic for jumping rendered content based on scroll position * address comments

virtual-scroll: add onContentRendered hook to `VirtualScrollStrateg…

f04b470

…y` (angular#10290) * virtual-scroll: add `onContentRendered` hook to `VirtualScrollStrategy` * address comemnts

fix bazel

3acadda

fix devapp

2538942

fix lint

8565c60

Merge remote-tracking branch 'upstream/master' into virtual-scroll

0bba2dc

cleanup

97d623b

Merge remote-tracking branch 'upstream/master' into virtual-scroll

1f9051e

virtual-scroll: address amcdnl's feedback (angular#10988)

285fe6c

* rewrite offsets to the end of the rendered content as offsets to the start * add some more autosize demos for testing * make sure not to remove too many items * virtual-scroll: address amcdnl's feedback

virtual-scroll: fix updating when data changes and add trackBy demos (a…

0e4a580

…ngular#11085)

virtual-scroll: add logic to correct the scroll position as user move… (

604f743

angular#11137) * virtual-scroll: add logic to correct the scroll position as user moves toward the top * address comments

fix(scrolling): adds right to fix pushed content (angular#11192)

11783b7

* fix(scrolling): adds right to fix pushed content * chore: address feedback * chore: address pr feedback

virtual-scroll: add tests for fixed size items (angular#11255)

5ad55da

* virtual-scroll: add tests for fixed size items * address comments * fix bug in fixed size strategy * fix bazel build

virtual-scroll: add tests for cdkVirtualFor logic (angular#11275)

c196eec

* merge fixed size test components into one * add tests for cdkVirtualFor logic * allow undefined to be explicitly passed as trackBy * fix bazel build * address comments

add some basic tests (angular#11295)

4bef998

Merge remote-tracking branch 'upstream/master' into virtual-scroll

0cb7d70

fix lint

2253b6d

virtual-scroll: add e2e tests for autosize scroll strategy (angular#1…

9497bad

…1345) * set up virtual scroll page in e2e app * add gulp task for e2e:watch * add e2e tests for autosize * address comments

mmalerba assigned amcdnl, jelbourn and crisbeto May 15, 2018

amcdnl reviewed May 16, 2018

View reviewed changes

amcdnl approved these changes May 16, 2018

View reviewed changes

address comments

341bb15

googlebot added cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels May 16, 2018

mmalerba added cla: yes PR author has agreed to Google's Contributor License Agreement and removed cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla labels May 16, 2018

mmalerba added 2 commits May 16, 2018 10:32

Merge remote-tracking branch 'upstream/master' into virtual-scroll

78e27b6

fix lint and tests

63e058e

googlebot added cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels May 16, 2018

fix bad rebase

c4177ff

mmalerba added cla: yes PR author has agreed to Google's Contributor License Agreement and removed cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla labels May 16, 2018

fix(scrolling): scrollbar jump when data change on scroll (angular#11193

fe54f73

) * fix(scrolling): data change not updating size * chore: remove todo * chore: more performant fix for jump

googlebot added cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels May 16, 2018

mmalerba added cla: yes PR author has agreed to Google's Contributor License Agreement and removed cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla labels May 16, 2018

mmalerba merged commit f59658d into angular:master May 22, 2018

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 9, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

virtual-scroll: merge into master under cdk-experimental #11354

virtual-scroll: merge into master under cdk-experimental #11354

Uh oh!

mmalerba commented May 15, 2018

Uh oh!

amcdnl May 16, 2018

Uh oh!

amcdnl May 16, 2018

Uh oh!

amcdnl left a comment

Uh oh!

amcdnl commented May 16, 2018

Uh oh!

googlebot commented May 16, 2018

Uh oh!

googlebot commented May 16, 2018

Uh oh!

googlebot commented May 16, 2018

Uh oh!

googlebot commented May 16, 2018

Uh oh!

googlebot commented May 16, 2018

Uh oh!

googlebot commented May 16, 2018

Uh oh!

angular-automatic-lock-bot bot commented Sep 9, 2019

Uh oh!

Uh oh!

virtual-scroll: merge into master under cdk-experimental #11354

virtual-scroll: merge into master under cdk-experimental #11354

Uh oh!

Conversation

mmalerba commented May 15, 2018

Uh oh!

amcdnl May 16, 2018

Choose a reason for hiding this comment

Uh oh!

amcdnl May 16, 2018

Choose a reason for hiding this comment

Uh oh!

amcdnl left a comment

Choose a reason for hiding this comment

Uh oh!

amcdnl commented May 16, 2018

Uh oh!

googlebot commented May 16, 2018

Uh oh!

googlebot commented May 16, 2018

Uh oh!

googlebot commented May 16, 2018

Uh oh!

googlebot commented May 16, 2018

Uh oh!

googlebot commented May 16, 2018

Uh oh!

googlebot commented May 16, 2018

Uh oh!

angular-automatic-lock-bot bot commented Sep 9, 2019

Uh oh!

Uh oh!