Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Virtual scroll flickering when we update it with new data #17540

Closed
roopeshchinnakampalli opened this issue Feb 20, 2019 · 43 comments
Closed

Virtual scroll flickering when we update it with new data #17540

roopeshchinnakampalli opened this issue Feb 20, 2019 · 43 comments
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@roopeshchinnakampalli
Copy link

Bug Report

Ionic version:

[x] 4.0.0

Current behavior:
We've a scenario in our app. We load the cached data from the local store and when we receive the latest data from server, we will update the list.

So when we're doing this, the page is flickering when update happens. I understand that when we set the data it may flicker, but trackBy option should solve this, which is not happening.

Expected behavior:
It should not flicker when we update the list with the latest data.

Steps to reproduce:
Add a viritual-scroll to a page and initialize with some items and update the list with new set of data.

Related code:

https://github.com/roopeshreddy/virtual-scroll-issue

<ion-virtual-scroll [items]="items" [trackBy]="trackByFn">
    <ion-item *virtualItem="let item">
      <ion-icon [name]="item.icon" slot="start"></ion-icon>
      {{ item.title }}
      <div class="item-note" slot="end">
        {{ item.note }}
      </div>
    </ion-item>
  </ion-virtual-scroll>

Other information:

We've tried the same in Chrome browser, iOS simulator and real iPhone 7 device.

Ionic info:

Ionic:

   ionic (Ionic CLI)             : 4.10.3 (/usr/local/lib/node_modules/ionic)
   Ionic Framework               : @ionic/angular 4.0.1
   @angular-devkit/build-angular : 0.12.4
   @angular-devkit/schematics    : 7.2.4
   @angular/cli                  : 7.2.4
   @ionic/angular-toolkit        : 1.4.0

System:

   NodeJS : v10.15.0 (/usr/local/bin/node)
   npm    : 6.5.0
   OS     : macOS Mojave
@ionitron-bot ionitron-bot bot added the triage label Feb 20, 2019
@bolivir
Copy link

bolivir commented Feb 20, 2019

Same problem here

@chrisbinnefeld
Copy link

Same here, despite I could verify that the trackBy function is getting called.

@roopeshchinnakampalli
Copy link
Author

@manucorporat - Any help on this?

@bolivir
Copy link

bolivir commented Feb 21, 2019

Same here, despite I could verify that the trackBy function is getting called.

Can confirm that the trackBy function is called.

@jayordway
Copy link

I see this issue too, if it’s the same thing for what I am seeing it looks like the cards are all being generating and their height expanded/stretched out within milliseconds.

It happens so fast it just looks like a flicker of all the cards being rendered real fast, but overall this gives the app a poor look and experience.

I have two virtual scroll lists that show/hide based on an ngIf, and each time I toggle I see this flicker. Tried to use trackBy, tried to use hidden, but with no luck. Using hidden with virtual scroll is another issue...

@gartorware
Copy link

gartorware commented Apr 19, 2019

Same problem here. As a solution I have set the itemHeight function, so the virtual scroll is not flickering any more.

In my particular case it was easy since all my items have the same height:
HTML:

<ion-virtual-scroll [items]="items" [trackBy]="trackByFn
approxItemHeight="52px" [itemHeight]="itemHeightFn">
   ...
</ion-virtual-scroll>

TS:

...

itemHeightFn(item, index) {
    return 52;
}

Now only the headers are flickering...

@nurfgun
Copy link

nurfgun commented May 21, 2019

Is there any workaround this flickering issue? Unfortunately I am stuck with items of varying heights in the virtual scroll.

---edit----

I have found a solution for my situation!

  1. Do not recreate the array that goes into virtual scroll's [items] (it's already warned in the doc).
  2. When loading more, run virtual scroll's "checkEnd" method.

Hinted from reading the documentation closely!

Happy coding!

@gartorware
Copy link

Are you using headers/footers? Are they flickering if you do not recreate the array?

