ProgressNS / nativescript-ui-feedback

This repository is used for customer feedback regarding Telerik UI for NativeScript. The issues system here is used by customers who want to submit their feature requests or vote for existing ones.
Other
115 stars 21 forks source link

RadListView jumping / flickering on load on demand #115

Closed ginev closed 6 years ago

ginev commented 7 years ago

From @ignaciolarranaga on January 30, 2017 11:55

Tell us about the problem

If you carefully pay attention to the scrolling there is a jump / flick when the page load on demand is loaded.

Which platform(s) does your issue occur on?

iOS

Please provide the following version numbers that your issue occurs with:

Please tell us how to recreate the issue in as much detail as possible.

Just scroll with on demand items on iOS

Copied from original issue: telerik/nativescript-ui-samples#164

ginev commented 7 years ago

From @imaginationcoder on February 4, 2017 8:51

+1

ginev commented 7 years ago

Hello @ignaciolarranaga,

Am I understanding it correctly: you notice a small jump while a scroll animation is running and a chunk of data is loaded into the list simultaneously?

ginev commented 7 years ago

From @ignaciolarranaga on February 7, 2017 17:9

In my case I have a list with images, while scrolling I'm like seeing 3 of them at a time and suddenly the scroll moves and the one that was on the scroll is out of the view, like a sudden jump on the scroll and a little blink (I think that associated to the same).

I think I'll have my app published on a few days, I'll ping back once available for reference if needed.

I have to double check, but I think that happens on the first on demand load (i.e. not sure if on every load or the first one, if I remember correctly it is on the first load).

ginev commented 7 years ago

From @imaginationcoder on February 7, 2017 19:56

For me it happens every time loadondemand function executed and my listview also showing images along with other data

ginev commented 7 years ago

@ignaciolarranaga, @imaginationcoder, thanks for the details. I guess with a project to reproduce the issue would be the best way for us to investigate what goes wrong.

ginev commented 7 years ago

From @KoushikJit on February 18, 2017 15:53

I got the same issue. I have a list where each list item has an image and some 2/3 lines of text under the image. If I scroll down no issue. But after I have scrolled down a bit, if I start scrolling upwards I see the flickering occurs. I think it's because when the list items are out of scope they are "unloaded" and when you scroll up they start to load again. Now the text is loaded fast but the image takes time to load. So when you scroll up to a list item the text loads first and then the image pops in (on top of the text , inside the list item) and the list items under it is pushed downwards a little bit to cause the flicker.

I wonder if there is any option so that we can stop the list item contents from being unloaded when they go out of scope due to scrolling down. So that when we scroll up the items should be there as they were before. Also loading the images again due to scroll up will require the same image to be downloaded twice.

ginev commented 7 years ago

From @ignaciolarranaga on February 19, 2017 12:46

Hey @ginev here the examples:

Slowly scroll down from the top and you will see suddenly the image change.

ginev commented 7 years ago

From @StJoker on March 1, 2017 18:47

Try add itemHeight <lv:ListViewLinearLayout scrollDirection="Vertical" itemHeight="350" />

ginev commented 7 years ago

From @ignaciolarranaga on March 1, 2017 18:51

The height is already fixed actually, here is the code:

I tried dynamic sizing initially, but it does not behave correctly so I resign to fixed one.

ginev commented 7 years ago

Hello @ignaciolarranaga,

First, sorry for the late reply.

I cannot say what the reason for the glitchy behavior is. Can you paste the code that you have in the onLoadMoreItemsRequested event handler? It seems to me that the list control is entirely reset as soon as the user requests more data using the load-on-demand UX. This should not be the case.

Also, please note that from now one issues related to UI for NativeScript should be submittet in the dedicated repository: https://github.com/telerik/nativescript-ui-feedback.

ginev commented 7 years ago

From @ignaciolarranaga on March 6, 2017 13:5

Hey, no worries, here is the code:

onLoadMoreItemsRequested(args: ListViewEventData) {
        let listView: RadListView = args.object;
        this.getNextPage((page: Page<SearchResult>) => {
            if (page.last) {
                //console.log("It was the last page");
                listView.loadOnDemandMode = ListViewLoadOnDemandMode[ListViewLoadOnDemandMode.None];
            }

            listView.notifyLoadOnDemandFinished();
        });
    }

