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.03k stars 13.51k forks source link

RC4 ion-img doesn't correctly work with virtualScroll #9660

Closed pavimus closed 5 years ago

pavimus commented 7 years ago

Ionic version: (check one with "x") [ ] 1.x [x ] 2.x

I'm submitting a ... (check one with "x") [x ] bug report [ ] feature request [ ] support request => Please do not submit support requests here, use one of these channels: https://forum.ionicframework.com/ or http://ionicworldwide.herokuapp.com/

Current behavior: When you use virtualScroll and ion-img, after scrolling down (when buffer ends) ion-img stops loading images

_034

Please look at code inspector:

2016-12-16 10-23-17

url http://base247.lin2/thumbs/c6/files_56_222bf7a3335ce7c20553f0f374eb0266b871af41.jpg_mobileListImage_1412346660_3.jpg

is correct (for my local machine) and image is available.

Expected behavior: ion-img must load images

Steps to reproduce:

Related code:

<ion-content>
    <ion-refresher (ionRefresh)="dataService.refresh($event)">
    <ion-refresher-content
        pullingText="Потяните для обновления"
        refreshingText="Загрузка">
    </ion-refresher-content>
</ion-refresher>
    <ion-list [hidden]="(works|filterWorks:filter.price.mode:filter.price.from:filter.price.to:filter.price.currency_id:filter.location.mode:filter.added.mode:sort.orderBy:commonData).length==0" [virtualScroll]="works|filterWorks:filter.price.mode:filter.price.from:filter.price.to:filter.price.currency_id:filter.location.mode:filter.added.mode:sort.orderBy:commonData" approxItemHeight="80px" >
    <ion-item-sliding *virtualItem="let work">
        <button ion-item (click)="goWork(work)" [ngClass]="{'work-list-item-selected':selectedWorks.works[work.id]}">
            <ion-thumbnail item-left>
                                    <ion-img [src]="work.image|defaultImage"></ion-img>
                            </ion-thumbnail>
                        <h3>{{commonData.authors[work.author_id]?.title}}</h3>
            <h5>{{work.title}}</h5>
            <p class="work-list-price">{{work.price | price : work.price_currency_id : commonData.currencyExchangeRate : true}}</p>
            <p>{{work.year ? work.year+',':''}} {{work.techniques | techniques : commonData.techniques : commonData.mainLanguageId}}</p>
        </button>
        <ion-item-options side="left">
            <button ion-button icon-only color="primary" (click)="selectedWorks.toggle(work)">
                <ion-icon name="{{selectedWorks.works[work.id] ? 'checkmark-circle':'radio-button-off'}}"></ion-icon>
            </button>
        </ion-item-options>
        <ion-item-options side="right">
            <button ion-button icon-left color="primary" (click)="sendWorkByEmail(work)">
                <ion-icon name="mail"></ion-icon>
                Email
            </button>
        </ion-item-options>
    </ion-item-sliding>
</ion-list>
</ion-content>

Other information:

Bug appeared in RC4, in RC3 all was OK.

Ionic info: (run ionic info from a terminal/cmd prompt and paste output below):

Cordova CLI: 6.3.1 
Ionic Framework Version: 2.0.0-rc.4
Ionic CLI Version: 2.1.13
Ionic App Lib Version: 2.1.7
Ionic App Scripts Version: 0.0.45
ios-deploy version: Not installed
ios-sim version: Not installed
OS: Linux 4.4
Node Version: v4.2.6
Xcode version: Not installed
agrt56 commented 7 years ago

I have the same issue with my app since updating to rc4.

naveedahmed1 commented 7 years ago

I am facing the similar issue with RC 4, in RC 3 it was working fine.

tomaszczechowski commented 7 years ago

Same issue on my end

sajTempler commented 7 years ago

try using just "img" instead of "ion-img". Worked for me.

filipsuk commented 7 years ago

"ion-img" stopped working for me in rc4, too (even without virtual scroll). Could replacing with "img" cause some performance issues in longer lists?