@gartorware
Copy link

Just tried. For me, headers are still flickering even if you do not recreate the source array.

@heikomat
Copy link

Looking at the source, i'd assume that:

  1. The trackBy is indeed used to track changes (which is why it is being called):
    https://github.com/ionic-team/ionic/blob/8beeff2c52b331eb8901602d88aadea37101e89e/angular/src/directives/virtual-scroll/virtual-scroll.ts#L145

  2. The tracked changes are correctly calculated and then completely ignored, and the whole list gets updated, no matter what changed:
    https://github.com/ionic-team/ionic/blob/8beeff2c52b331eb8901602d88aadea37101e89e/angular/src/directives/virtual-scroll/virtual-scroll.ts#L156-L161

@heikomat
Copy link

heikomat commented Jul 13, 2019

@gartorware using the itemHeight-function does prevent the flicker, but i'd assume (haven't tested it enough) that it is still unnecessarily updating the whole list.

It probably doesn't flicker with itemHeight set, because with itemHeight the virtualScoll doesn't need to ask the browser for positions and sizes of the elemets, so the browser isn't forced to actually paint the items before the virtualScroll can continue.

Setting itemHeight is definitely always a good option, if you can calculate the exact size yourself (without asking the browser for measurements). However, the virtualScroll should only update the parts of the list that changed, regardless of whether itemHeight is set or not.

@anagstef
Copy link
Contributor

Hello! I've just opened a PR adding 2 more functions for header and footer, that work like the itemHeight @gartorware used to resolve the flicker on data change. If this is merged, headerHeight and footerHeight can be optionally used to provide the exact height size of them, resolving the flicker they have.

@liamdebeasi
Copy link
Contributor

Thanks! This issue has been resolved and will be available in the Ionic 4.7.0 release. If anyone encounters new issues with virtual-scroll, please open a new issue.

@heikomat
Copy link

heikomat commented Jul 24, 2019

@liamdebeasi a related issue was resolved, but not this one.

