naver / egjs-infinitegrid

A module used to arrange card elements including content infinitely on a grid layout.
https://naver.github.io/egjs-infinitegrid/
MIT License
2.23k stars 95 forks source link

Scrolling doesn't resize images and keeps flickering white areas #443

Closed elementalTIMING closed 2 years ago

elementalTIMING commented 3 years ago

Description

Something is wrong with scrolling. At the beginning, the images are loaded and displayed normally. After 2-3 reloads (append), however, the new images are no longer resized. In addition, it flickers when scolling and a large area becomes white. Please see the video that shows the problem:

https://www.dropbox.com/s/lm576fiver9zj43/ngx-video1.mp4?dl=0

I used the templates from the description for the implementation and did not add any new functions. This was my first test with the scroller.

The implementation is based on Angular 13.

What could be the reason for this behaviour, or how can I get it to run properly?

daybrush commented 3 years ago

@elementalPRESS

Thanks for reporting. Do you have a demo address where I can test it?

elementalTIMING commented 3 years ago

@elementalPRESS

Thanks for reporting. Do you have a demo address where I can test it?

unfortunately there is no demo, because I wasn't able to get it to run on StackBlitz. But I guess it has also something to do with where I get the photos from. I collect my images from a server of mine which has some larger response times. So if the image is from a URL it seems to force the "crash". But this can also be a conicidence.

It's a real awesome gallery and I would love to use it for one of my projects!

daybrush commented 3 years ago

@elementalPRESS

If so, can you show the code corresponding to requestAppend?

elementalTIMING commented 3 years ago

@elementalPRESS

If so, can you show the code corresponding to requestAppend?

sure.

onRequestAppend(e) {
    this.isLoading = true;
    this.cdr.detectChanges();

    let key = e.groupKey?.replace('infinitegrid_', '') || 0;

    const nextGroupKey = (+key! || 0) + 1;

    if (this.loaded.includes(key)) return;
    this.loaded.push(key);

    e.wait();
    //e.currentTarget.appendPlaceholders(5, nextGroupKey);

    setTimeout(() => {
        e.ready();

        this.getItems(nextGroupKey, this.limit).subscribe((response: any[]) => {
            this.items = [...this.items].concat(response).filter(item => item.imageID);
        });

        this.cdr.detectChanges();
    }, 500);
}

getItems(nextGroupKey: number, count: number) {
    const nextItems = [];
    const nextKey = nextGroupKey * count;

    return this.photoService
        .getPhotos({
            eventID: 283,
            searchTags: this.searchTags,
            fromIndex: nextKey,
            limit: this.limit,
        })
        .pipe(
            tap((response: any) => {
                for (let i = 0; i < count; ++i) {
                    this.items.push({ groupKey: nextGroupKey, key: response[i]?.url });
                }

                this.isLoading = false;
            })
        );
}
daybrush commented 3 years ago

@elementalPRESS

If getPhotos is asynchronous, it seems better to apply the ready function inside.

this.getItems(nextGroupKey, this.limit).subscribe((response: any[]) => {
     e.ready();
     this.items = [...this.items].concat(response).filter(item => item.imageID);
});
daybrush commented 3 years ago

@elementalPRESS

Ohh. Since the key is used as a url, it can be a problem if there is an image with the same url.

elementalTIMING commented 3 years ago

@elementalPRESS

If getPhotos is asynchronous, it seems better to apply the ready function inside.

this.getItems(nextGroupKey, this.limit).subscribe((response: any[]) => {
     e.ready();
     this.items = [...this.items].concat(response).filter(item => item.imageID);
});

I tried but it doesn't solves the problem. If you like I can make it online. But it's my production system and can keep it online just for a very short time...

2nd: The url is unique and represents the key

daybrush commented 3 years ago

@elementalPRESS How about key:

this.items.push({ groupKey: nextGroupKey, key: nextKey + i });
elementalTIMING commented 3 years ago
nextKey + i

unfortunately, it doesn't solve it.

At the beginning everything looks brilliant, when I'm scrolling down. The problems start when I scroll up some pages and then scroll down again...

daybrush commented 3 years ago

@elementalPRESS

If so, can I see the template code using ngx-infinitegrid?

elementalTIMING commented 3 years ago

@elementalPRESS

If so, can I see the template code using ngx-infinitegrid?

<div class="pt-4 pb-3 mx-5">
    <search-tag [rink]="true" (onSearchSelected)="onSearchChanged($event)" (onClear)="onSearchChanged($event)"></search-tag>
</div>