naveedahmed1 commented 7 years ago

@sajTempler yes it works but we loose the performance benefits such lazy loading that comes with ion-img

sajTempler commented 7 years ago

You can still use ng2 lazy loader. There are two or more. I'm using this one https://github.com/tjoskar/ng2-lazyload-image

flolovebit commented 7 years ago

Ciao folks, ion-img work fine only if the tag is in the visible content., every List, segment an so on ....doesn't work . My temporary fix:

  1. open node_modules/ ionic-angular/component/img/img.js
  2. change the function ngAferContentInit with:

    Img.prototype.ngAfterContentInit = function () { var _this = this; this._img = this._elementRef.nativeElement.firstChild;

    if(!this._img.hasAttribute("src")){
    
        this._img.src=this._elementRef.nativeElement.getAttribute("src");
    
    }
    
    this._unreg = this._platform.addEventListener(this._img, 'load', function () {
        _this._hasLoaded = true;
        _this.update();
    }, { passive: true });

    };

I hope in quickly fix by Ionic team:)

jgw96 commented 7 years ago

Hello all! Thanks for using Ionic! Could someone post a repo that i can use to reproduce this issue?

naveedahmed1 commented 7 years ago

@jgw96 I emailed you a sample

GeeKanJi commented 7 years ago

Do you still have the problem? The "ion-img" does not work as expected on my app, but I can not find an explanation for the moment

pantonis commented 7 years ago

Same here. Was this fixed?

GeeKanJi commented 7 years ago

@naveedahmed1 Could you post your sample here? I'm not sure we're talking about the same problem

fastf0rward commented 7 years ago

We also ran into this issue with our non-virtualScroll grid. We reproduced the issue in a plunker. Seems to work fine in a finite (non-virtualScroll) list. Whenever we add ion-infinite-scroll to the list the ion-images show issues with loading and unloading.

So 'normal' lists seem to work just fine. Lists with virtualScroll or infiniteScroll seem to have issues.

WholeEggz commented 7 years ago

RC5 - same issue

fastf0rward commented 7 years ago

Well apparently ion-img should only be used in conjunction with virtualScroll: https://github.com/driftyco/ionic/commit/a5bbbd55cbd0e256614b6a1062aabbab4b1f21bd

GeeKanJi commented 7 years ago

I have exactly the same behavior as @pavimus

After the virtual scroll buffer, image does not appear, css class is "img-unloaded" (see screenshot below)

ionimg

with ionic 2 final. Please, could you communicate on this problem?

JustasKuizinas commented 7 years ago

I have this bug also in latest ionic

fastf0rward commented 7 years ago

As we really needed this to work in our scrollable grid of images (ion-grid, without virtual scroll), we have temporarily replaced the <project_root>/node_modules/ionic-angular/components/img/img.js file in our project with this gist.

After some digging around it appears that the top and bottom of an ion-img are not relative to the absolute top of the container it is in, causing the wrong images to be loaded/unloaded. See lines 231 and 242 of the gist for the workaround.

We also tried extending ion-img, but without much success. And building a (temporary) custom version of the Ionic dist files and using that as a local dependency also appears to be more complicated than we thought. So while the Ionic team works on this, we will be using this workaround.

JustasKuizinas commented 7 years ago

@sajTempler Is it possible to load images only when scroll stopped with that plugin?

ventr1x commented 7 years ago

This bug and some more (images not responsive and only 5x5 on chrome/android, images not loading on first start before any scrolling..) broke a production app which heavily relies on showing articles with lazy loaded images. Nice that we get no documentation whatsoever on such changes. Not that this is anything news.

JustasKuizinas commented 7 years ago

@ventr1x I also wonder what devs are doing because this critical bug is still open from december 16

ventr1x commented 7 years ago

It's just plain broken (it never really worked, in beta it wasn't even mentioned in any doc).

I switched to ng2-lazyload-image and got it working with rc6 like that (so some might not go through the same non documented things..):

