ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
51.07k stars 13.51k forks source link

bug: ion-virtual-scroll overlapping items #18409

Closed andsee closed 2 years ago

andsee commented 5 years ago

Bug Report

Ionic version: 4.4

Current behavior: When using a virtual scroll component containing items of varying height with an infinite scroll occasionally the height of some items is approxItemHeight instead of calculating the height which causes overlapping.

Expected behavior: It should be possible to have items of varying heights within a virtual scroll component when using infinite-scroll.

Steps to reproduce: I've created a simple test app with which you can reproduce the issue. It's available here: https://github.com/andsee/Ionic-Virtual-Scroll-Overlap-Bug

Run in chrome while inspecting in a portrait iPhone X and scroll down, keep scrolling until you see some overlapping. Note I've set the approxItemHeight to just 10px as this makes it easier to reproduce, it still occurs with larger approximations. Occasionally I've had to scroll down to item 200+, other times it happens quickly. It's probably to do with the speed at which you scroll.

image

Here is a video of it occurring: https://youtu.be/uu_R5L7KIOE Related code: Full project allowing repro: https://github.com/andsee/Ionic-Virtual-Scroll-Overlap-Bug

<ion-header>
  <ion-toolbar>
    <ion-buttons slot="start">
      <ion-menu-button></ion-menu-button>
    </ion-buttons>
    <ion-title>
      Home
      <ion-button (click)="refresh()">Refresh</ion-button>
    </ion-title>
  </ion-toolbar>
</ion-header>

<ion-content>
    <ion-virtual-scroll #vScroll [items]=ourItems approxItemHeight="10px">
      <div *virtualItem="let item">
        <app-dynamic-one  [layoutData]="item">
        </app-dynamic-one>
      </div>
    </ion-virtual-scroll>
    <ion-infinite-scroll threshold="100%" (ionInfinite)="infiniteScroll($event)" style="z-index: 99999">
      <ion-infinite-scroll-content style="z-index: 99999;"
              loadingSpinner="bubbles"
              loadingText="Loading more ...">
      </ion-infinite-scroll-content>
    </ion-infinite-scroll>
</ion-content>
import { Component, ViewChild } from '@angular/core';
import { IonVirtualScroll } from '@ionic/angular';

@Component({
  selector: 'app-home',
  templateUrl: 'home.page.html',
  styleUrls: ['home.page.scss'],
})
export class HomePage {

  @ViewChild('vScroll', { read: IonVirtualScroll }) vScroll: IonVirtualScroll;
  ourItems:any[] = []
  constructor()
  {
    this.add(10);
  }

  refresh()
  {
    this.vScroll.checkRange(0);
  }

  add(howMany:number)
  {
    var offset = this.ourItems.length;
    console.log("Add elements "+offset+" to "+(offset+howMany-1));
    for(var i = offset; i < offset+howMany; i++)
    {
      this.ourItems.push({
        index:i
      });
    }
  }

  infiniteScroll(event)
  {
    this.add(10);
    this.vScroll.checkEnd();
    event.target.complete();
  }

}

Other information:

Ionic info:

Ionic:
   ionic (Ionic CLI)             : 4.12.0 (/usr/local/lib/node_modules/ionic)
   Ionic Framework               : @ionic/angular 4.4.0
   @angular-devkit/build-angular : 0.13.9
   @angular-devkit/schematics    : 7.3.9
   @angular/cli                  : 7.3.9
   @ionic/angular-toolkit        : 1.5.1

System:
   NodeJS : v10.15.3 (/usr/local/bin/node)
   npm    : 6.9.0
   OS     : macOS High Sierra
positlabs commented 5 years ago

Confirmed, this is still an issue in 4.6.0 AND dev

positlabs commented 5 years ago

Seems like the code path for stencil usage never measures the elements. I tried hacking at it for a bit, but it only works once after the first scroll event, then things get janked up.

      this.el.forceUpdate();
      this.virtualDom.forEach((node: any) => {
        const domNode: any = this.el.children[node.cell.i - this.range.offset];
        if (!domNode) { return; }
        const update = () => {
          const style = window.getComputedStyle(domNode);
          const height = domNode.offsetHeight + parseFloat(style.getPropertyValue('margin-bottom'));
          this.setCellHeight(node.cell, height);
        };
        domNode && domNode.componentOnReady ? node.componentOnReady().then(update) : update();
      });
zeusstl commented 5 years ago

In version 4.6.2, ion-virtual-scroll still has issues. In fact, the update seems to have broken a working ion-virtual-scroll.

I decided to dig in a bit to see if I could isolate the problem since I seem to perpetually have ion-virtual-scroll issues like this:

My code: I removed all of my custom css to make sure it wasn't causing issues.

<ion-content>
        <ion-virtual-scroll #virtualScroll [items]="sessionsList" [approxItemHeight]="18" [approxHeaderHeight]="1" [approxFooterHeight]="1">
            <ion-card *virtualItem="let session;">
                Hi
            </ion-card>
        </ion-virtual-scroll>
</ion-content>

How it renders: (Note that there are 6 cards instead of 11 showing)

