components icon indicating copy to clipboard operation
components copied to clipboard

bug(COMPONENT): CDK Virtual Scroller jump back/flickers to items on top

Open Donovantxy opened this issue 2 years ago • 7 comments
trafficstars

Is this a regression?

  • [ ] Yes, this behavior used to work in the previous version

The previous version in which this bug was not present was

No response

Description

I'm getting an annoying behaviour while scrolling, where items snap/jump back and I'm never able to scroll all the way down the end of the list. I've notice though, that this bug happens only when I use the virtual scrolling within a material dialog

Template

<cdk-virtual-scroll-viewport class="scroll-viewport" itemSize="20">
   <li class="scan-list__item" cdkVirtualFor="let scan of pointClouds| keyvalue:classesByNameAsc;templateCacheSize:0">
        <div class="scan-list__scan">
            <div class="scan-list__scan__left">
              <div>{{scan.value.name}}</div>
            </div>
        </div>
   </li>
</cdk-virtual-scroll-viewport\>

CSS

.scroll-viewport { height: 200px; }

Reproduction

  • Implement cdk-virtual-scroll-viewport scroller inside a Angular Material dialog
  • in the template set templateCacheSize:0
  • try to scroll down

Expected Behavior

https://github.com/angular/components/assets/6199235/63111f97-c665-4cc0-8f8b-eead8d75a025

Actual Behavior

While scrolling down, the scroll jump up to the first times

Environment

  • Angular: ~11.2.14
  • CDK/Material: ~11.2.13
  • Browser(s): Chrome
  • Operating System (e.g. Windows, macOS, Ubuntu): macOs Catalina

Donovantxy avatar May 16 '23 19:05 Donovantxy

I think when I got this behavior last it was because the itemSize was a tiny bit off of the actual row height. Maybe double check that your itemSize includes border or margins?

aeslinger0 avatar Jul 26 '23 12:07 aeslinger0

@Donovantxy did you manage to fix the issue?

NikGurev avatar Jan 16 '24 08:01 NikGurev

Does anyone have success with this?

RCout1nho avatar Feb 06 '24 19:02 RCout1nho

I am currently experiencing this also, but only when applying templateCacheSize:0 which I need as I need the components to be destroyed and constructed when scrolling.

If I remove this templateCacheSize:0 it scrolls OK.

Anyone figured out the issue here ?

Faulkner368 avatar Feb 21 '24 15:02 Faulkner368

@Faulkner368 the same happened to me, I think this happens because the cached elements increase the size of the container div and it must interfere with the scroll, I don't know if it's a bug in the lib or if it's possible to solve it by changing the container

RCout1nho avatar Feb 21 '24 17:02 RCout1nho

@RCout1nho was wondering the same, if so I dont think there will be a work around as the component is getting destroyed it cant have any height

Faulkner368 avatar Feb 21 '24 18:02 Faulkner368

@RCout1nho @Faulkner368 @NikGurev I have spent a few hours with this problem and I found a solution that worked for me.

Browser: Chrome (Version 124.0.6367.118 (Official Build) (64-bit))

Original layout:

<cdk-virtual-scroll-viewport>
    <div class="cdk-virtual-scroll-content-wrapper"></div>
    <div class="cdk-virtual-scroll-spacer"></div>
</cdk-virtual-scroll-viewport>

New layout:

<cdk-virtual-scroll-viewport>
    <div class="cdk-virtual-scroll-spacer"></div>
    <div class="cdk-virtual-scroll-content-wrapper"></div>
</cdk-virtual-scroll-viewport>

I moved .cdk-virtual-scroll-spacer before .cdk-virtual-scroll-content-wrapper

EmilOlovsson avatar May 11 '24 03:05 EmilOlovsson

~I ended up using the appendOnly mode~

<cdk-virtual-scroll-viewport
            appendOnly
            itemSize="68"
            minBufferPx="1020"
            maxBufferPx="1360">
</cdk-virtual-scroll-viewport>

https://github.com/angular/components/issues/27104#issuecomment-2141015215

shiv19 avatar May 30 '24 00:05 shiv19

@RCout1nho @Faulkner368 @NikGurev I have spent a few hours with this problem and I found a solution that worked for me.

