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 9.1.0 Android: itemTap args.view gets undefined when removing all items in ObservableArray and add new items #1508

Closed felixkrautschuk closed 2 years ago

felixkrautschuk commented 3 years ago

Please, provide the details below:

Tell us about the problem

When using a RadListView with pullToRefresh on Android, an app crashes when initiating the pullToRefresh operation and after that tapping an item, because args.view is getting undefined after the pullToRefresh.

See the GIF to understand: temp

This is what I am doing:

const array = [
    {id: "1", title: "This is the title"},
    {id: "2", title: "This is the title"},
    {id: "3", title: "This is the title"},
    {id: "4", title: "This is the title"},
    {id: "5", title: "This is the title"},
    {id: "6", title: "This is the title"},
    {id: "7", title: "This is the title"},
    {id: "8", title: "This is the title"},
    {id: "9", title: "This is the title"},
    {id: "10", title: "This is the title"}
];

export function onListViewLoaded(args: EventData) {
    lv = args.object as RadListView;
}

export function onItemTap(args: ListViewEventData) {
    alert(args.view);
}

export function onPullToRefreshInitiated(args: ListViewEventData) {
    loadItems().then(() => {
        lv.notifyPullToRefreshFinished();
    });
}

export function loadItems(): Promise<any> {
    return new Promise((resolve, reject) => {
        setTimeout(() => {
            vm.get("items").splice(0, vm.get("items").length);
            vm.get("items").push(array);

            resolve();
        }, 1000);
    });
}

(In this sample app, the pull-to-refresh is simply removing the items from ObservableArray and pushing the same items again...)

When trying to access the bindingContext of the view inside itemTap-event, the app crashes:

System.err: TypeError: Cannot read property 'bindingContext' of undefined System.err: System.err: StackTrace: System.err: onItemTap(file: app/main-page.ts:20:17) System.err: at _handleEvent(file: node_modules/@nativescript/core/data/observable/index.js:233:0) System.err: at notify(file: node_modules/@nativescript/core/data/observable/index.js:216:0) System.err: at ListViewItemClickListenerImpl.onItemClick(file: node_modules/nativescript-ui-listview/ui-listview.android.js:597:0) System.err: at com.tns.Runtime.callJSMethodNative(Native Method) System.err: at com.tns.Runtime.dispatchCallJSMethodNative(Runtime.java:1302) System.err: at com.tns.Runtime.callJSMethodImpl(Runtime.java:1188) System.err: at com.tns.Runtime.callJSMethod(Runtime.java:1175) System.err: at com.tns.Runtime.callJSMethod(Runtime.java:1153) System.err: at com.tns.Runtime.callJSMethod(Runtime.java:1149) System.err: at com.tns.gen.java.lang.Object_vendor_57309_28_ListViewItemClickListenerImpl.onItemClick(Object_vendor_57309_28_ListViewItemClickListenerImpl.java:20) System.err: at com.telerik.widget.list.RadListView.notifyOnTapUp(RadListView.java:746) System.err: at com.telerik.widget.list.RadListView.notifyOnTapUp(RadListView.java:755) System.err: at com.telerik.widget.list.ListViewGestureListener.onTapUp(ListViewGestureListener.java:191) System.err: at com.telerik.widget.list.ListViewGestureListener.onSingleTapUp(ListViewGestureListener.java:290) System.err: at android.view.GestureDetector.onTouchEvent(GestureDetector.java:730) System.err: at com.telerik.widget.list.ListViewGestureListener.onTouchEvent(ListViewGestureListener.java:165) System.err: at com.telerik.widget.list.RadListView.onTouchEvent(RadListView.java:555) System.err: at android.view.View.dispatchTouchEvent(View.java:13415) System.err: at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3054) System.err: at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2741) System.err: at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3060) System.err: at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2755) System.err: at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3060) System.err: at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2755) System.err: at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3060) System.err: at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2755) System.err: at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3060) System.err: at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2755) System.err: at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3060) System.err: at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2755) System.err: at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3060) System.err: at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2755) System.err: at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3060) System.err: at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2755) System.err: at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3060) System.err: at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2755) System.err: at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3060) System.err: at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2755) System.err: at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3060) System.err: at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2755) System.err: at com.android.internal.policy.DecorView.superDispatchTouchEvent(DecorView.java:465) System.err: at com.android.internal.policy.PhoneWindow.superDispatchTouchEvent(PhoneWindow.java:1849) System.err: at android.app.Activity.dispatchTouchEvent(Activity.java:3993) System.err: at androidx.appcompat.view.WindowCallbackWrapper.dispatchTouchEvent(WindowCallbackWrapper.java:69) System.err: at com.android.internal.policy.DecorView.dispatchTouchEvent(DecorView.java:423) System.err: at android.view.View.dispatchPointerEvent(View.java:13674) System.err: at android.view.ViewRootImpl$ViewPostImeInputStage.processPointerEvent(ViewRootImpl.java:5482) System.err: at android.view.ViewRootImpl$ViewPostImeInputStage.onProcess(ViewRootImpl.java:5285) System.err: at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4788) System.err: at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:4841) System.err: at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:4807) System.err: at android.view.ViewRootImpl$AsyncInputStage.forward(ViewRootImpl.java:4947) System.err: at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:4815) System.err: at android.view.ViewRootImpl$AsyncInputStage.apply(ViewRootImpl.java:5004) System.err: at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4788) System.err: at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:4841) System.err: at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:4807) System.err: at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:4815) System.err: at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4788) System.err: at android.view.ViewRootImpl.deliverInputEvent(ViewRootImpl.java:7505) System.err: at android.view.ViewRootImpl.doProcessInputEvents(ViewRootImpl.java:7474) System.err: at android.view.ViewRootImpl.enqueueInputEvent(ViewRootImpl.java:7435) System.err: at android.view.ViewRootImpl$WindowInputEventReceiver.onInputEvent(ViewRootImpl.java:7630) System.err: at android.view.InputEventReceiver.dispatchInputEvent(InputEventReceiver.java:188) System.err: at android.os.MessageQueue.nativePollOnce(Native Method) System.err: at android.os.MessageQueue.next(MessageQueue.java:336) System.err: at android.os.Looper.loop(Looper.java:174) System.err: at android.app.ActivityThread.main(ActivityThread.java:7356) System.err: at java.lang.reflect.Method.invoke(Native Method) System.err: at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492) System.err: at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:930)