Screen Shot 2019-07-11 at 7 06 24 PM

Tried switching to items instead of cards, which helped, but I'm trying to use cards. Not items.

Screen Shot 2019-07-11 at 7 05 56 PM

Added a console.log() in: node_modules/@ionic/angular/node_modules/@ionic/core/dist/esm/legacy/ion-virtual-scroll.entry.js

The height should be 18px for each. First render ( in the console log) shows empty. Second render - Note the top field is multiples of 18:

Screen Shot 2019-07-11 at 10 13 14 PM

Third render - Note the top field is suddenly multiples of 28 for some reason:

Screen Shot 2019-07-11 at 10 13 30 PM

The solution for me, for the time being, was to put the ion-card inside of an ion-item.

This is especially ironic since the documentation for ion-virtual-scroll gives the example using a card: https://ionicframework.com/docs/api/virtual-scroll

Screen Shot 2019-07-11 at 10 57 31 PM

Hope all of this helps someone find the source of this issue and resolve this once and for all. I feel like I've spent more time debugging ion-virtual-scroll since (I think) ionic v1, than debugging anything else in ionic.

dorontal commented 4 years ago

You may want to watch or join in on https://github.com/ionic-team/ionic/issues/16632 - it lists a lot of related bugs.

mrSingh007 commented 4 years ago

I am also facing this overlapping problem. :/ any workaround for fix for this problem

<ion-content *ngIf="($myObs | async) as tests; else loading">
  <ion-virtual-scroll [items]="tests" approxItemHeight="320px">
    <div *virtualItem="let item;">
    <app-demo [field]="item" ></app-demo>
    </div>
  </ion-virtual-scroll>
</ion-content>
elvisgraho commented 4 years ago

Same Issue. Its been a long one and had not been fixed for years https://github.com/ionic-team/ionic/issues/11542 If you have and observable that throws data at virtual-scroll, you will have an overlapping issue.

Can we fix this after 3 years?

andsee commented 4 years ago

Well it's still with us in Version 5 :-(

They are aware that Virtual Scroll is broken. I'm hoping it will be fixed this year else we are looking at a V6 fix, I'm not quite sure what to read from the following video about V5 at '30:41' when @brandyscarney says "Are we allowed to mention Virtual Scroll?' in response to beginning to talk about V6. I'm hoping she's suggesting it may come before hand.

https://www.youtube.com/watch?v=eegTEIRDlhg

However when I spoke to Ionic support Mid 2019 they suggested that they were looking at Angular's virtual scroll and had found that the manner in which it works would fit better into the ecosystem. This suggests to me there may be an API breaking change coming in V6.

Ionic pointed me towards the following: https://github.com/mhartington/cdk-virtual-scroll-ionic

I've not looked into this as a replacement yet, as we are not near release and are ignoring the overlap for now, preying it will be fixed.

If anyone does try Angular's solution and it solves this issue please post here so that others can gain confidence to go give it a go.

dmunicio commented 4 years ago

I have reproduced the bug with v5. I think that the problem is that ion-virtual-scroll uses an internal list called "heightIndex" which contains the height of every item inside. When there are ion-card with ion-img inside and it doesn't have a height set by css class, it seems that the height is not updated when ion-img src is loaded.

I'm not an ionic expert but I think i have found a workaround by using ionImgDidLoad. Maybe there is a cleaner solution but I hope it helps.

In page.html

  <ion-virtual-scroll [items]="myitems" [approxItemHeight]="500">
    <ion-card *virtualItem="let myitem; let i = index">
      <ion-img (ionImgDidLoad)="_loaded(i)" class="card-img" src="https://dummyimage.com/600x400"></ion-img>
      <ion-card-header><ion-card-title>MyTitle</ion-card-title></ion-card-header>
      <ion-card-content>
        My content
      </ion-card-content>
    </ion-card>
  </ion-virtual-scroll>

In page.ts

export class MyPage {
  @ViewChild(IonVirtualScroll, {read: IonVirtualScroll, static: false}) scroll: IonVirtualScroll;
  _loaded(i: any) {
    this.scroll.checkRange(i,5);
    console.log(i);
  }
}
coolvasanth commented 4 years ago

@dmunicio are you able to get the index of the list with the help of let i = index ?

next-diegomunicio commented 4 years ago

Yes, if you run that code, you can see the output of console.log(i) while making scroll. 1 2 3 And so on...

bgibers commented 4 years ago

This issue is majorly affecting my app. Does anyone have a solid alternative to the virtual scroll card combo?

bgibers commented 4 years ago

It seems like using a regular <img> tag rather than <ion-img> within <ion-card-content> has fixed the issue for me.

zeusstl commented 4 years ago

I have been using <img> for a long time and now I'm finding out that a few users are getting this issue. (I don't see the issue when I test locally, but various users are still seeing this issue in their browsers.)

xXcoronaXx commented 4 years ago

Hi everyone this is my fix/workaround for this problem:

.fixOverlapp {
  height: 100% !important;
  overflow: auto;
}
.fixOverlapp >.virtual-item {
  position: initial !important;
  transform: initial !important;
}
<ion-virtual-scroll class="fixOverlapp" [items]="orders">
....
</ion-virtual-scroll>
mrSingh007 commented 4 years ago

Hi everyone this is my fix/workaround for this problem:

.fixOverlapp {
  height: 100% !important;
  overflow: auto;
}
.fixOverlapp >.virtual-item {
  position: initial !important;
  transform: initial !important;
}
<ion-virtual-scroll class="fixOverlapp" [items]="orders">
....
</ion-virtual-scroll>

It showing then duplicate/old data when Array is changed.

weathon commented 4 years ago

Is anyone fixed this? I tried to use img but it gives me another issue which it will show the wrong image until the new is loaded

mrSingh007 commented 4 years ago

Is anyone fixed this? I tried to use img but it gives me another issue which it will show the wrong image until the new is loaded

No. I tried also all work arounds, it always giving an issue. One finished then another started. So finally I am using for scroll https://material.angular.io/cdk/scrolling/overview

graphefruit commented 4 years ago

Sad there is still no feedback here, having the same issue

arcbus commented 4 years ago

We started seeing this issue as well, randomly, and may have narrowed down a culprit. It seems to be when the native keyboard opens up that our virtual scroll has either scrunched up a bunch of overlapping items if at the top of the virtual scroll, or a blank virtual scroll if it was already scrolled down a bit.

My guess is the viewport is getting relocated/resized, throwing off the virtual scroll's positioning math.

Hope this helps!

arcbus commented 4 years ago

Another update, came across this post where they talk about using:

window.dispatchEvent(new Event('resize'));

To trigger a redraw of the virtual scroll. This does work, plus i added onFocus events to keep track of when the keyboard comes up. But you don't always end up back where you started in the virtual scroll in terms of position. This i think can be mitigated by ensuring your scroll items are the same size as your approx height,

Hope this helps...

Update: Ensuring that my virtual scroll item ion-card was alwasy exactly the same height as approxItemHeight, including margins, makes the resize land me back where i was accurately most of the time. Like almost pixel perfect location. But sometimes, it will be like 15 items off, and this i can't figure out...

Hanzofm commented 3 years ago

Same problem here on ionic 5...

mrSingh007 commented 3 years ago

Guys, I don't think it's not gonna fix! This bug is there from decades. Stop making hope just use angular virtual scroll (https://material.angular.io/cdk/scrolling/overview)

Sampath-Lokuge commented 3 years ago

Yes, this is not working with a very simple use case too. i.e. no images, no dynamic size changes and etc. Just around 500 items only

"@ionic/angular": "5.3.3",

zalza13 commented 3 years ago

This workaround works for me:

style="min-height: 150px!important; max-height: 300px!important;"

<ion-virtual-scroll [items]="items" approxItemHeight="300px">
   <ion-card *virtualItem="let item; let itemBounds = bounds;" 
             style="min-height: 150px!important; max-height: 300px!important;">
         ...
   </ion-card>
</ion-virtual-scroll>
achakra21 commented 3 years ago

This workaround works for me:

style="min-height: 150px!important; max-height: 300px!important;"

<ion-virtual-scroll [items]="items" approxItemHeight="300px">
   <ion-card *virtualItem="let item; let itemBounds = bounds;" 
             style="min-height: 150px!important; max-height: 300px!important;">
         ...
   </ion-card>
</ion-virtual-scroll>

didn't worked for me ... It create strange spaces between the items

achakra21 commented 3 years ago

So far i found this solution which is working for me

window.dispatchEvent(new Event('resize'));

graphefruit commented 3 years ago

I've tried the resize event aswell, and it fixed about 80% of the time those issues. But with more dynamic heights even this didn't fix my issues.

My solution was to swap to: https://github.com/ericferreira1992/ag-virtual-scroll

I've tried multiple virtuell scroll plugins, and this was the only one which could handle dynamic heights and is very fast. Sometimes a whitespace appears but this is the lower tradeoff I accepted. Only one thing could be an issue: margin/paddings.

achakra21 commented 3 years ago

I've tried the resize event aswell, and it fixed about 80% of the time those issues. But with more dynamic heights even this didn't fix my issues.

My solution was to swap to: https://github.com/ericferreira1992/ag-virtual-scroll

I've tried multiple virtuell scroll plugins, and this was the only one which could handle dynamic heights and is very fast. Sometimes a whitespace appears but this is the lower tradeoff I accepted. Only one thing could be an issue: margin/paddings.

Thanks for mentioning another way of doing for special cases where the items have dynamic height , But in my case with my solution with dynamic height of items it will working without any issues ,but then i have to check more to see then i will check this plugin.

GosuMania commented 3 years ago

So far i found this solution which is working for me

window.dispatchEvent(new Event('resize'));

where you add this code? can you do a example please? i have the same problem, each item has a different height, and every 50-100 items happen that these overlap

liamdebeasi commented 2 years ago

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 https://github.com/ionic-team/ionic-framework/issues/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[bot] commented 2 years ago

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.