Component

import { Component, ViewChild } from '@angular/core';
import { Content } from 'ionic-angular';

@Component({
  templateUrl: 'image-modal.html'
})
export class ImageModal {
  @ViewChild(Content) content: Content;

  constructor() {
  }

}

View <img [src]="img" [lazyLoad]="img" [scrollObservable]="content.ionScroll" />

JustasKuizinas commented 7 years ago

@ventr1x Is it possible to load images only when user stopped scrolling with this plugin?

JustasKuizinas commented 7 years ago

@jgw96 any status update?

rodrigoreal commented 7 years ago

Same problem here.. If i put the ion-img inside a segment it won't load. http://plnkr.co/edit/Zb2EQQVCFgdyZ1CRfPsW?p=preview

shprink commented 7 years ago

can easily reproduce with Ionic 3.1.1 here https://github.com/shprink/ionic-withwebworker-vs-withoutwebworker

naveedahmed1 commented 7 years ago

Though this isn't related to the issue, @shprink have you found any way to run complete ionic app from webworker?

shprink commented 7 years ago

Have not tried anything like this yet @naveedahmed1

naveedahmed1 commented 7 years ago

Ok, the link you shared had webworker in url so I thought you might have tried it. I just checked you are using service workers. But unfortunately service workers aren't supported on device whereas web workers work on device as well.

shprink commented 7 years ago

ion-img within a virtual scroll is supposed to load the img in a web worker that's why it is in the title.

chandanch commented 7 years ago

I replaced ion-img with img and it works fine within the virtual scroll use img instead of ion-img

frankpapenmeier commented 7 years ago

The doc says that one should use ion-img for performance reasons, see: https://ionicframework.com/docs/api/components/virtual-scroll/VirtualScroll/

Therefore, going for imginstead of ion-img cannot be a permanent solution and I hope that this long standing bug will be fixed soon.

@manucorporat Any progress on this issue, yet?

DEG-7 commented 7 years ago

+1

DavidWiesner commented 7 years ago

I also had this issue. The bug is the calculation of the img::top and img::bottom in ion-img. This will be used to set the flag canRequest and canRender in Content::updateImgs for ion-img while scrolling. My first workaround was to extend ion-img for virtual lists.

import {Component, ElementRef, Renderer, Optional} from "@angular/core";
import {Img, Platform, DomController, Content} from "ionic-angular";

@Component({
  selector: 'virtual-ion-img',
  template: '<img>'
})
export class VirtualIonImg extends Img {
  constructor(private __elementRef: ElementRef,
              __renderer: Renderer,
              __plt: Platform,
              @Optional() private __content: Content,
              __dom: DomController) {
    super(__elementRef, __renderer, __plt, __content, __dom);
  }

  get top(){
    this._rect = (<HTMLElement>this.__elementRef.nativeElement).getBoundingClientRect();
    return this._rect.top + this.__content.scrollTop - this.__content._cTop;
  }
  get bottom(){
    this._rect = (<HTMLElement>this.__elementRef.nativeElement).getBoundingClientRect();
    return this._rect.bottom + this.__content.scrollTop - this.__content._cTop;
  }
}

Another option is to fix the calculation in Content::imgsUpdate by override this function in your page.

import {updateImgs} from "ionic-angular/components/content/content";
import {Component, ViewChild} from "@angular/core";
import {Content} from "ionic-angular";

@Component({
  templateUrl: 'virtual-list.html',
})
export class VirtualListPage {
  @ViewChild(Content) _content: Content;
  ngAfterViewInit(){
    if(this._content) {
      this._content.imgsUpdate = () => {
        if (this._content._scroll.initialized && this._content._imgs.length && this._content.isImgsUpdatable()) {
          // reset cached bounds
          this._content._imgs.forEach((img:Img)=>img._rect = null);
          // use global position to calculate if an img is in the viewable area
          updateImgs(this._content._imgs, this._content._cTop * -1, this._content.contentHeight, this._content.directionY, 1400, 400);
        }
      };
    }
  }
}
alainib commented 7 years ago