This issue is NOT happening on iOS and it was working as expected on Android when using NS 6 with RadListView 8.2.0. After upgrading to NS 7 using multiple @nativescript/core versions and using RadListView 9.1.0, this issue occurs.

Which platform(s) does your issue occur on?

Android (multiple versions)

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.

  1. Start the application ..
  2. tap any item -> args.view is getting alerted
  3. pull to refresh
  4. tap any item -> args.view is undefined

Is there code involved? If so, please share the minimal amount of code needed to recreate the problem.

ns-rlv-issue-android.zip

OPADA-Eng commented 3 years ago

I have the same issue any workaround?

felixkrautschuk commented 3 years ago

This is still an issue using nativescript-ui-listview latest 9.1.4 and NS 8. Updated sample app with latest dependencies: ns-rlv-issue-android.zip

We had accidentally updated to 9.1.4 in our production app and we have seen that the amount of crashes heavily increased. So there must be other situations than pull-to-refresh where this issue occurs as well, as not that many people use the pull-to-refresh feature. After downgrading to 9.0.4 the number of crashes immediately decreased again, so this version is the last working version on our side.

It would really help to see what changes where made between 9.0.4 and 9.1.0.

felixkrautschuk commented 2 years ago

Just checked out nativescript-ui-listview@10.0.0, but unfortunately the issue is still existing

felixkrautschuk commented 2 years ago

updated demo app with latest dependencies and new observations ns-rlv-issue-android.zip

I noticed, that this issue is NOT related to the pull-to-refresh functionality of the RadListView component on Android...

Instead, the issue occurs when removing all items in the ObserableArray (the datasource of the RadListView) and adding new items to it.

export function updateItems() {
    setTimeout(() => {
                //clearing datasource
        vm.get("items").splice(0, vm.get("items").length);

                //adding new items
        vm.get("items").push([{id: 99, title: "Item with id 99"}, ..., ...]);
    }, 500);
}

After doing that, the new items are visible, but when tapping an item of the ListView, args.view is undefined.

https://user-images.githubusercontent.com/6443021/131102415-b6b6059b-7a8b-40cf-a423-b168b702c678.mov

It does not matter in which way I am clearing the ObservableArray, so the issue occurs the same when:

When I leave at least one item in the ObservableArray and add the new items afterwards, everything works as expected (vm.get("items").splice(0, vm.get("items").length); or listview.items.length = 1;)

When clearing ALL items of the ObservableArray and calling listview.refresh() afterwards, the issue does not occur. But then the usage of the ObservableArray has no meaning anymore, if I need to call the refresh method all the time.

Clearing a list and filling it with new items should be a common use-case and in addition, ALL of that was working as expected with nativescript-ui-listview 9.0.4

Updated demo app with latest dependencies: see beginning of this comment

felixkrautschuk commented 2 years ago

I made some furher investigations and noticed, that accessing the view from the event data of itemTap, itemHold etc is working as expected after loading the data items into the RadListView for the first time. The ListViewAdapter contains 9 viewHolders as expected.