private getNextPage(callback?: (page: Page<SearchResult>) => void) {
        let nextPage = this.currentPage != null ? this.currentPage + 1 : 0;

        //console.log("Loading next page: " + nextPage);
        this.isLoading = true;
        this._searchService.findAll(nextPage)
            .subscribe(
                page => {
                    if (page.first) {
                        this.properties = new ObservableArray<SearchResult>(page.content);
                    } else {
                        this.properties.push(page.content);
                    }
                    //console.log("Data received: " + JSON.stringify(page.content));

                    this.currentPage = page.number;
                    this.hasMorePages = !page.last;
                    //console.log("Page loaded: " + this.currentPage);

                    this.isLoading = false;
                    this.dataLoaded = true;

                    if (callback) {
                        callback(page);
                    }
                },
                error => this._errorService.handleCommunicationError(error, () => { this.ngOnInit(); })
            );
    }

Are you guys going to move the issues from repo ?

ginev commented 7 years ago

@ignaciolarranaga, thanks for the snippet. Yes, we would like to redirect all NS UI related issues to the github.com/telerik/nativescript-ui-feedback repository as a part of our initiative to speed up issue processing. We also plan to close the issues section here so, please, open any forthcoming issues in the dedicated repository.

Can you also show me the Pull-to-Refresh logic. I think it might be the one causing problems?

ginev commented 7 years ago

From @ignaciolarranaga on March 8, 2017 20:10

Sure, here is the entire component actually, may be better:

import * as application from "application";
import * as platformModule from "platform";

import { Subscription } from "rxjs/Rx";
import { Component, ViewChild, ElementRef, OnDestroy, ChangeDetectorRef } from "@angular/core";
import { Router } from "@angular/router";
import { RadListView, ListViewEventData, ListViewLoadOnDemandMode } from "nativescript-telerik-ui/listview";
import { RadListViewComponent } from "nativescript-telerik-ui/listview/angular";
import { ObservableArray } from "data/observable-array";

import { AppConfig } from "../../../app.config";
import { Page, SearchResult } from "../../../shared/dtos";
import { ErrorService, DialogService, SearchService } from "../../../shared/services";
import { GlobalEventsUtilities, GlobalEvent, navigationHandler } from "../../../shared/util";
import { BaseComponent } from "../../base.component";

@Component({
    moduleId: module.id,
    selector: "all-properties",
    templateUrl: "all-properties.html",
    styleUrls: [ "all-properties-common.css" ]
})
export class AllPropertiesComponent extends BaseComponent implements OnDestroy {

    // Both used by the navigationHandler to recover the scroll on Android
    currentScrollPosition: number = null;
    @ViewChild("listView") listViewComponent: RadListViewComponent;

    private _globalEventsSubscription: Subscription;

    currentPage: number;
    hasMorePages: boolean;

    properties: ObservableArray<SearchResult>;

    constructor(
        private _changeDetectorRef: ChangeDetectorRef,
        private _router: Router,
        private _errorService: ErrorService,
        private _dialogService: DialogService,
        private _globalEventsUtilities: GlobalEventsUtilities,
        private _searchService: SearchService) {
        super();
    }

    ngOnInit() {
        this.properties = new ObservableArray<SearchResult>();

        this.getNextPage((page: Page<SearchResult>) => {
            this._changeDetectorRef.detectChanges();
        });

        // Handling the scroll on Android
        if (platformModule.isAndroid) {
            // The navigation happens on the PropertyComponent but the control is here
            this._router.events.subscribe(navigationHandler.bind(this, "all-properties"));
        }

        // Subscribing for refresh on search filter changes
        this._globalEventsSubscription = this._globalEventsUtilities.globalEventEmitter.subscribe((e: GlobalEvent) => {
            switch (e.event) {
                case GlobalEvent.SEARCH_FILTERS_CHANGED:
                    //console.log("Refreshing the properties after search filters changed!");
                    this.reset((page: Page<SearchResult>) => {
                        this._changeDetectorRef.detectChanges();
                    });
                    break;
                case GlobalEvent.PROPERTY_FAVORITE_ADDED:
                    this.properties.forEach((p: SearchResult) => {
                        if (p.id == e.data.id) {
                            p.favorite = true;
                        }
                    });
                    break;
                case GlobalEvent.PROPERTY_FAVORITE_REMOVED:
                    this.properties.forEach((p: SearchResult) => {
                        if (p.id == e.data.id) {
                            p.favorite = false;
                        }
                    });
                    break;
                case GlobalEvent.PROPERTY_CONTACTED:
                    this.properties.forEach((p: SearchResult) => {
                        if (p.id == e.data.id) {
                            p.contacted = true;
                            p.calls = e.data.calls;
                        }
                    });
                    break;
            }
        });

        // On the check properties
        application.on(application.resumeEvent, (args: application.ApplicationEventData) => {
            this.checkProperties();
        });
    }