DavidWiesner thanks for the answer i'm new in ionic and angular ( testing ionic vs react native in fact )

in which file I have to past this please ? in app.components.ts ?

kabus202 commented 7 years ago

@DavidWiesner, Thanks for the solution. Can you provide us an example? I'm stuck with the updateImgs() function

DavidWiesner commented 7 years ago

@alainib

for the first workaround

you have to create a new file e.g. src/components/virtual-ion-img.ts this new component than has to register in src/app/app.module.ts

import {VirtualIonImg} from "../components/virtual-ion-img";
...
@NgModule({
    declarations: [
        //... all other pages and components,
        VirtualIonImg
    ],
//... all other stuff e.g.: imports, providers
    entryComponents: [
        //... other pages and components,
        VirtualIonImg
    ]
})

Than you can use the tag <virtual-ion-img [src]="url"></virtual-ion-img> in place of <ion-img> like described in http://ionicframework.com/docs/api/components/virtual-scroll/VirtualScroll/#images-within-virtual-scroll

for the second workaround

Just edit the typescript page file which has an virtual scroll inside and add the imports, the ViewChild stuff and the method ngAfterViewInit.

@kabus202 I edited my comment and add the missing import statement. The function updateImgs is defined in the ionic angular content package and has to be imported.

bparvizi commented 7 years ago

@DavidWiesner The first workaround works better, but I still see stale images in place of images that do not load.

nicolus commented 7 years ago

@DavidWiesner : Thanks a lot ! Your first workaround seems to be working perfectly for me (in a very simple example I built to convince myself that I was not crazy and that VirtualScroll was indeed broken), I'll try it in a real life app now.

Could you maybe make a pull request to fix the actual Ionic Img component ?

alainib commented 7 years ago