I am logging it like this in the itemLoading-event:

export function onItemLoading(args: ListViewEventData) {
    ...
    if(isAndroid) {
        console.log("viewHolders count:");
        // @ts-ignore
        console.log(args.object._android._listViewAdapter._viewHolders.length);
    }
}

When removing all items of the ObservableArray (splice) and adding new items to it like this:

//main-page.ts

export function updateItemsAsync() {
    setTimeout(() => {
        vm.get("items").splice(0, vm.get("items").length);

        const newItems = [];
        for(let i = 1000; i < 1050; i++) {
            newItems.push({id: i, title: "Async item with id " + i});
        }

        vm.get("items").push(newItems);
    }, 1000);
}

... the views and their corresponding bindingContext are also accessible within the itemLoading event, however the viewHolders element of the ListViewAdapter is now an empty list and now events like itemTap or itemHold return undefined for the view element of the event data

//node_modules/nativescript-ui-listview/index.android.js

ListViewItemClickListenerImpl.prototype.onItemClick = function (itemPosition, motionEvent) {
        //...
        var item = listView.getItemAtIndex(originalPosition);
        var tappedView = listView._listViewAdapter.getViewForItem(item);    //--> undefined, because this._listViewAdapter._viewHolders is empty
        //...
};
//node_modules/nativescript-ui-listview/index.android.js

ListViewAdapter.prototype.getViewForItem = function (item) {
        for (var i = 0; i < this._viewHolders.length; i++) {  //for loop is getting skipped, as viewHolders is empty array
            if (this._viewHolders[i]['nsView'] && this._viewHolders[i]['nsView'].bindingContext === item) {
                return this._viewHolders[i]['nsView'].getChildAt(0);
            }
        }
        return undefined;
};

When scrolling the RadListView, the viewHolders list is getting filled with 3 or 4 items and the itemTap, itemHold, (...) events are returning the correct view and its bindingContext for those items, but still undefined for the other items.

https://user-images.githubusercontent.com/6443021/133612367-5d8ea116-e83d-402e-b17e-ee5efe64c061.mov

By the way, the behaviour is exactly the same if the addition of the list items is done synchronously (without setTimeout).

I noticed that the new rangeAdd functionality introduced in version 9.1.0 seems to cause this issue somehow:

_nodemodules/nativescript-ui-listview/index.android.js

onSourceCollectionChanged(data) {
        ...
        if (this._listViewAdapter) {
            ...
            else if (data.action === ChangeType.Add) {
                if (isNaN(data.index)) {
                    this._listViewAdapter.rangeAdd(data.addedCount);
                    this._currentId += data.addedCount;
                }
                else {
                    this._listViewAdapter.rangeAdd(data.index, data.addedCount);
                    this._currentId += data.addedCount;
                }
            }
            ...
        }
}

At least I do not get this issue anymore, when replacing that part with the code from nativescript-ui-listview 9.0.4:

onSourceCollectionChanged(data) {
        ...
        if (this._listViewAdapter) {
            ...
            else if (data.action === ChangeType.Add) {
                for (let i = 0; i < data.addedCount; i++) {
                    if (isNaN(data.index)) {
                        this._listViewAdapter.add(new java.lang.Integer(this._getUniqueItemId()));
                    } else {
                        this._listViewAdapter.add(data.index, new java.lang.Integer(this._getUniqueItemId()));
                    }
                }
            }
            ...
        }
}

It is hard to say why that rangeAdd stuff should cause this issue, as we have no access to the native com.telerik.widget.list.ListViewAdapter package.

So it would be great if @NathanWalker @triniwiz @rigor789 or whoever is working on the RadListView could have a look at this. Right now we are still fine with nativescript-ui-listview 9.0.4, but it would be better to be able to upgrade to the latest dependencies.

Demo app with latest dependencies: ns-rlv-issue-android.zip

felixkrautschuk commented 2 years ago

@NathanWalker @triniwiz @rigor789

the issue does NOT occur, when keeping at least one item in the list during splice, like this:

vm.get("items").splice(1, vm.get("items").length);

I guess that this rangeAdd functionality does something special if the list items are empty before adding new items, but I have no idea why and without access to the native sources behind the RadListView it is hard to further invesigate and to provide a pull-request

OPADA-Eng commented 2 years ago

@NathanWalker @triniwiz @rigor789 Any progress on fixing this?

rigor789 commented 2 years ago

@OPADA-Eng @felixkrautschuk has kindly contributed a fix, we'll be releasing it early next week (or today/tomorrow, depending on time) and ping back here once it's out.

Edit: try nativescript-ui-listview@10.0.1

OPADA-Eng commented 2 years ago

It works! Thanks