Browser: Chrome (Version 124.0.6367.118 (Official Build) (64-bit))

Original layout:

<cdk-virtual-scroll-viewport>
    <div class="cdk-virtual-scroll-content-wrapper"></div>
    <div class="cdk-virtual-scroll-spacer"></div>
</cdk-virtual-scroll-viewport>

New layout:

<cdk-virtual-scroll-viewport>
    <div class="cdk-virtual-scroll-spacer"></div>
    <div class="cdk-virtual-scroll-content-wrapper"></div>
</cdk-virtual-scroll-viewport>

I moved .cdk-virtual-scroll-spacer before .cdk-virtual-scroll-content-wrapper

Can you explain how you achieved this please? Since these are auto generated for me when I use cdkVirtualFor, and I also encounter this scrolling issue.

BdDragos avatar May 30 '24 09:05 BdDragos

@BdDragos I found there were two causes, not having correctly calculated the itemSize but this is a well known cause and was not the issue for me, which was caused by using templateCacheSize:0.

Setting the cache to zero was not ideal since I want as much of a performance boost as possible so to understand my solution you need to understand why I was setting cache to zero.

I am working with existing, large and complex components that ideally would have been refactored but I did not have time for that. These components were processing and changing property values during construction and / or during OnInit life cycle which resulted in an issue where I was seeing the wrong text on screen for a component after scrolling and the bound data changing which occured because instead of constructing new components when scrolling it reuses previously constructed components by changing the bound data and during change detection this would get picked up; however the component is an existing one so the constructor and the OnInit method is not executed again so the processing that was needed in my case did not occur and was thus seeing the wrong values.

Setting cache to zero means it would not reuse existing components and would construct new ones, thus solving my issue. However it introduced the issue discussed here and I suspect its due to the calculations on itemSize when components are being destroyed and new ones created there is a brief moment where total height is not adding up... that is my suspicion anyway.

So I did a hacky fix as had insufficient time for a refactor on these large components, so I removed the templateCacheSize:0 and used OnChanges to do the processing that was taking place originally in OnInit and in the constructor so when the bindings are updated so when change detection runs everything was displayed as it should be.

So in summary I fixed this by removing templateCacheSize:0 and achieved my goal by using OnChanges to do the processing that was previously happening in OnInit and constructor, hope that helps

Faulkner368 avatar May 30 '24 09:05 Faulkner368

For me the bug happens with and without the templaceCacheSize (but templateCacheSize not to 0 fixes it in most cases*). Unfortunately in our case ngOnChanges is not a solution since there are some very complex components that need to be rendered and I also depend on other browser limitations that would make the processing not possible without a lot of refactoring and extra code.

BdDragos avatar May 30 '24 09:05 BdDragos

I hadn't seen this without templaceCacheSize:0 and most often seemed from stackoverflow and the likes to be caused by element sizes not matching what you have defined in itemSize, have you physically checked in the DOM he heights of all the visible elements, I think I initially had issue not having accounted for border

Faulkner368 avatar May 30 '24 09:05 Faulkner368

I hadn't seen this without templaceCacheSize:0 and most often seemed from stackoverflow and the likes to be caused by element sizes not matching what you have defined in itemSize, have you physically checked in the DOM he heights of all the visible elements, I think I initially had issue not having accounted for border

Yes, I set the itemSize to two decimals accuracy. I also tried to make it "perfect" and intentionally not perfect (small variations), still the same issue.

BdDragos avatar May 30 '24 09:05 BdDragos

Hard to know then without seeing it first hand, maybe if you have time see if can reproduce in something like https://stackblitz.com/ and share here or Stackoverflow and someone maybe able to help but in the least can help narrow down the cause when in a clean / simple app

Faulkner368 avatar May 30 '24 10:05 Faulkner368

@RCout1nho @Faulkner368 @NikGurev I have spent a few hours with this problem and I found a solution that worked for me. Browser: Chrome (Version 124.0.6367.118 (Official Build) (64-bit)) Original layout:

<cdk-virtual-scroll-viewport>
    <div class="cdk-virtual-scroll-content-wrapper"></div>
    <div class="cdk-virtual-scroll-spacer"></div>