    ngOnDestroy() {
        this._globalEventsSubscription.unsubscribe();

        // Off the check properties
        application.off(application.resumeEvent);
    }

    onLoadMoreItemsRequested(args: ListViewEventData) {
        let listView: RadListView = args.object;
        this.getNextPage((page: Page<SearchResult>) => {
            if (page.last) {
                //console.log("It was the last page");
                listView.loadOnDemandMode = ListViewLoadOnDemandMode[ListViewLoadOnDemandMode.None];
            }

            listView.notifyLoadOnDemandFinished();
        });
    }

    onPullToRefreshInitiated(args: ListViewEventData) {
        let listView: RadListView = args.object;
        this.reset((page: Page<SearchResult>) => {
            listView.notifyPullToRefreshFinished();
            this._changeDetectorRef.detectChanges();
        });
    }

    private checkProperties() {
        console.log("Checking for new properties");
        this._searchService.findAll(0)
            .subscribe(
                page => {
                    let firstPropertyId = this.properties && this.properties.length > 0 ? this.properties.getItem(0).id : null;
                    if (page.content && page.content.length > 0 && page.content[0].id != firstPropertyId) {
                        this._dialogService.confirmWithKey("main.all-properties.new-properties", 
                            (result: boolean) => {
                                if (result) {
                                    this.properties = new ObservableArray<SearchResult>(page.content);

                                    this.currentPage = page.number;
                                    this.hasMorePages = !page.last;

                                    // Not really necessary
                                    this.isLoading = false;
                                    this.dataLoaded = true;

                                    this._changeDetectorRef.detectChanges();
                                }
                            }
                        );
                    } else {
                        console.log("No new properties where detected.");
                    }
                },
                error => this._errorService.handleCommunicationError(error, () => { this.ngOnInit(); })
            );
    }

    private reset(callback?: (page: Page<SearchResult>) => void) {
        this.dataLoaded = false;
        this.currentPage = null;
        this.hasMorePages = true;

        // Reloading the elements
        this.getNextPage(callback);
    }

    private getNextPage(callback?: (page: Page<SearchResult>) => void) {
        let nextPage = this.currentPage != null ? this.currentPage + 1 : 0;

        //console.log("Loading next page: " + nextPage);
        this.isLoading = true;
        this._searchService.findAll(nextPage)
            .subscribe(
                page => {
                    if (page.first) {
                        this.properties = new ObservableArray<SearchResult>(page.content);
                    } else {
                        this.properties.push(page.content);
                    }
                    //console.log("Data received: " + JSON.stringify(page.content));

                    this.currentPage = page.number;
                    this.hasMorePages = !page.last;
                    //console.log("Page loaded: " + this.currentPage);

                    this.isLoading = false;
                    this.dataLoaded = true;

                    if (callback) {
                        callback(page);
                    }
                },
                error => this._errorService.handleCommunicationError(error, () => { this.ngOnInit(); })
            );
    }

}
ginev commented 7 years ago

@ignaciolarranaga I've been playing with your app on iOS and actually don't see any glitches with Load on Demand. What I see is that you reset the whole list when scrolling up because the Pull-to-Refresh logic is triggered. Normally, Pull-to-Refresh is used to append more items at the beginning of the list as demonstrated here: https://github.com/telerik/nativescript-ui-samples/tree/release/sdk/app/listview/pull-to-refresh. The way you are applying Pull-to-Refresh makes the list suddenly disappear which is not a gut UX practice. Anyway, we have identified a small issue which degrades performance with RadListView when using a List-Footer and will fix it. This should improve scrolling performance.

ginev commented 7 years ago

From @ignaciolarranaga on March 9, 2017 12:58

Hi @ginev, thanks for the answer, will fix the pull on request as you suggest.

Regarding the original problem, it is actually the components changing on the load on the demand, scroll slowly and pay attention on the first request for example, you will notice suddenly the images that you were seeing near to the top are replaced by new ones (like if you were jump ahead in the scroll). Actually the entire component is replaced, but the image is the most noticeable thing. The scroll and list keeps consistent, it is just this "jump". I think it is because the component reutilization that lost sync with the scroll (like component N should be at Ypx and after recalculate another component takes over), but this is most a guess.

I can prepare a video capture if you prefer.

ginev commented 7 years ago