<div NgxJustifiedInfiniteGrid class="image-container" [gap]="5" [items]="items" [align]="'justify'" [trackBy]="trackBy" [groupBy]="groupBy" [useLoading]="false" [usePlaceholder]="false" (requestAppend)="onRequestAppend($event)" *ngFor="let item of [0]; trackBy: trackBy" #ig>
    <ng-container *ngFor="let item of ig.visibleItems; trackBy: trackBy">
        <div class="item" *ngIf="item.type === ITEM_TYPE.NORMAL">
            <div class="thumbnail">
                <!-- <img [src]="'https://naver.github.io/egjs-infinitegrid/assets/image/' + ((item.data.key % 33) + 1) + '.jpg'" alt="egjs" data-grid-maintained-target="true" /> -->

                <img [src]="item.data.url" alt="egjs" data-grid-maintained-target="true" />
            </div>
            <div class="info">{{ item.data.imageID }}</div>
        </div>
        <!--
        <div class="loading" *ngIf="item.type === ITEM_TYPE.LOADING">Loading...</div>
        -->
        <div class="placeholder" *ngIf="item.type === ITEM_TYPE.VIRTUAL"></div>
    </ng-container>
</div>

<div class="lead text-center mx-5 my-3" *ngIf="isLoading">
    <i class="fa fa-spinner fa-pulse fa-lg fa-fw"></i>
    Loading...
</div>
elementalTIMING commented 3 years ago

@elementalPRESS If so, can I see the template code using ngx-infinitegrid?

I made it online for demonstrating the issue. Please see Demo

daybrush commented 3 years ago

The definition of items seems strange.

items have no groupKey and key. There are only the results of the response.


this.items = [...this.items].concat(response).filter(item => item.imageID);
elementalTIMING commented 3 years ago

The definition of items seems strange.

items have no groupKey and key. There are only the results of the response.

this.items = [...this.items].concat(response).filter(item => item.imageID);

There is a groupKey. The response comes from here:

getItems(nextGroupKey: number, count: number) {
    const nextItems = [];
    const nextKey = nextGroupKey * count;

    return this.photoService
        .getPhotos({
            eventID: 283,
            searchTags: this.searchTags,
            fromIndex: nextKey,
            limit: this.limit,
        })
        .pipe(
            tap((response: any) => {
                for (let i = 0; i < count; ++i) {
                    this.items.push({ groupKey: nextGroupKey, key: nextKey + i });
                }
            })
        );
}

But indeed the filter isn't used anymore, but dien't affect the result

daybrush commented 3 years ago

@elementalPRESS

The code below filters all { groupKey, key } objects.

// before
this.items.push({ groupKey: nextGroupKey, key: nextKey + i });

// after
this.items = [...this.items].concat(response).filter(item => item.imageID);

Try printing the item in groupBy or trackBy to the console.

An item containing properties such as url and filename in the response is returned.

elementalTIMING commented 3 years ago

Ok, I checked and there was no groupKey. I revised the code (see below), but unfortunately it doesn't solved the problem as you can see at the Demo-Link (some posts above) :-(

But the good news it: the problem looks now a bit different and happens already when scrolling down...

This is the updated code:

import { ChangeDetectorRef, Component, OnInit } from '@angular/core';
import { OnRequestAppend } from '@egjs/infinitegrid';
import { ITEM_TYPE } from '@egjs/infinitegrid';
import { Observable, tap, map } from 'rxjs';
import { PhotoService } from './photo.service';

@Component({
    selector: 'app-photo',
    templateUrl: './photo.component.html',
    styleUrls: ['./photo.component.scss'],
})
export class PhotoComponent implements OnInit {
    public ITEM_TYPE = ITEM_TYPE;
    public items: any[] = [];
    public items$: Observable<any[]>;

    private limit: number = 20;
    private loaded = [];
    private searchTags: string[] = [];

    /**
     *
     * @param photoService
     * @param cdr
     */
    constructor(private photoService: PhotoService, private cdr: ChangeDetectorRef) {}

    /**
     *
     */
    ngOnInit(): void {
        this.onSearchChanged([]);
    }

    /**
     *
     * @param nextGroupKey
     * @param count
     * @returns
     */
    getItems(nextGroupKey: number, count: number) {
        const nextItems = [];
        const nextKey = nextGroupKey * count;

        return this.photoService
            .getPhotos({
                eventID: 283,
                searchTags: this.searchTags,
                fromIndex: nextKey,
                limit: this.limit,
            })
            .pipe(
                map((response: any) => {
                    let result = [];

                    for (let i = 0; i < count; ++i) {
                        result.push(Object.assign({ groupKey: nextGroupKey, key: nextKey + i }, response[i]));
                    }

                    return result;
                })
            );
    }