</cdk-virtual-scroll-viewport>

New layout:

<cdk-virtual-scroll-viewport>
    <div class="cdk-virtual-scroll-spacer"></div>
    <div class="cdk-virtual-scroll-content-wrapper"></div>
</cdk-virtual-scroll-viewport>

I moved .cdk-virtual-scroll-spacer before .cdk-virtual-scroll-content-wrapper

Can you explain how you achieved this please? Since these are auto generated for me when I use cdkVirtualFor, and I also encounter this scrolling issue.

@BdDragos This should work for you.

// HTML


<cdk-virtual-scroll-viewport
	#virtualscroll
	autosize
	[minBufferPx]="A_NUMBER_HERE"
	[maxBufferPx]="A_NUMBER_HERE"
>
	// Your content here
</cdk-virtual-scroll-viewport>

// .ts file

import {CdkVirtualScrollViewport} from "@angular/cdk/scrolling";

@ViewChild('virtualscroll') private _virtualscrollRef?: CdkVirtualScrollViewport;

ngAfterViewInit() {
	if (this._virtualscrollRef) {
		// @ts-ignore
		const scrollable = this._virtualscrollRef.scrollable.elementRef.nativeElement;
		if (scrollable && scrollable.children) {
			const spacer = scrollable.children[1];
			if (spacer && spacer.classList.contains('cdk-virtual-scroll-spacer')) {
				// Move spacer as first child
				scrollable.insertBefore(spacer, scrollable.firstChild);
			} else {
				console.error("Could not find spacer");
			}
		} else {
			console.error("Could not find scrollable");
		}
	}
}

EmilOlovsson avatar May 30 '24 10:05 EmilOlovsson

I succeeded in shifting the elements with a JavaScript type of solution after I thought a bit.

    effect(() => {
      const nativElem = this.cdkScrollElement()?.elementRef.nativeElement;
      if (nativElem) {
        const last = nativElem.lastElementChild;
        if (nativElem.firstElementChild && last) {
          nativElem.replaceChild(nativElem.firstElementChild, nativElem.lastElementChild);
          nativElem.insertBefore(last, nativElem.firstElementChild);
        }
      }
    });

the above will be put into the constructor(), and it will use a signal based viewChild to trigger as soon as the rendering of the cdkScrollElement is done. The viewChild should be binded directly to the <cdk-virtual-scroll-viewport tag

public cdkScrollElement = viewChild<CdkVirtualScrollViewport | undefined>('cdkScrollElement');

This fixed my issue it seems, now I can scroll without problems. Thanks for the suggestion. @shiv19

Hard to know then without seeing it first hand, maybe if you have time see if can reproduce in something like https://stackblitz.com/ and share here or Stackoverflow and someone maybe able to help but in the least can help narrow down the cause when in a clean / simple app

It would take too much time to set up everything, the behaviour is pretty complex, but the shifting did it.

BdDragos avatar May 30 '24 10:05 BdDragos

@BdDragos In my case the actual problem was not having the correct item size, and also rending category headers just inside the for loop even though they were not a part of the list itself. I changed the category headers to be a part of the list and gave a fixed height for the category rows and the list item rows. And everything is working fine now. I even removed the appendOnly mode.

This is fine when all the list items can have a fixed height. When items can't have a fixed height, we'll need to use a custom virtual scroll strategy.

I found a few articles that talk about this:

https://dev.to/georgii/virtual-scrolling-of-content-with-variable-height-with-angular-3a52#final-code

https://angularindepth.com/posts/1091/writing-custom-virtual-scroll-strategy

And here's some code from the CDK experimental: https://github.com/angular/components/blob/main/src%2Fcdk-experimental%2Fscrolling%2Fauto-size-virtual-scroll.ts

shiv19 avatar May 30 '24 23:05 shiv19

I can't believe it! switching the spacer fixed the issue for me also. Should this change be brought to the CDK?

canadakickass avatar Jun 11 '24 13:06 canadakickass

switching the spacer fixed my issue as well. Im currently using Angular 17, and my issue is not caused by the itemSize and templaceCacheSize:0.

EdGuan avatar Jun 18 '24 22:06 EdGuan