Sure, the headerHeight and footerHeight methods do help if you can calculate those heights, but:

  • trackBy is still broken (because the ion-virtual-scroll always updates every element in the list, even if it is unchanged)
  • and incorrectly documented (the documentation calls it virtualTrackBy, which doesn't exist).

The broken trackBy is a big part of the reason this issue was opened, and the cause of unnecessary re-rendering, and consequently flickering of list items (if you cannot calculate the heights beforehand)

@liamdebeasi liamdebeasi reopened this Jul 25, 2019
@liamdebeasi
Copy link
Contributor

Thanks! I reopened the issue and we will investigate.

@heikomat
Copy link

Thank you for reopening the issue!
If you need some place to start investigating, you might want to take a look an earlier comment from me:
#17540 (comment)

i wouldn't bet money on it, but otherwise i'm somewhat sure, that that is the cause (at least one of the causes) why the trackBy has no effect

@9i0xin9
Copy link

9i0xin9 commented Sep 15, 2019

I just found a work around for the flickering header.
I was checking the css and during the loading part a class .virtual-loading is assign to ion-item-divider element. Forcing the class with a state of opacity: 1 did the trick.

@brandyscarney brandyscarney added package: core @ionic/core package type: bug a confirmed bug report labels Oct 8, 2019
@ionitron-bot ionitron-bot bot removed the triage label Oct 8, 2019
@gaurav-chandra
Copy link

ionic 4.11.4 and the flicker still happens even if approxItemHeight is added. Man, this needs to be a priority.

@vlafranca
Copy link

Inspired by @9i0xin9 It seems I figured it out by putting opacity:1 on my virtualItem div like this :

<div *virtualItem="let item;" style="opacity:1;"> <custom-comp [data]="item" ></custom-comp> </div>

the flicker does not happen anymore (Ionic 4.11.5) but this not a very elegant solution ...

@devner007
Copy link

As mentioned by @9i0xin9, to fix the Header flicker, I used opacity: 1 like so:
<ion-item-divider sticky="true" *virtualHeader="let header" style="opacity:1;">

Now the Header would not flicker, BUT the would start flickering again, despite adding the following function to the <ion-virtual-scroll>:

//HTML
<ion-virtual-scroll [items]="allData" approxItemHeight="320px" [headerFn]="populateListHeader" [itemHeight]="itemHeightFn">

// TS
itemHeightFn(item, index) {
        return 71;
}

As of now, I don't see a way to fix both Virtual Item flicker and Header flicker at the same time. If anyone knows of any solution for this, please share.

Thank you.

@iwantwin
Copy link

iwantwin commented Apr 1, 2020

Seeing the same thing with ionic 5 in chrome, with a very basic virtual scroll, even with just 3 items in the array.

I do use the approxItemHeight.

Ionic info

Ionic:

   Ionic CLI                     : 5.4.13 (C:\Users\Kees\AppData\Roaming\npm\node_modules\ionic)
   Ionic Framework               : @ionic/angular 5.0.0
   @angular-devkit/build-angular : 0.803.23
   @angular-devkit/schematics    : 8.1.3
   @angular/cli                  : 8.1.3
   @ionic/angular-toolkit        : 2.1.2

Utility:

   cordova-res : not installed
   native-run  : not installed

System:

   NodeJS : v12.14.1 (C:\Program Files\nodejs\node.exe)
   npm    : 6.14.4
   OS     : Windows 10

@stephannagel
Copy link

Setting the right heights and approx heights did do the trick for me.

HTML
[headerHeight]="headerHeightFn" [itemHeight]="itemHeightFn" approxItemHeight="50px" approxHeaderHeight="30px"

TS File

headerHeightFn(item, index) {
    return 30;
}

itemHeightFn(item, index) {
   return 50;
}

@mbahiense
Copy link

Same problem here using the ionic version 5, apparently it's related to trackBy, without it it works..

@algertgjoka
Copy link

Same problem with 5.26.0

@rdlabo
Copy link
Contributor

rdlabo commented Jul 15, 2020

I have same idea as #17540 (comment)

trackByFn don't effect to rendering. We expand this line will effect to nodeRender method:

https://github.com/ionic-team/ionic-framework/blob/master/angular/src/directives/virtual-scroll/virtual-scroll.ts#L172-L175

Expected behavior:
trackByFn must effect to rendering. Diff patten handle rendering pattern like Angular ngFor:

https://github.com/angular/angular/blob/master/packages/common/src/directives/ng_for_of.ts#L229-L246

Related code:
https://stackblitz.com/edit/ionic-v4-angular-tabs-4ehr1o

@rdlabo
Copy link
Contributor

rdlabo commented Jul 15, 2020

@emps
Copy link

emps commented Jul 16, 2020

for any new item i get my virtual screll ion list items rebuild,

itemHeight fix the problem

@rdlabo
Copy link
Contributor

rdlabo commented Jul 17, 2020

[INFO] I created support ion-virtual-scroll service sample in angular instead of trackBy:
https://github.com/rdlabo-team/support-virtual-scroll-service

OR you can use virtual-scroll of Angular CDK:
https://github.com/ionic-team/ionic-docs/pull/1406/files

@Exocomp
Copy link

Exocomp commented Nov 15, 2020

For items added to the top none of the work arounds is fixing it for me, I tried itemHeight, trackBy but these don't work when item is added to the top.

I also tried not changing the reference to items and just adding the item with unshift then using checkRange but the problem is calling checkRange(0) creates the flicker while using something like checkRange(0, 1) will update the first item only but not the rest of the items. (Note checkEnd doesn't work since I add the item to the top).

Are there any other solutions?

Version: 5.4.3

@Exocomp
Copy link

Exocomp commented Nov 22, 2020

Regarding the issue I brought up with adding items to the top the virtual scroll is working just fine it was an issue on my side of removing items from the DOM (user error). The only issue I see now is the broken trackBy which as been mentioned previously. Hopefully there is a fix for that at some point.

@BendrissM
Copy link

adding "opacity: 1" seems to have fixed the problem, but it's only a visual fix, the list is still re-rendering, and ItemHeigh is not an option for me, because i have dynamic item heights, a fix for trackBy would be highly appreciated, thanks.

@Simbaclaws
Copy link

Simbaclaws commented Mar 12, 2021

I'm having the same issue as BendrissM, my content is based on the width of the screen and therefore can have differentiating heights. And the list seems to be re-rendering every now and then. This did not happen when I was using ionic version 4.7.1.

After updating to the latest version this bug started to exist. Please make a fix for trackBy.
I am now in the position where I have to tell my clients that because we did an update their app starts to look weird with lists because of this bug :(

@BendrissM
Copy link

@Simbaclaws i have just switched to @angular/cdk and used their version of virtual scroll, that worked well for me.

@Simbaclaws
Copy link

Simbaclaws commented Mar 12, 2021

@BendrissM can I just import @angular/cdk and then use their <cdk-virtual-scroll-viewport> component?
Or would I have to do more to get this to work?

@BendrissM
Copy link

BendrissM commented Mar 12, 2021

@Simbaclaws first you import it in your page's module like this :
import { ScrollingModule } from '@angular/cdk/scrolling';
@NgModule({ imports: [ ... ,ScrollingModule, ... ], declarations: [RefuelsPage] })
and then you can use the component in your html file :

<cdk-virtual-scroll-viewport
    itemSize="72"
    style="height: 760px"
    maxBufferPx="760"
    minBufferPx="760"
    *ngIf="!isLoading && data.length > 0"
>
    <ion-item-sliding
      *cdkVirtualFor="let item of data | filterRf: search; trackBy:trackById"
    >
      <ion-item>
          ......
      </ion-item>
   </ion-item-sliding>
</cdk-virtual-scroll-viewport>

you should specify maxBuffer and minBuffer or else you might face some visual bugs, or at least that's what i've experienced

@Simbaclaws
Copy link

Simbaclaws commented Mar 12, 2021

Hmmm odd, with the same data as with the ion-virtual-scroll it only generates 2 of my components instead of all of them inside the cdk-virtual-scroll-viewport, and I can not see the components at all...

Could I perhaps contact you somehow? I'm having a really hard time building a good solution.

I do not know how big my list will be so setting a height of some sort is not something I can do :/

EDIT: After your edit I got it working 👍 !! Thank you so much!

Is there also a way to scroll the ion-content instead of the viewport within? It seems like some parts of my content get cut off when I scroll inside the viewport. Instead of extending the ion-content...

I don't know how long my list will be so I don't know how high I should set it....

@BendrissM
Copy link

happy it worked, i actually still have this problem, and couldn't bother to fix it tbh , since if you just scroll again you'll see the other content, but i think it should be fixed if you set the viewport height to the height of the screen the it should be working, since i guess in my example, 760px is maybe bigger than the the height of the ion-content, that's why your content get's cut off in the end

@Simbaclaws
Copy link

Simbaclaws commented Mar 12, 2021

I decided to go without virtual scrolling (just put everything in a div), see what the actual performance hit will be. Since most of the solutions don't seem to work well for me. It's not a lot of content that is generated I suppose.

EDIT: I found a solution, I can just put the virtual scroll viewport around the entire page. This solved the issue I was having with a scoll area within the page, rather then scrolling the entire page.

@BendrissM
Copy link

oh good idea, i will try it as soon as i can, thank you. 👍

@Simbaclaws
Copy link

Simbaclaws commented Mar 12, 2021

The only issue I'm still having is that I can't get the scroll position back to a previous state when moving from and to the view.

I think I need to use a customRouteReuseStrategy in order to keep my pages cached to the correct scroll position inside CdkVirtualScrollViewport.
But then I walk into the following issue:
#17673

Where we are not allowed to use our own route reuse strategy...

Calling this.virtualScroll.scrollToIndex(offset); where virtualScroll is of type CdkVirtualScrollViewport doesn't work in ngAfterViewInit(). It only works when I use a timeout, and that will create a jump from the top of the page to the current offset, which looks horrible.

This issue also happens without virtual scroll view, apparently I just can't get back to the same scroll position in a situation where I have:

main view -> detail view -> main view

I'm using router navigation with a custom router history service in order to navigate to the previous page.
So I am not pushing and popping from the router navigation stack, since this causes a wrong navigation stack history for my app. I already have to make workarounds to get a correct navigation history...
Let alone be able to get scroll positions to work correctly.

import { Injectable } from '@angular/core';
import { Router, RoutesRecognized } from '@angular/router';
import { filter, pairwise } from 'rxjs/operators';

@Injectable({
  providedIn: 'root'
})
export class RouterHistoryService {
  previousUrl: any = new Array();
  currentUrl: any;
  blockPrevious = false;

  constructor(
    private router: Router
  ) {
    this.router.events
    .pipe(filter((evt: any) => evt instanceof RoutesRecognized), pairwise())
    .subscribe((event: RoutesRecognized[]) => {
      if (!this.blockPrevious) {
        this.previousUrl.push(event[0].urlAfterRedirects);
        this.blockPrevious = false;
        this.currentUrl = event[0].urlAfterRedirects;
      }
      this.currentUrl = event[0].urlAfterRedirects;
      this.blockPrevious = false;
    });
   }

   comesFromPreviousUrl() {
     this.blockPrevious = true;
   }

   getPreviousUrl() {
     return this.previousUrl.pop();
   }

   getCurrentUrl() {
     return this.currentUrl;
   }

   isLastHistory() {
     return this.previousUrl.length === 0;
   }
}

I'm getting more and more frustrated by the ionic framework because I can't seem to do basic things an app should have.
Maybe I'm just doing it wrong, but so far it seems like I have to write a TON of workarounds for things that should be working out of the box....

EDIT: I was able to fix my issue by creating a low interval that keeps setting the virtual scroll view position until after ionViewDidEnter with a timeout. This causes the page to get scrolled to the correct position when I pass them around using router parameters with NavigationExtras.

@Coder7777
Copy link

Same problem here. As a solution I have set the itemHeight function, so the virtual scroll is not flickering any more.

In my particular case it was easy since all my items have the same height: HTML:

<ion-virtual-scroll [items]="items" [trackBy]="trackByFn
approxItemHeight="52px" [itemHeight]="itemHeightFn">
   ...
</ion-virtual-scroll>

TS:

...

itemHeightFn(item, index) {
    return 52;
}

Now only the headers are flickering...

thanks, you save my day.

@liamdebeasi
Copy link
Contributor

Thanks for the issue! With the release of Ionic 6, we made the decision to deprecate ion-virtual-scroll in favor of 3rd party solutions.

Moving forward, ion-virtual-scroll will only receive critical security fixes, and the component will be removed in Ionic 7. As a result, I am going to close this issue.

We have prepared migration guides for each of the 3 JavaScript frameworks we support, so developers can get started migrating right away.

Migration for Angular
Migration for React
Migration for Vue

Some Ionic components that rely on scrolling inside of ion-content have some limitations when using 3rd party virtual scrollers, but we are actively working on a solution for this. Please follow #23437 for updates. Thanks for your patience as we work to resolve this issue.

We believe this change will lead to a healthier Ionic ecosystem as it frees up resources for the Ionic team to dedicate to other areas of this project while still giving developers options for performant virtual scroll solutions.

Thank you!

@ionitron-bot
Copy link

ionitron-bot bot commented Feb 19, 2022

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Feb 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Development

No branches or pull requests