virtual scroll: implement scrollTo functions #11498

amcdnl · 2018-05-24T18:50:31Z

This PR adds the ability for users to scroll to an offset or an index with the ability to do so smoothly and/or lazily.

Partial fix for #10116 (scrollToIndex only supported for fixed size strategy)

mmalerba · 2018-05-24T19:09:03Z

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

@@ -151,6 +151,11 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
    }
  }

+  /** Get the offset for the given index. */
+  getOffsetForIndex(index: number) {
+    return index * this._averager.getAverageItemSize();


I think this case is a little more complex. we should use the currently rendered content as a reference point and fallback to the estimate for items that aren't rendered

I'm not sure I follow? Can you explain a bit more?

The average item size is just a best guess, but for items that are already rendered we don't have to guess, we can just measure them. For ones that are not rendered we use the guess.

e.g. Let's say elements 50-100 are rendered. I'm scrolled to item 55, I want to go to 45. I can measure items 50-55 + 5*averageSize for the non-rendered items

The more I think about this the more I think this is a bad idea. Would you be ok with throwing an error here and saying its not supported? The delta could be way off very easily. I think we would just get plagued with bugs if we tried to do this.

Yes, it could be way off but I have logic to slowly correct the error in the scroll listener code, so I think it should be ok

Gotcha. I think we need a mechanism to cache the index of the item with its respective size so i can calculate the deltas. Thoughts?

mmalerba · 2018-05-24T19:09:32Z

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

@@ -37,4 +37,7 @@ export interface VirtualScrollStrategy {

  /** Called when the offset of the rendered items changed. */
  onRenderedOffsetChanged();
+
+  /** Get the offset for the given index. */
+  getOffsetForIndex(index: number);


Lets call it getScrollOffsetForIndex just to be super clear

mmalerba · 2018-05-24T20:53:07Z

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

+    const left = this.orientation === 'horizontal' ? offset : 0;
+
+    let shouldScroll = true;
+    if (options.lazy) {


I'm not sure if this logic is quite right because the rendered content may be larger than the viewport.

Your right. I changed it to measure the viewport size rather than content.

I'm wondering if lazy scrolling is a common enough use case to be something we want to include in the API. It's pretty easy to implement on top if we just provide the basic functionality

mmalerba · 2018-05-24T20:54:24Z

src/cdk/platform/features.ts

@@ -27,6 +27,25 @@ export function supportsPassiveEventListeners(): boolean {
  return supportsPassiveEvents;
 }

+/** Cached result of whether the user's browser supports scroll behaviors. */
+let scrollSupport = true;


did you mean to init to null?

amcdnl · 2018-05-31T19:34:07Z

@mmalerba - Any idea why the tests fail when using scrollTo?

naveedahmed1 · 2018-07-10T09:38:45Z

Any update on this?

amcdnl · 2018-07-15T19:17:50Z

nudge @mmalerba

mmalerba

Sorry about that, this fell off my radar after vacation

mmalerba · 2018-07-16T16:23:57Z

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

@@ -37,4 +37,7 @@ export interface VirtualScrollStrategy {

  /** Called when the offset of the rendered items changed. */
  onRenderedOffsetChanged();
+
+  /** Get the offset for the given index. */
+  getScrollOffsetForIndex(index: number);


I think we may want to put the whole scrollToIndex function as part of the strategy and just have the viewport delegate. While the autosize strategy might not know exactly where the item is, I think we can create an implementation later on that just kind of guesses. It would require the strategy to have full control over what the scroll method does though, not just spitting back an offset.

mmalerba · 2018-07-16T16:24:33Z

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

@@ -151,6 +151,11 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
    }
  }

+  /** Get the offset for the given index. */
+  getScrollOffsetForIndex() {
+    throw new Error('Offset location is not supported with autosize.');


Let's say "not currently supported", this sounds like it never will be

Does this mean that the scrollToIndex will not be supported for autosize in initial version?

The autosize strategy will probably stay in cdk-experimental a little longer than the fixed size strategy, while we work out some of the bugs. This will be implemented before it goes into cdk

mmalerba · 2018-07-16T16:28:15Z

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

+    const left = this.orientation === 'horizontal' ? offset : 0;
+
+    let shouldScroll = true;
+    if (options.lazy) {


I'm wondering if lazy scrolling is a common enough use case to be something we want to include in the API. It's pretty easy to implement on top if we just provide the basic functionality

amcdnl · 2018-07-22T13:50:51Z

Closing given #12272

@amcdnl

…12272) Note: `scrollToIndex` currently does not work with the `AutoSizeVirtualScrollStrategy`. Support for this will be added in a future PR. This PR is based off #11498 by @amcdnl. I removed the `lazy` option from #11498 to avoid bloating the API. I'm not sure how common the use case is, and its relatively simple to implement on top using the current APIs.

@amcdnl

…12272) Note: `scrollToIndex` currently does not work with the `AutoSizeVirtualScrollStrategy`. Support for this will be added in a future PR. This PR is based off #11498 by @amcdnl. I removed the `lazy` option from #11498 to avoid bloating the API. I'm not sure how common the use case is, and its relatively simple to implement on top using the current APIs.

angular-automatic-lock-bot · 2019-09-09T07:02:34Z

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.}

virtual scroll: implement scrollTo functions

868bbf9

amcdnl self-assigned this May 24, 2018

amcdnl requested a review from mmalerba May 24, 2018 18:50

amcdnl requested review from devversion and jelbourn as code owners May 24, 2018 18:50

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label May 24, 2018

mmalerba reviewed May 24, 2018

View reviewed changes

fix(scroll): fix scrolling not working all the time

a31a4c8

amcdnl added 3 commits May 31, 2018 14:35

chore: pr nit

017a131

chore: throw exception for offset getter

2527306

chore: better checks for scroll smooth

05c29b9

Merge branch 'master' into scroll-to

dce1cad

mmalerba reviewed Jul 16, 2018

View reviewed changes

mmalerba mentioned this pull request Jul 18, 2018

virtual-scroll: add support for scrollToOffset and scrollToIndex #12272

Merged

amcdnl closed this Jul 22, 2018

amcdnl deleted the scroll-to branch July 22, 2018 13:50

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

virtual scroll: implement scrollTo functions #11498

virtual scroll: implement scrollTo functions #11498

Uh oh!

Conversation

amcdnl commented May 24, 2018 • edited by mmalerba Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amcdnl commented May 31, 2018

Uh oh!

naveedahmed1 commented Jul 10, 2018

Uh oh!

amcdnl commented Jul 15, 2018

Uh oh!

mmalerba left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amcdnl commented Jul 22, 2018

Uh oh!

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

Uh oh!

Uh oh!

amcdnl commented May 24, 2018 •

edited by mmalerba

Loading