    /**
     *
     * @param _
     * @param item
     * @returns
     */
    groupBy(_: any, item: any) {
        return item.groupKey;
    }

    /**
     *
     * @param _
     * @param item
     * @returns
     */
    trackBy(_: any, item: any) {
        return item.key;
    }

    /**
     *
     * @param e
     * @returns
     */
    onRequestAppend(e) {
        let key = e.groupKey?.toString().replace('infinitegrid_', '') || 0;

        const nextGroupKey = (+key! || 0) + 1;

        if (this.loaded.includes(key)) return;
        this.loaded.push(key);

        e.wait();

        this.cdr.detectChanges();

        setTimeout(() => {
            this.getItems(nextGroupKey, this.limit).subscribe((response: any[]) => {
                e.ready();
                this.items = [...this.items].concat(response);
                this.cdr.detectChanges();
            });
        }, 500);
    }

    /**
     *
     * @param value
     */
    onSearchChanged(value) {
        this.searchTags = value;
        this.loaded = [0];
        this.cdr.detectChanges();

        this.getItems(0, this.limit).subscribe(response => {
            this.items = [...this.items].concat(response);
            this.cdr.detectChanges();
        });
    }
}
elementalTIMING commented 3 years ago

@daybrush

Maybe I found an affecting aspect: The problem seems to be the LIMIT in this.getItems(nextGroupKey, limit). The bug (or misplaced behaviour) occurs when the LIMIT value is pretty small. My default value is 20 and you saw how it looks. But if I set the value e.g. to 100 the behaviour seems to be gone and everything looks fine.

Unfortunately, I would need smaller LIMIT values for performance and "user response" reasons. So I hope that this info can help you to find a fix.

P.S.: after some more tests the problem has not been disappeared but seems to be reduced

daybrush commented 3 years ago

@elementalPRESS

Thank you. I found the cause. I will try to find a way to solve this cause.

elementalTIMING commented 3 years ago

@elementalPRESS

Thank you. I found the cause. I will try to find a way to solve this cause.

Awesome. Thx so much!

daybrush commented 3 years ago

@elementalPRESS

Hello. I released the beta version.(@egjs/ngx-infinitegrid@4.1.2-beta.0)

I think that the cause may have been a timing problem due to change detection. let me know if it's resolved.

elementalTIMING commented 3 years ago

@daybrush

That sounds so great and I'm exciting to test it. But how can I install it or where can I get it from? The package name @egjs/ngx-infinitegrid@4.1.2-beta.0 is still unknownnpm and a direct download gives me the wrong version, too.

daybrush commented 3 years ago

@elementalPRESS

Will it still be installed?

It was published on npm. https://www.npmjs.com/package/@egjs/ngx-infinitegrid/v/4.1.2-beta.0

elementalTIMING commented 3 years ago

@daybrush

It was published on npm. https://www.npmjs.com/package/@egjs/ngx-infinitegrid/v/4.1.2-beta.0

strange, after a second install retry it worked and I have installed.