@ignaciolarranaga, I have managed to reproduce this behavior. I see items suddenly replaced while scrolling slowly towards the end of the list.

Looking at your code snippets I cannot say what causes the issue. It will be most helpful if you either isolate the case in a project and send it to me for debugging or directly ZIP your app and let me run it on my side.

Since posting here the source of your app isn't a good idea, you can upload it on Google Cloud or Dropbox and send me a private link per e-mail.

Let me know how it works for you?

ignaciolarranaga commented 7 years ago

Let me prepare something.

NickIliev commented 7 years ago

+1 reported via t.1107652

Sample project depicting the issue can be found here

Note: resolved when this line was introduced for iOS. If this is a mandatory setting for LoadOnDemand in iOS then perhaps we should add it to the documentation.

VladimirAmiorkov commented 7 years ago

@NickIliev Yes that line of code is required on iOS 9+ (it is not required on iOS 8.4) as it sets the size of the cells in the RadListView. On iOS having dynamic cell size + load on demand is not supported due to native platform limitations of the UICollectionView. We have done a lot of research on how we can improve this strange behavior of the UICollectionView but there is still no good solution on how such behavior could be achieve in iOS.

Due to this internal behavior change that comes from the native iOS UICollectionView we have not documented this officially in our documentation but if you feel it could be useful we could easily do that.

leocaseiro commented 7 years ago

I was wondering if using loadOnDemandBufferSize would avoid the flicks...

Eq:

loadOnDemandBufferSize="5"

praven07 commented 7 years ago

I'm having the same issue. Is there any fix for it?

ginev commented 7 years ago

Hello @praven07 - this is quite a long thread. Would it be possible if you opened a new Issue here with details on your case as per the Issue Template and we will assist you?

codeback commented 6 years ago

Hi, I am experiencing the same problem.

Please see #377.

hettiger commented 6 years ago

Probably highly related to #400

I've provided a playground there, that clearly shows the issue.

hettiger commented 6 years ago

@NickIliev @VladimirAmiorkov

Note: resolved when this line was introduced for iOS. If this is a mandatory setting for LoadOnDemand in iOS then perhaps we should add it to the documentation.

On iOS having dynamic cell size + load on demand is not supported due to native platform limitations of the UICollectionView. We have done a lot of research on how we can improve this strange behavior of the UICollectionView but there is still no good solution on how such behavior could be achieve in iOS.

I understand that there are platform limitations but this is a really common design in apps nowadays. Just take the Facebook timeline as a prominent example. There is pull to refresh, load on demand and dynamic height involved. Also they have multiple templates...

In my opinion there must be a simple approach to this kind of design in NativeScript. Until now I thought RadListView would solve these issues but obviously this is not the case. Designers and or managers will ask for this kind of design. How are we, the developers, supposed to do it in NativeScript?

I hope someone can provide an alternative solution to a vertical linear list with multiple item templates, pull to refresh and load on demand that works on both Android and iOS or even better: Find a way to make it possible with RadListView.

Edit:

Btw. I just tried to use ListView which resulted in very similar scroll jumps etc. Here's the playground if someone's interested: https://play.nativescript.org/?id=4OVVnRPoq00YscBso3h40&template=play-ng

mrazahasan commented 6 years ago

I am experiencing the same issue, I have multiple templates with different heights. Is there any solution?

hettiger commented 6 years ago

@mrazahasan We had a discussion on Slack about it: https://nativescriptcommunity.slack.com/archives/C0L9EEURY/p1510669520000094

Unfortunately not everyone seems to give this the highest priority:

sean [4:59 PM] The specific use-case is smaller in scale from most apps and every article does point out how much of a pain it is to handle the varying heights.

Source

On the other hand it is hopefully going to happen if someone experienced enough get's the chance to work on this:

sean [4:56 PM] Xamarin has it working: https://developer.xamarin.com/guides/ios/user_interface/controls/tables/autosizing-row-height/

Which means NS should be able to do it as well.

Source

There were also a few suggestions on how we could possibly work around but I couldn't get anything to work yet. It has been pure frustration so far.

Suggestions were:

sean [4:50 PM] In that case, you're going to need to get at the inner workings of the cells. You can extend the UICollectionView and use UICollectionViewDelegateFlowLayout.

The method you are looking for is:

func collectionView(collectionView : UICollectionView,layout collectionViewLayout:UICollectionViewLayout,sizeForItemAtIndexPath indexPath:NSIndexPath) -> CGSize