@DavidWiesner thanks for the explanation. I still have trouble sorry (i'm new and trying react/ionic/nativscript in same time)

this is what i do :

app.module.ts

import { NgModule, ErrorHandler } from '@angular/core';
import { IonicApp, IonicModule, IonicErrorHandler } from 'ionic-angular';
import { MyApp } from './app.component';
import { AboutPage } from '../pages/about/about';
import { ContactPage } from '../pages/contact/contact';
import { HomePage } from '../pages/home/home';
import { TabsPage } from '../pages/tabs/tabs';
import {VirtualIonImg} from "../components/virtual-ion-img";

@NgModule({
  declarations: [
    MyApp,      AboutPage,      ContactPage,        HomePage,       TabsPage
    VirtualIonImg
  ],
  imports: [
    IonicModule.forRoot(MyApp)
  ],
  bootstrap: [IonicApp],
  entryComponents: [
    MyApp,  AboutPage,      ContactPage,    HomePage,TabsPage
    VirtualIonImg
  ],
  providers: [{provide: ErrorHandler, useClass: IonicErrorHandler}]
})
export class AppModule {}

virtual-ion-img.ts

import {Component, ElementRef, Renderer, Optional} from "@angular/core";
import {Img, Platform, DomController, Content} from "ionic-angular";

@Component({
  selector: 'virtual-ion-img',
  template: '<img>'
})
export class VirtualIonImg extends Img {
  constructor(private __elementRef: ElementRef,
              __renderer: Renderer,
              __plt: Platform,
              @Optional() private __content: Content,
              __dom: DomController) {
    super(__elementRef, __renderer, __plt, __content, __dom);
  }

  get top(){
    this._rect = (<HTMLElement>this.__elementRef.nativeElement).getBoundingClientRect();
    return this._rect.top + this.__content.scrollTop - this.__content._cTop;
  }
  get bottom(){
    this._rect = (<HTMLElement>this.__elementRef.nativeElement).getBoundingClientRect();
    return this._rect.bottom + this.__content.scrollTop - this.__content._cTop;
  }
}

my view

<ion-content padding>
  <h2>Ionic List items</h2>
    <ion-list [virtualScroll]="items">
        <ion-item *virtualItem="let item">
            {{ item.i }}
            <virtual-ion-img [src]="item.src"
                 alt="item.image"  height="200" width="300" >
            </virtual-ion-img>
        </ion-item>
    </ion-list>

  FIN
</ion-content>

i have this error :

[WARN] Error occurred during command execution from a CLI plugin (@ionic/cli-plugin-cordova). Your plugins may be out of
       date.
Error: Template parse errors:
Can't bind to 'src' since it isn't a known property of 'virtual-ion-img'.
1. If 'virtual-ion-img' is an Angular component and it has 'src' input, then verify that it is part of this module.
2. If 'virtual-ion-img' is a Web Component then add "CUSTOM_ELEMENTS_SCHEMA" to the '@NgModule.schemas' of this componen
t to suppress this message.
 ("
        {{ item.i }}

        <virtual-ion-img [ERROR ->][src]="item.src"
             alt="item.image"  height="200" width="300" >
                </virtual-ion-img>
"): HomePage@16:25

what i'm missing please ?

kabus202 commented 7 years ago

@alainib Maybe you need to use [src]="item.src" instead of src="item.src

alainib commented 7 years ago

kabus202 . thanks for help. the error i get is with [src] , i tryed both but i paste wrong html code her ;)

with 'src' i get this error

typescript: ..._amap/react-native-compare-ionic2-master/ionic/src/components/virtual-ion-img.ts, line: 14
Supplied parameters do not match any signature of call target.

            __dom: DomController) {
  super(__elementRef, __renderer, __plt, __content, __dom);

ngc failed: Failed to transpile TypeScript
DavidWiesner commented 7 years ago

@alainib which version of ionic do you use? In 3.2 the constructor of Img was changed. Before Ionic 3.2 you have to add NgZone as another parameter to the constructor.

VirtualIonImg before Ionic 3.2

import {Component, ElementRef, Renderer, Optional, NgZone} from "@angular/core";
import {Img, Platform, DomController, Content} from "ionic-angular";

@Component({
  selector: 'virtual-ion-img',
  template: '<img>'
})
export class VirtualIonImg extends Img {
  constructor(private __elementRef: ElementRef,
              __renderer: Renderer,
              __plt: Platform, 
              __zone: NgZone,
              @Optional() private __content: Content,
              __dom: DomController) {
    super(__elementRef, __renderer, __plt, __zone, __content, __dom);
  }

  get top(){
    this._rect = (<HTMLElement>this.__elementRef.nativeElement).getBoundingClientRect();
    return this._rect.top + this.__content.scrollTop - this.__content._cTop;
  }
  get bottom(){
    this._rect = (<HTMLElement>this.__elementRef.nativeElement).getBoundingClientRect();
    return this._rect.bottom + this.__content.scrollTop - this.__content._cTop;
  }
}
WangRongda commented 7 years ago

It's been six months, why hasn't it been fixed?

janpio commented 7 years ago

@WangRongda Did you submit a PR that should have been accepted that fixes this?

WangRongda commented 7 years ago

@janpio Magically, it display normally right now. I has been in trouble several days becouse the ion-img can't display normally.Sorry.

dspachos commented 7 years ago

Still have the issue, in a list with ~200 items with remote images, the images either not loading or are scrambled (images from previous items are on a current item). Any idea how to fix this? I tried the solution with virtual-ion-img and didn't worked

JustasKuizinas commented 7 years ago

@dspachos as I understand ionic team mentioned that fixing virtual scroll bugs is not a priority for them at the moment, try to use infinite-scroll instead.

vahidvdn commented 7 years ago

@JustasKuizinas But infinite-scroll is not the solution. At the end, all items are in the DOM. So there are lots of memory issues especially in iOS.

There was similar problems in ionic1.x. I thought all of them is fixed in ionic2 or 3. Unfortunately there is no solution.