But (and I'm so sorry to say) - the problem still exists :-( Please see here the updated DEMO - you need to scroll a bit down and up

daybrush commented 3 years ago

@elementalPRESS

There is an incomprehensible phenomenon.

What version of angular is it? Would you like to update the minor version of angular?

elementalTIMING commented 3 years ago

@elementalPRESS

There is an incomprehensible phenomenon.

What version of angular is it? Would you like to update the minor version of angular?

I'm running one the latest major release of Angular. It's 13.0.0.

"dependencies": {
    "@angular/animations": "^13.0.0",
    "@angular/cdk": "^13.0.0",
    "@angular/common": "^13.0.0",
    "@angular/compiler": "^13.0.0",
    "@angular/core": "^13.0.0",
    "@angular/elements": "^13.0.0",
    "@angular/forms": "^13.0.0",
    "@angular/google-maps": "^13.0.0",
    "@angular/platform-browser": "^13.0.0",
    "@angular/platform-browser-dynamic": "^13.0.0",
    "@angular/platform-server": "^13.0.0",
    "@angular/router": "^13.0.0",
    "@angular/service-worker": "^13.0.0",
    "@egjs/ngx-infinitegrid": "^4.1.2-beta.0",
    "@ng-select/ng-option-highlight": "^0.0.7",
    "@ng-select/ng-select": "^8.1.1",
    "@ngneat/until-destroy": "^8.1.4",
    "@nguniversal/express-engine": "^13.0.0",
    "angular-responsive-carousel": "^2.1.2",
    "bootstrap": "4.6.0",
    "compression": "^1.7.4",
    "crypto-es": "^1.2.7",
    "decimal.js": "^10.3.1",
    "express": "^4.15.2",
    "font-awesome": "^4.7.0",
    "hammerjs": "^2.0.8",
    "highcharts": "^9.2.2",
    "highcharts-angular": "^2.10.0",
    "md5-typescript": "^1.0.5",
    "memory-cache": "^0.2.0",
    "ng2-adsense": "^10.0.1",
    "ngx-aside": "^1.4.2",
    "ngx-bootstrap": "^7.1.0",
    "ngx-paypal": "^8.0.0",
    "ngx-ui-switch": "^12.0.1",
    "pluralize": "^8.0.0",
    "regression": "^2.0.1",
    "replace-in-file": "^6.2.0",
    "rxjs": "^7.4.0",
    "tslib": "^2.3.1",
    "zone.js": "~0.11.4"
},
"devDependencies": {
    "@angular-devkit/build-angular": "^13.0.1",
    "@angular/cli": "^13.0.1",
    "@angular/compiler-cli": "^13.0.0",
    "@angular/language-service": "^13.0.0",
    "@nguniversal/builders": "^13.0.0",
    "@types/express": "^4.17.13",
    "@types/jasmine": "^3.9.1",
    "@types/jasminewd2": "^2.0.10",
    "@types/node": "^16.10.3",
    "bufferutil": "^4.0.4",
    "codelyzer": "^6.0.2",
    "domino": "^2.1.6",
    "jasmine-core": "^3.9.0",
    "jasmine-spec-reporter": "^7.0.0",
    "karma-chrome-launcher": "~3.1.0",
    "karma-coverage-istanbul-reporter": "~3.0.2",
    "karma-jasmine": "~4.0.0",
    "karma-jasmine-html-reporter": "^1.7.0",
    "ng-packagr": "^13.0.0",
    "postcss": "^8.3.9",
    "protractor": "~7.0.0",
    "ts-node": "^10.3.0",
    "tslint": "~6.1.0",
    "utf-8-validate": "^5.0.6",
    "webpack-bundle-analyzer": "^4.5.0",
    "ws": "^8.2.3",
    "xhr2": "^0.2.1",
    "xmlhttprequest": "^1.8.0"
}
daybrush commented 3 years ago

@elementalPRESS

Try @egjs/ngx-infinitegrid@4.1.2-beta.1 version.

Unfortunately, I haven't been able to find a perfect solution.

However, a defense code was added to prevent the following phenomenon.

daybrush commented 3 years ago

I tested it with version 12 and 13.0.2, but it doesn't reproduce.

If the opportunity arises, consider updating the framework as well.

elementalTIMING commented 3 years ago

I tested it with version 12 and 13.0.2, but it doesn't reproduce.

If the opportunity arises, consider updating the framework as well.

@daybrush unfortunately I didn't hat time to test it today. But a bit later in day I will do, and I'm still excited to see it work. I'll let you know!

elementalTIMING commented 3 years ago

I tested it with version 12 and 13.0.2, but it doesn't reproduce.

If the opportunity arises, consider updating the framework as well.

@daybrush Great job! I've tested a bit and it works now. Yiehaw! Thx so much for your support!

Still, two little questions:

  1. is there a way to specify the image size (min or max of the image with/height or the number of thumbs per row) of the thumbs?
  2. is there a way to limit the gallery to a number of rows? For example to say just print 2 rows with images

Greets, Lars

daybrush commented 3 years ago

@elementalPRESS

  1. Use sizeRange (default: [0, Infinity])[sizeRange]="[100, 500]"
  2. Use rowRange (default: 0) [rowRange]="2"

Also if you thought of columnRange then use columnRange (default: [1, 8]) [columnRange]="[1, 8]"

elementalTIMING commented 3 years ago

@elementalPRESS

  1. Use sizeRange (default: [0, Infinity])[sizeRange]="[100, 500]"
  2. Use rowRange (default: 0) [rowRange]="2"

Also if you thought of columnRange then use columnRange (default: [1, 8]) [columnRange]="[1, 8]"

Thx a lot!

stale[bot] commented 2 years ago

This issue/PR has been automatically marked as stale because it has not had any update (including commits, comments, labels, milestones, etc) for 30 days. It will be closed automatically if no further update occurs in 7 day. Thank you for your contributions!

한글 이 이슈/PR은 commits, comments, labels, milestones 등 30일간 활동이 없어 자동으로 stale 상태로 전환되었습니다. 이후 7일간 활동이 없다면 자동으로 닫힐 예정입니다. 프로젝트 개선에 기여해주셔서 감사합니다.

daybrush commented 2 years ago

@elementalPRESS

infinitegrid's new version is released. Try it again. Thank you :)