If that's outside of your comfort, you could try manually resizing each cell after it paints, based on it's contents (to see if it'll even resize), if it does, you could probably come up with a mock calculation to handle the height of each cell. (all of this in your NS app, using the .ios. property tree)

Source

and/or porting a Pod like https://cocoapods.org/pods/UICollectionView-ARDynamicHeightLayoutCell.

My attempt

I've tried to combine the above two suggestions. You can check it out at: https://github.com/hettiger/nativescript-dynamic-collection-view-demo/

All the changes that I've done are combined in a single commit: https://github.com/hettiger/nativescript-dynamic-collection-view-demo/commit/1e660a958c9c720a13d6f5c6647898022b86b646

Please note that I also had to add the following line to the file node_modules/nativescript-pro-ui/listview/listview.ios.js:

exports.TKListViewDelegateImpl = TKListViewDelegateImpl;

Anyways this did not improve performance or solve the issue in any way, but at least it's building and running with the extended delegate. Please also note that I did totally ignore android as this repo is just for testing and problem solving...

I uploaded that code in the hopes that anyone can possibly help finding a workaround until this issue get's resolved.

Sharique-Hasan commented 6 years ago

I am facing exactly the same issue and ultimately I had to migrate to ngFor approach which is not recommended at all. I did that just to have a smooth scroll and smooth loadOnDemand data addition.

But RadListView team should have given it a priority as now most of the apps are not limited listing apps. I submitted my android app on playstore and Only stuck in iOS app just because of this reason and I now cannot afford to move to native or react-native app. Kindly give this a priority as this will become almost an year old issue reported first on January 30, 2017 11:55

@VladimirAmiorkov

tgpetrov commented 6 years ago

Hi @hettiger, @Sharique-Hasan, @mrazahasan and @codeback

I have to agree that even though not most apps use a lists with different item size and load on demand, it is still a common enough scenario, where the current behavior of RadListView on iOS is not acceptable. We will raise the priority of this issue and do our best to improve the scrolling experience when having load on demand even when item size is not fixed.

tgpetrov commented 6 years ago

Hi @hettiger, @Sharique-Hasan, @mrazahasan and @codeback

As this is a very long thread where different scenarios discussed, I’d like to try to sum them up: 1) While using manual load on demand with items that have dynamic height, when additional items are loaded there is a noticeable flicker of the items (this is the original issue and in order to better illustrate it I have also logged it here) 2) While using automatic load on demand with items that have dynamic height, when additional items are loaded there is a noticeable jump of the items (as if the scroll position changes, in order to better illustrate it I have also logged it here) 3) The playground provided by @hettiger also shows how the list scrolls to the top each time items are loaded. This issue was only reproducible on Angular and after updating the Angular version to 5.0 this seems to no longer happen 4) In the thread this issue is also mentioned, but though it is also related with the Load on Demand feature it is not related with the original cause of this thread – the jumping/flickering.

The latest NativeScript Pro UI (3.3.0) release contains fixes to the first two issues. As they were those that started this thread, I will now close it. In order to follow the status of the other load on demand issue, keep track on its own item.

hettiger commented 6 years ago

Thank you for this late Christmas present. I really do appreciate it. This will make NativeScript much more versatile to use.

jfturcot commented 6 years ago

@tgpetrov This isn't the right place for this question, but I am also experiencing this problem with regular ListView. Do you know if those fixes will also be added to ListView soon?

Thanks!

Cayan commented 6 years ago

Hello I'm still experiencing issues with regular ListView, is there any expectation for that fix?

hettiger commented 6 years ago

@Cayan and anyone else having issues with the regular ListView:

Have a look at https://github.com/NativeScript/NativeScript/issues/5588#issuecomment-376509345

BasyaLipman commented 6 years ago

@tgpetrov I seem to be experiencing this when using RadListView with dynamic height items, from "nativescript-ui-listview": "^3.5.7". My app is NS 4.1 + Angular 6, and the list items have dynamic height.

When I click Load More the list flickers, and scrolls to the beginning of the list.

I have verified that pull-to-refresh is not being triggered when I click Load More, so that is not the cause of the problem.

On other lists on my app which do not have dynamic height items, there is no problem and Load More is smooth.

Am I implementing wrong, or is this still an issue?

Thanks!

BasyaLipman commented 6 years ago

My problem was caused by wrapping the ng-templates in *ngIf directive. Switching to the approach described here worked.

uzarsalan commented 4 years ago

@tgpetrov, can you please help with same issue in svelte-native-nativescript-ui that i mentioned above? I'm not well in iOS programming. Will be very grateful