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

Back navigation to a screen with RadListView causes exception due to having an selected item #350

Closed bradmartin closed 6 years ago

bradmartin commented 6 years ago

Please, provide the details below:

Did you verify this is a real problem by searching Stack Overflow?

YES

Tell us about the problem

Please, ensure your title is less than 63 characters long and starts with a capital letter.

Navigating back to a master list using the RLV from the detail screen using the press selectionBehavior and single item selection will throw an exception unless you deselectItemAt on the itemSelection event or sometime before the itemSelection event fires again when you reload the RLV page. Somewhat related to https://github.com/telerik/nativescript-ui-feedback/issues/271

Which platform(s) does your issue occur on?

Android confirmed

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

3.2.0 PRO-UI = 3.1.3

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

XML

 <lv:RadListView row="1" rowSpan="2" id="inventoryListView" paddingTop="{{ headerHeight }}" items="{{ inventoryListItems }}" selectionBehavior="Press" itemSelected="{{ onInventoryItemTap }}" pullToRefresh="true" pullToRefreshInitiated="{{ onPullToRefreshInitiated }}" visibility="{{ inventoryListItems.length >= 1 ? 'visible' : 'collapse' }}" itemSwipeProgressStarted="{{ onSwipeCellStarted }}" swipeActions="true">
                <lv:RadListView.itemTemplate>
                    <GridLayout minHeight="70" rows="auto, *, auto" columns="130, *" class="list-group-item">
/// removed the specific template views, not needed for this issue

                    </GridLayout>
                </lv:RadListView.itemTemplate>

In the itemSelected event

 const inventoryItem = this.inventoryListItems[
        args.index
      ] as GQL.IInventoryItem;
      CLog(JSON.stringify(inventoryItem));
      if (!inventoryItem.id) {
        return;
      }

      NavigationService.navigate(
        NavigationService.MODULES.InventoryDetail,
        inventoryItem
      );

No matter how I handle the obs-array during the page cycle (new instance, popping and pushing items) the exception throws. Unless you deselectItemAt which I've added this to the itemSelection event from above and the exception no longer throws.

    const listView = topmost().currentPage.getViewById(
        "inventoryListView"
      ) as RadListView;

      listView.deselectItemAt(args.index);
      CLog("deselcting item at index = " + args.index);

I've debugged it a bunch but at this point I'm not sure if this is related to NativeScript caching the views/pages and it trying to reload the cached views so that item is still selected in the RLV when you back nav. I tried capturing the page isBackNavigation and returning out of the itemSelected event but the exception was still thrown as it seems to be deeper down in the RLV source code.

Just curious if this is by design on how things should work, maybe the RLV can tie into the NS page cycle and when navigatingFrom the items are deselected by default and a new property is introduced on the RLV to retain the items during navigation. Not sure since I don't have the source πŸ˜„ so really curious to hear any insight.

NickIliev commented 6 years ago

Hey, @bradmartin I have tried to reproduce the issue using this test project and noticed that you are sele4cting an item and then navigating to the detailed page using the itemSelected event.

What is happening on my side is that the RVL is caching the state and the item stays selected which creates a circular navigation - going back to the master view will automatically trigger the logic inside the itemSelected callback, and the navigation will execute again going to the details page. In your case, there is an additional logic which will be executed again before the navigation and perhaps this is the reason for throwing the error.

bradmartin commented 6 years ago

Thanks @NickIliev - using my approach of deselectItemAt after calling my frame navigation works. I just had to spend many hours trying to figure this out, i might have overlooked it in one of the examples but I feel like this might be something that we could add some better documentation around. With the core listview this is not the behavior when you select an item obviously, so you don't have the issue. I know the controls are different, but my concern is a user, like myself, migrating from core list-view to RLV and they stumble on this. It could be a huge time waste for them and I wouldn't want a new user to be scared or not enjoy their experience with the UI controls πŸ˜„ I'm not even sure how best to improve the documentation in this area but some mention somewhere about the common scenario of using a listview for master-detail behavior would be a good section for it.

VladimirAmiorkov commented 6 years ago

Hi @bradmartin ,

We are currently investigating a possible solution about the discussed scenario in this thread. Could you verify that the issue is that when the selectionBehavior is set to Press the itemSelected is raised on Android when navigating to a Page which contains an selected item?

Here is my current issue and a proposal for a possible resolution: On iOS the itemSelected is only raised once when the item is selected while on Android it is raised once when the item is selected and each time follow if you navigate to this Page that contains the selected item. The proposal for a solution is to limit those "secondary" raised events on Android and make it behave as it is on iOS. Does that sound appropriate to you ?

bradmartin commented 6 years ago

That sounds good @VladimirAmiorkov and yes you're correct. It only occurred on Android, sorry if i failed to mention that πŸ‘ but absolutely your idea sounds like a great solution. I just think anything to avoid throwing an exception deep in the RLV for the scenario would help new RLV users switching from core list-view πŸ˜„

Thanks for your time and work, much appreciated as always πŸŽ‰

VladimirAmiorkov commented 6 years ago

@bradmartin Thanks for the lightning fast response πŸ’― , I will update you once this lands in the "next" version of nativescript-pro-ui

VladimirAmiorkov commented 6 years ago

I have reopened this and edited its title accordingly.

VladimirAmiorkov commented 6 years ago

Hi @bradmartin ,

The discussed fix is available in our next version, to test it simply do:

tns plugin add nativescript-pro-ui@next

or change your package.json to:

nativescript-pro-ui="next"

This fix is also planned for our next official release 3.2.0

peppeg85 commented 5 years ago

hello, i still have this problem, not only on back button but also on the view change event: i have a radlistview, i execute an http request on every item selected/deselected event....but the radlistview fires the deselected event also at the changing of the view, so i have a lot of trouble to handle it.. try to imagine, i first select the elements and set them in the db with the httprequest, and then they are deselected at the change of the page! the version of listview is the latest. how can i solve this? observing the back button and use a state variable to check if i'm changing view? in the xml i usethis:

    `<lv:RadListView id="topics" items="{{ topics }}" selectionBehavior="Press" multipleSelection="true" 
         itemSelected="onItemSelected" itemDeselected="onItemDeselected">
    `

thank you

VladimirAmiorkov commented 5 years ago

@peppeg85 Can you share with us the version of the nativescript-ui-listview you are using, it would be easier to provide the entire package.json contents. Also can you try to reproduce the "going forward" issue inside our playground and share with us a link of the project.

peppeg85 commented 5 years ago

hello, thanks for your answer, here is the package.json:

{ "description": "xxxxxx", "license": "SEE LICENSE IN <your-license-filename>", "readme": "xxxxxxx", "repository": "<fill-your-repository-here>", "nativescript": { "id": "xxxxxxx", "tns-ios": { "version": "5.0.0-rc-2018-10-17-161243-01" }, "tns-android": { "version": "4.2.0" } }, "dependencies": { "moment": "^2.22.2", "nativescript-checkbox": "^3.0.3", "nativescript-loading-indicator": "^2.4.0", "nativescript-plugin-firebase": "^7.1.5", "nativescript-theme-core": "^1.0.4", "nativescript-ui-listview": "^3.8.0", "nativescript-ui-sidedrawer": "^4.3.0", "tns-core-modules": "^4.2.1" }, "devDependencies": { "babel-traverse": "6.4.5", "babel-types": "6.4.5", "babylon": "6.4.5", "lazy": "1.0.11" } }

at this moment i can not set a playground example