Uno: NavigationView - Android - unable to select first menuitem after selecting Settings item

Created on 12 May 2020  路  36Comments  路  Source: unoplatform/uno

Current behavior

When selecting the navigationview settings item and then selecting the top menu item nothing happens and the ItemInvoked is not fired on Android. If first selecting the settings item and then the second menu item everything works as expected.

Expected behavior

When selecting the navigationview settings item and then any menu item the event should be fired on Android as it does on Windows/UWP

How to reproduce it (as minimally and precisely as possible)

  • Start Visual Studio 2019
  • Create new project using Uno Platform App Template

In shared projects mainpage.xaml paste in this:

<Grid>
        <NavigationView ItemInvoked="OnItemInvoked">
            <NavigationView.MenuItems>
                <NavigationViewItem Content="First item"></NavigationViewItem>
                <NavigationViewItem Content="Second item"></NavigationViewItem>
            </NavigationView.MenuItems>
        </NavigationView>
   </Grid>

In mainpage.cs paste in ths:

private void OnItemInvoked(object sender, NavigationViewItemInvokedEventArgs args)
{}
  • Set a breakpoint at OnItemInvoked
  • Make sure that the android project is set as Startup project
  • Debug project
  • Select Settings item
  • Select "First item"

Environment

Nuget Package:
Uno.UI 2.2.0

Package Version(s):

Affected platform(s):

  • [ ] iOS
  • [x] Android
  • [ ] WebAssembly
  • [ ] Windows
  • [ ] Build tasks

Visual Studio

  • [ ] 2017 (version: )
  • [x ] 2019 (version: )
  • [ ] for Mac (version: )

Relevant plugins

  • [ ] Resharper (version: )
kinbug
Source
picture jabak

All 36 comments

Wondering if I could fix the problem myself I took a brief look at the Uno Platform codebase the other day. It's quite a large if not a huge codebase.

To someone with little or no experience with Uno Platform and no insights of how Uno works it just seem to be an overwhelming and daunting task to take on. It would be much appreciated if someone could point in the right direction: where to look, how to test in gitpod etc.

jabak picture jabak on 24 May 2020

Your first step will be to locally build Uno for Android and make sure you can debug and hit breakpoints in Uno code. The contributors section of the docs covers local set-up: https://platform.uno/docs/articles/uno-development/debugging-uno-ui.html

Make sure to note the section on solution filters, since building for a single target (Android in this case) is a lot faster and less RAM-hungry than building for every target.

davidjohnoliver picture davidjohnoliver on 25 May 2020

Thank you David.

I cloned the repo, copied crosstargeting_override.props.sample into crosstargeting_override.props, made the modifications to the file (MonoAndroid10.0) and voil谩! After a few times of "Rebuild All" it worked: ========== Rebuild All: 17 succeeded, 0 failed, 0 skipped ==========

After that i tested if I could hit a breakpoint by setting a breakpoint at in the BindableButton constructor. Success.

    public BindableButton(Android.Content.Context context)
        : base(context)
    {
        Initialize(context, null); <<< breakpoint hit
    }

So I guess i'm ready to move on then. I searched the solution for 'navigationview' and found two instances of interest: the first instance found was located under generated\3.0.0.0 - that seems to be outputted by a sourcegenerator/translator you guys have built... i think. The other instance found was located under UI\Xaml\Controls.

So no Android specific control but rather some "magic" i haven't had enough time to decipher. And that's where my lack of knowledge of how Uno works manifests itself :o)

jabak picture jabak on 25 May 2020

@jabak Excellent first step, as you note the NavigationView class is split amongst multiple files. (It's pretty important to understand how partial classes work when working on Uno.) You can ignore the generated files for now - those cover members of NavigationView that haven't been implemented yet.

In the case of NavigationView there's no 'Android-specific control', because the code is shared between all platforms Uno supports (Android, iOS, WASM, MacOS). In other cases, if code is specific to a particular platform it's usually indicated by the file suffix (eg *.Android.cs).

The next step is to debug a repro of the issue. You can create a new sample for that - there's a doc on creating samples here, and the existing NavView samples also can give you examples to work from. https://github.com/unoplatform/uno/tree/master/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Controls/NavigationViewTests

GitHub
Build Mobile, Desktop and WebAssembly apps with C# and XAML. Today. Open source and professionally supported. - unoplatform/uno
davidjohnoliver picture davidjohnoliver on 26 May 2020

@davidjohnoliver Luckily partial classes/methods are no strangers to me.

I've have now read and understood the "creating samples" doc. The existing navigationview samples all use the 'old' SampleControlInfoAttribute. The new sample I will be adding should be decorated with the new default which is 'SampleAttribute'. Correct?

How about naming conventions for samples? Should samples be named after what they are reproducing like "navigationview_navigate_settings_then_top_item' or?

When i'm done creating and debugging the sample the next step would be creating the UI test(s) in SamplesApp.UITests - Windows_UI_Xaml_Controls\NavigationViewTests right?

jabak picture jabak on 27 May 2020

I've have now read and understood the "creating samples" doc. The existing navigationview samples all use the 'old' SampleControlInfoAttribute. The new sample I will be adding should be decorated with the new default which is 'SampleAttribute'. Correct?

Yes, that's correct, with the same category name as the others, so [Sample("NavigationView")]

How about naming conventions for samples? Should samples be named after what they are reproducing like "navigationview_navigate_settings_then_top_item' or?

There's no hard-and-fast convention. The name you suggest would be fine (with CamelCasing), a simpler name would be fine too.

By the way, I notice that there's an existing sample that already hooks up the ItemInvoked method: https://github.com/unoplatform/uno/blob/master/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Controls/NavigationViewTests/NavigationViewSample.xaml

Are you able to repro your issue with that one?

When i'm done creating and debugging the sample the next step would be creating the UI test(s) in SamplesApp.UITests - Windows_UI_Xaml_Controls\NavigationViewTests right?

Yes indeed :)

GitHub
Build Mobile, Desktop and WebAssembly apps with C# and XAML. Today. Open source and professionally supported. - unoplatform/uno
davidjohnoliver picture davidjohnoliver on 27 May 2020

Using either the existing sample or my own sample I was able to reproduce the issue. When debugging i noticed that there's a huge difference in the call stack going from Settings to first menu item when compared to going from Settings to second menu item.

OnLayoutUpdated is not called in the first case as well as bunch of other stuff like AnimateSelection, FindSelection etc. I could dig deeper but if this info rings any bells then maybe you could point me in the right direction before i start digging.

Also I noticed a DispatchTouchEvent (android thing) in both cases btw.

jabak picture jabak on 27 May 2020

@jabak It sounds like you're on the right track, nothing jumps out as obvious from what you've said.

Could you post the full callstack for the ItemInvoked invocation in the case that it works correctly, for the second item?

Also I'm curious if you've tested on WebAssembly or iOS for whether the bug also occurs there. Knowing if a bug is cross-platform or specific to a single platform often helps in tracking down the source.

davidjohnoliver picture davidjohnoliver on 28 May 2020

I'll test both WebAssembly and iOS tonight - danish time tonight ;o)

The call stack for the working case (settings -> second item) as captured from the sampleapp OnItemInvoked:

0x1 in SamplesApp.Samples.NavigationViewSample.NavigationViewSample.NvSample_ItemInvoked at D:\Git\uno\src\SamplesApp\UITests.Shared\Windows_UI_Xaml_Controls\NavigationViewTests\NavigationViewSample.xaml.cs:76,4 C#
0x18 in SamplesApp.Samples.NavigationViewSample.NavigationViewSample. at D:\Git\uno\src\SamplesApp\SamplesApp.Droid\obj\Debug\100g\XamlCodeGenerator\NavigationViewSample_684788ea96183cb66363d8cf57c1ff85.g.cs:209,85 C#
0xA1 in Windows.UI.Xaml.Controls.NavigationView.RaiseItemInvoked at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\NavigationView\NavigationView.cs:1429,4 C#
0xB2 in Windows.UI.Xaml.Controls.NavigationView.ChangeSelection at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\NavigationView\NavigationView.cs:1331,9 C#
0x10 in Windows.UI.Xaml.Controls.NavigationView.OnSelectedItemPropertyChanged at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\NavigationView\NavigationView.cs:1880,4 C#
0x84 in Windows.UI.Xaml.Controls.NavigationView.OnPropertyChanged at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\NavigationView\NavigationView.cs:2659,5 C#
0xD in Windows.UI.Xaml.Controls.NavigationView.<>c.<.cctor>b__458_27 at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\NavigationView\NavigationView.Properties.cs:491,40 C#
0xF in Windows.UI.Xaml.PropertyMetadata.RaisePropertyChanged at D:\Git\uno\src\Uno.UI\UI\Xaml\PropertyMetadata.cs:144,4 C#
0x11B in Windows.UI.Xaml.DependencyObjectStore.InvokeCallbacks at D:\Git\uno\src\Uno.UI\UI\Xaml\DependencyObjectStore.cs:1430,4 C#
0x77 in Windows.UI.Xaml.DependencyObjectStore.RaiseCallbacks at D:\Git\uno\src\Uno.UI\UI\Xaml\DependencyObjectStore.cs:1344,5 C#
0x1CF in Windows.UI.Xaml.DependencyObjectStore.InnerSetValue at D:\Git\uno\src\Uno.UI\UI\Xaml\DependencyObjectStore.cs:475,5 C#
0x40 in Windows.UI.Xaml.DependencyObjectStore.SetValue at D:\Git\uno\src\Uno.UI\UI\Xaml\DependencyObjectStore.cs:418,5 C#
0x6 in Windows.UI.Xaml.DependencyObjectStore.SetValue at D:\Git\uno\src\Uno.UI\UI\Xaml\DependencyObjectStore.cs:382,4 C#
0x8 in Uno.UI.Controls.BindableView.SetValue at D:\Git\uno\src\Uno.UI\obj\Debug\MonoAndroid10.0g\DependencyObjectGenerator\Uno_UI_Controls_BindableView_72bb8e78163ad5c11de91e301960e619.g.cs:44,64 C#
0x7 in Windows.UI.Xaml.Controls.NavigationView.set_SelectedItem at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\NavigationView\NavigationView.Properties.cs:18,11 C#
0xA8 in Windows.UI.Xaml.Controls.NavigationView.SetSelectedItemAndExpectItemInvokeWhenSelectionChangedIfNotInvokedFromAPI at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\NavigationView\NavigationView.cs:1938,4 C#
0x98 in Windows.UI.Xaml.Controls.NavigationView.OnSelectionChanged at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\NavigationView\NavigationView.cs:1224,6 C#
0x24 in Windows.UI.Xaml.Controls.Primitives.Selector.InvokeSelectionChanged at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\Primitives\Selector.cs:182,5 C#
0xEE in Windows.UI.Xaml.Controls.Primitives.Selector.OnSelectedItemChanged at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\Primitives\Selector.cs:116,4 C#
0x149 in Windows.UI.Xaml.Controls.ListViewBase.OnSelectedItemChanged at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\ListViewBase\ListViewBase.cs:231,5 C#
0x13 in Windows.UI.Xaml.Controls.Primitives.Selector.<>c.<.cctor>b__63_0 at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\Primitives\Selector.cs:43,40 C#
0xF in Windows.UI.Xaml.PropertyMetadata.RaisePropertyChanged at D:\Git\uno\src\Uno.UI\UI\Xaml\PropertyMetadata.cs:144,4 C#
0x11B in Windows.UI.Xaml.DependencyObjectStore.InvokeCallbacks at D:\Git\uno\src\Uno.UI\UI\Xaml\DependencyObjectStore.cs:1430,4 C#
0x77 in Windows.UI.Xaml.DependencyObjectStore.RaiseCallbacks at D:\Git\uno\src\Uno.UI\UI\Xaml\DependencyObjectStore.cs:1344,5 C#
0x1CF in Windows.UI.Xaml.DependencyObjectStore.InnerSetValue at D:\Git\uno\src\Uno.UI\UI\Xaml\DependencyObjectStore.cs:475,5 C#
0x40 in Windows.UI.Xaml.DependencyObjectStore.SetValue at D:\Git\uno\src\Uno.UI\UI\Xaml\DependencyObjectStore.cs:418,5 C#
0x6 in Windows.UI.Xaml.DependencyObjectStore.SetValue at D:\Git\uno\src\Uno.UI\UI\Xaml\DependencyObjectStore.cs:382,4 C#
0x8 in Uno.UI.Controls.BindableView.SetValue at D:\Git\uno\src\Uno.UI\obj\Debug\MonoAndroid10.0g\DependencyObjectGenerator\Uno_UI_Controls_BindableView_72bb8e78163ad5c11de91e301960e619.g.cs:44,64 C#
0x7 in Windows.UI.Xaml.Controls.Primitives.Selector.set_SelectedItem at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\Primitives\Selector.cs:50,11 C#
0x3D in Windows.UI.Xaml.Controls.Primitives.Selector.OnSelectedIndexChanged at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\Primitives\Selector.cs:210,5 C#
0x4 in Windows.UI.Xaml.Controls.ListViewBase.OnSelectedIndexChanged at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\ListViewBase\ListViewBase.cs:256,4 C#
0x1C in Windows.UI.Xaml.Controls.Primitives.Selector.<>c.<.cctor>b__63_1 at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\Primitives\Selector.cs:195,15 C#
0xF in Windows.UI.Xaml.PropertyMetadata.RaisePropertyChanged at D:\Git\uno\src\Uno.UI\UI\Xaml\PropertyMetadata.cs:144,4 C#
0x11B in Windows.UI.Xaml.DependencyObjectStore.InvokeCallbacks at D:\Git\uno\src\Uno.UI\UI\Xaml\DependencyObjectStore.cs:1430,4 C#
0x77 in Windows.UI.Xaml.DependencyObjectStore.RaiseCallbacks at D:\Git\uno\src\Uno.UI\UI\Xaml\DependencyObjectStore.cs:1344,5 C#
0x1CF in Windows.UI.Xaml.DependencyObjectStore.InnerSetValue at D:\Git\uno\src\Uno.UI\UI\Xaml\DependencyObjectStore.cs:475,5 C#
0x40 in Windows.UI.Xaml.DependencyObjectStore.SetValue at D:\Git\uno\src\Uno.UI\UI\Xaml\DependencyObjectStore.cs:418,5 C#
0x6 in Windows.UI.Xaml.DependencyObjectStore.SetValue at D:\Git\uno\src\Uno.UI\UI\Xaml\DependencyObjectStore.cs:382,4 C#
0x8 in Uno.UI.Controls.BindableView.SetValue at D:\Git\uno\src\Uno.UI\obj\Debug\MonoAndroid10.0g\DependencyObjectGenerator\Uno_UI_Controls_BindableView_72bb8e78163ad5c11de91e301960e619.g.cs:44,64 C#
0xC in Windows.UI.Xaml.Controls.Primitives.Selector.set_SelectedIndex at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\Primitives\Selector.cs:189,11 C#
0x99 in Windows.UI.Xaml.Controls.ListViewBase.OnItemClicked at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\ListViewBase\ListViewBase.cs:395,6 C#
0x8 in Windows.UI.Xaml.Controls.Primitives.Selector.OnItemClicked at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\Primitives\Selector.cs:462,61 C#
0x21 in Windows.UI.Xaml.Controls.Primitives.SelectorItem.OnPointerReleased at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\Primitives\SelectorItem.cs:287,5 C#
0x7 in Windows.UI.Xaml.Controls.Control.<>c.<.cctor>b__309_11 at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\Control\Control.cs:781,52 C#
0xDF in Windows.UI.Xaml.UIElement.InvokeHandler at D:\Git\uno\src\Uno.UI\UI\Xaml\UIElement.RoutedEvents.cs:686,6 C#
0x8E in Windows.UI.Xaml.UIElement.RaiseEvent at D:\Git\uno\src\Uno.UI\UI\Xaml\UIElement.RoutedEvents.cs:545,7 C#
0x12 in Windows.UI.Xaml.UIElement.RaisePointerEvent at D:\Git\uno\src\Uno.UI\UI\Xaml\UIElement.Pointers.cs:655,5 C#
0x8E in Windows.UI.Xaml.UIElement.SetPressed at D:\Git\uno\src\Uno.UI\UI\Xaml\UIElement.Pointers.cs:789,5 C#
0x1B in Windows.UI.Xaml.UIElement.OnPointerUp at D:\Git\uno\src\Uno.UI\UI\Xaml\UIElement.Pointers.cs:553,4 C#
0x3 in Windows.UI.Xaml.UIElement.OnNativePointerUp at D:\Git\uno\src\Uno.UI\UI\Xaml\UIElement.Pointers.cs:545,66 C#
0xE6 in Windows.UI.Xaml.UIElement.OnNativeMotionEvent at D:\Git\uno\src\Uno.UI\UI\Xaml\UIElement.Pointers.Android.cs:123,6 C#
0xC4 in Windows.UI.Xaml.UIElement.OnNativeMotionEvent at D:\Git\uno\src\Uno.UI\UI\Xaml\UIElement.Pointers.Android.cs:94,5 C#
0x1F in Uno.UI.UnoViewGroup.n_OnNativeMotionEvent_Landroid_view_MotionEvent_Landroid_view_View_Z at D:\Git\uno\src\Uno.UI.BindingHelper.Android\obj\Debug\MonoAndroid10.0generated\src\Uno.UI.UnoViewGroup.cs:601,4 C#
[External Code]
0x16 in Windows.UI.Xaml.Controls.ListViewBase.DispatchTouchEvent at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\ListViewBase\ListViewBase.Android.cs:329,4 C#
0x4B in Uno.UI.BaseActivity.DispatchTouchEvent at D:\Git\uno\src\Uno.UI\BaseActivity.Android.cs:324,4 C#

jabak picture jabak on 28 May 2020

My advice then would be to try to find out at what point it's failing for the case of the first item by going up the call stack and finding out which part _isn't_ getting hit. These are the methods I'd check:

Windows.UI.Xaml.Controls.NavigationView.SetSelectedItemAndExpectItemInvokeWhenSelectionChangedIfNotInvokedFromAPI at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\NavigationView\NavigationView.cs:1938,4 C#

0x98 in Windows.UI.Xaml.Controls.NavigationView.OnSelectionChanged at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\NavigationView\NavigationView.cs:1224,6 C#

0x149 in Windows.UI.Xaml.Controls.ListViewBase.OnSelectedItemChanged at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\ListViewBase\ListViewBase.cs:231,5 C#

0x99 in Windows.UI.Xaml.Controls.ListViewBase.OnItemClicked at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\ListViewBase\ListViewBase.cs:395,6 C#

0x21 in Windows.UI.Xaml.Controls.Primitives.SelectorItem.OnPointerReleased at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\Primitives\SelectorItem.cs:287,5 C#

0x1B in Windows.UI.Xaml.UIElement.OnPointerUp at D:\Git\uno\src\Uno.UI\UI\Xaml\UIElement.Pointers.cs:553,4 C#

davidjohnoliver picture davidjohnoliver on 28 May 2020

@davidjohnoliver I'll take your advice and go through the call stacks of both cases comparing the two while keeping an eye on the methods you've pointed out.

WebAssembly work fine in both cases btw. Unfortunately I have not had the time to get my Mac up and running since I recently moved to a new place. I'll get things going in the next few days. Meanwhile I'll concentrate on debugging the android side of things. Cheers

jabak picture jabak on 28 May 2020

Ouch.. 16GB of RAM is not really enough for my machine any longer. Debugging Uno demands a lot of resources. So I've purchased 16GB but haven't had the time to install it.

Anyway i manged to get some debugging done today. It seems like when hitting: internal virtual void OnSelectedIndexChanged(int oldSelectedIndex, int newSelectedIndex) in: Windows.UI.Xaml.Controls.Primitives.Selector.OnSelectedIndexChanged at D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\Primitives\Selector.cs:208,4 things have already gone wrong.

The if statement below return true because both SelectedItem and newSelectedItem is equal to the first item.
if (!object.Equals(SelectedItem, newSelectedItem))
{
SelectedItem = newSelectedItem;
}

I dug a bit deeper and what really goes wrong is that when selecting the settings item SelectedItem is set to the first menu item no matter what item was selected before hitting Settings. So going from second menu item to settings causes SelectedItem to be set to First menu item. That's is what causes this unwanted behaviour. I have not had the time to investigate why that is but know we know why this is happening.

jabak picture jabak on 30 May 2020

The code that ultimately leads to the unwanted behaviour is this:
D:\Git\uno\src\Uno.UI\UI\Xaml\Controls\Primitives\Selector.cs

Line 91: SelectedItem = selectionToReset;

SelectedItem end up being null and later on this leads to SelectedItem being set to the first item in the menuitemlist.

There is a comment in the code that i don't quite understand:

// Note: in this scenario (previous SelectedItem no longer in collection either), Windows still leaves it at the
// previous value. We don't try to reproduce this behaviour, amongst other reasons because under Uno's DP system
// this would push an invalid value back through any 2-way binding.

jabak picture jabak on 31 May 2020

@jabak Interesting findings. Seems like you're getting close.

Regarding that comment: this code path is hit when Selector.SelectedItem is set to a non-null value that's not found in Selector.ItemsSource. The new value is considered invalid, and normally in this case, the previous value of SelectedItem is restored.

That comment refers to a particular edge case where the _previous_ value of SelectedItem is no longer found in the ItemsSource either, eg because the underlying collection was modified without the list being made aware. Uno handles this edge case slightly differently from UWP.

Anyway, if I understand correctly, selecting the Settings item somehow causes the first menu item to be set as SelectedItem, which means that SelectionChanged is not raised when the first item actually is selected?

davidjohnoliver picture davidjohnoliver on 1 Jun 2020

That is correct. Selecting the Settings items always sets the first menu item as Selecteditem and SelectionChanged is not raised when selecting the first item again. Something weird is going on in Selector.cs that's for sure.

When going from second menu item to Settings then line 91 in selector.cs gets hit and then line 137 eventually gets hit too thus setting first menu item's property IsSelected = true. It happens because ContainerFromItem(object item ) in Itemscontrol.cs by line 1141 default returns the first menu item when called with param = null. In that method line 1143: var index = IndexFromItem(item); returns index = -1 which is correct considering null is not an item in the menu item list. But somehow line 1144: MaterializedContainers.FirstOrDefault(container => Equals(container.GetValue(IndexForItemContainerProperty), index)); returns first menu item when index = -1.

jabak picture jabak on 1 Jun 2020

Aaah. Ok it's starting to make sense.

Can you confirm the runtime type of the Selector? I'm guessing it's a NavigationViewList. Also can you check what the type of InternalItemsPanelRoot is?

davidjohnoliver picture davidjohnoliver on 1 Jun 2020

Yup Selector is NavigationViewList. InternalItemsPanelRoot is of type Windows.UI.Xaml.Controls.NativeListViewBase

jabak picture jabak on 1 Jun 2020

So I think the problem is that IndexForItemContainer is not initialized, which means it has its default value of -1, which is causing the confusion.

Couple of potential bugs here:

  1. ContainerFromItem() shouldn't be accessing the IndexForItemContainerProperty directly. Instead it should call IndexFromContainer(), whose inner method is overridden by ListViewBase and also NavigationViewList. So:
    return MaterializedContainers.FirstOrDefault(container => Equals(IndexFromContainer(container), index));
  1. That on its own might fix the problem, but there should probably also be a catch-all check in ContainerFromItem(), so if index is -1 then we return null before checking MaterializedContainers.
davidjohnoliver picture davidjohnoliver on 1 Jun 2020

I'll add the fixes later today and do some debugging to see if it works.

jabak picture jabak on 2 Jun 2020

Implemented the fix: return index == -1 ? null : MaterializedContainers.FirstOrDefault(container => Equals(IndexFromContainer(container), index));

On android it worked like a charm. When trying to build the entire solution I get

Generation failed: System.AggregateException: One or more errors occurred. ---> System.OutOfMemoryException: Exception of type 'System.OutOfMemoryException' was thrown.

So it's time to get the newly acquired 16 GB RAM installed.

jabak picture jabak on 2 Jun 2020
👍1

Yeah, building the entire solution without solution filters + single-targeting is _very_ RAM-intensive.

If the fix works on Android, I'm fairly confident the code is also valid for other platforms, since it's pretty evident where the failure of intent was and how this change corrects it. Feel free to go ahead and create a PR for this with the appropriate test - the CI will verify that it works for iOS+WASM.

davidjohnoliver picture davidjohnoliver on 3 Jun 2020

I will do that. I'm not sure if I totally understand how Uno is put together yet. So I have to take your word for it ;o)

Anyway I've been working in the master branch locally. Should I go ahead and create the tests in the master branch as well? Any suggestions to what test would be appropriate to do? My thought is a test that first select the second item, then settings and finally the first item checking the content page name of each item.

jabak picture jabak on 3 Jun 2020

Yes, that makes sense for the test. You can probably reuse the existing NavigationViewSample.xaml sample, and create a new UI test in SamplesApp.UITests - Windows_UI_Xaml_Controls\NavigationViewTests.

Yep, we develop on master, you can create a bugfix branch from there.

davidjohnoliver picture davidjohnoliver on 3 Jun 2020

I've read the general guidelines at https://github.com/unoplatform/uno/blob/master/doc/articles/contributing/guidelines/creating-tests.md and the auto test guide at https://github.com/unoplatform/uno/blob/master/doc/articles/uno-development/working-with-the-samples-apps.md

A few questions regarding the tests
I've noticed that the default test platform is WASM as defined in constants.cs. Is that the preferred platform to write and run UI tests for? Or should I attrib my test [ActivePlatforms(Platform.Android)] and run it on the android emulator from Uno.UI-Android-only.slnf ? Or should I keep it WASM only and launch it from Uno.UI-Wasm-only.slnf ?

NavigationView is also somewhat special. If navigated to lets say the first item then I need to either hit the hardware back button or the navigationview back button to go back in order to select let's say the settings Item. I could try to tap the screen using a set of coordinates but is that the recommended way to do things like that? I mean what if the rendering is not pixel perfect on every platform? Then the test could fail.

jabak picture jabak on 5 Jun 2020

I've noticed that the default test platform is WASM as defined in constants.cs. Is that the preferred platform to write and run UI tests for? Or should I attrib my test [ActivePlatforms(Platform.Android)] and run it on the android emulator from Uno.UI-Android-only.slnf ? Or should I keep it WASM only and launch it from Uno.UI-Wasm-only.slnf ?

The CurrentPlatform line in Constants.cs and the ActivePlatforms test attribute are two separate things. The CurrentPlatform property basically determines what happens locally when you run/debug a unit test from Visual Studio. For authoring the test, either WASM or Android are valid options, whatever you find more convenient. (iOS is less straightforward because the test needs to be run from a Mac.)

Whichever way you choose to test the test locally, the CI will run it on all 3 currently tested platforms (WASM, Android, iOS). If the test shouldn't run on certain platforms (eg it's testing a platform-specific feature, or there's a known bug causing it to fail on a particular platform), the [ActivePlatforms] attribute can be used to restrict which platforms the CI will run the test for. Most of the time however you'd omit that attribute.

NavigationView is also somewhat special. If navigated to lets say the first item then I need to either hit the hardware back button or the navigationview back button to go back in order to select let's say the settings Item. I could try to tap the screen using a set of coordinates but is that the recommended way to do things like that? I mean what if the rendering is not pixel perfect on every platform? Then the test could fail.

Yes, it's not recommended to do it that way and there's no need to. You should be able to find for the button by name - NavigationViewBackButton - and tap it in the following way:

var backButton = _app.Marked("NavigationViewBackButton");
backButton.FastTap();
davidjohnoliver picture davidjohnoliver on 8 Jun 2020

I've fiddling around with the navigationview test and there are still things I don't quite get. Given the following XAML

<Grid>
    <NavigationView Loaded="NavigationView_Loaded"
                    Margin="0,12,0,0"
                    SelectionChanged="NavigationView_SelectionChanged"
                    ItemInvoked="NvSample_ItemInvoked"
                    xamarin:BackRequested="NvSample_BackRequested"
                    x:Name="nvSample"
                    IsSettingsVisible="true"
                    IsTabStop="False"
                    Windows10version1803:IsBackButtonVisible="Visible"
                    Windows10version1803:PaneTitle="This the pane title"
                    Header="This is header text.">
        <NavigationView.MenuItems>
            <NavigationViewItem x:Name="menuitem1" Content="Menu Item1"
                                Tag="SamplePage1">
                <NavigationViewItem.Icon>
                    <SymbolIcon Symbol="Play" />
                </NavigationViewItem.Icon>
            </NavigationViewItem>
            <NavigationViewItemHeader Content="Actions" />
            <NavigationViewItem x:Name="menuitem2"  Content="Menu Item2"
                                Tag="SamplePage2">
                <NavigationViewItem.Icon>
                    <SymbolIcon Symbol="Save" />
                </NavigationViewItem.Icon>
            </NavigationViewItem>
            <NavigationViewItem x:Name="menuitem3" Content="Menu Item3"
                                Tag="SamplePage3">
                <NavigationViewItem.Icon>
                    <SymbolIcon Symbol="Refresh" />
                </NavigationViewItem.Icon>
            </NavigationViewItem>
        </NavigationView.MenuItems>

        <NavigationView.AutoSuggestBox>
            <AutoSuggestBox x:Name="controlsSearchBox"
                            VerticalAlignment="Center"
                            PlaceholderText="Search"
                            QueryIcon="Find"
                            RequestedTheme="Light">
            </AutoSuggestBox>
        </NavigationView.AutoSuggestBox>

        <Frame x:Name="contentFrame">
            <Frame.ContentTransitions>
                <TransitionCollection>
                    <NavigationThemeTransition>
                        <NavigationThemeTransition.DefaultNavigationTransitionInfo>
                            <EntranceNavigationTransitionInfo />
                        </NavigationThemeTransition.DefaultNavigationTransitionInfo>
                    </NavigationThemeTransition>
                </TransitionCollection>
            </Frame.ContentTransitions>
        </Frame>
    </NavigationView>

</Grid>

I would expect this to work:

Run("SamplesApp.Samples.NavigationViewSample.NavigationViewSample");

_app.WaitForElement(_app.Marked("nvSample"));

var secondMenuItem = _app.Marked("menuitem2");
secondMenuItem.FastTap();

var backButton = _app.Marked("NavigationViewBackButton");
backButton.FastTap();

var firstMenuItem = _app.Marked("menuitem1");
firstMenuItem.FastTap();

But menuitem2 is null when _app.Marked("menuitem2") returns.

If I do this :

Run("SamplesApp.Samples.NavigationViewSample.NavigationViewSample");

_app.WaitForElement(_app.Marked("nvSample"));

var firstMenuItem = _app.Marked("menuitem1");
firstMenuItem.FastTap();

then firstMenuItem is return correctly and FastTap() works for that item. I must be doing something wrong.

jabak picture jabak on 8 Jun 2020

@jabak I'm not sure why it would be working for the first item and not the second item in that case, I'm afraid. :(

davidjohnoliver picture davidjohnoliver on 9 Jun 2020

No problem. I will try to figure it out by using my own simple repro of the original problem.

jabak picture jabak on 9 Jun 2020

There's definitely something weird going on here. To inspect the visual tree I added _app.Repl() to the UI test. From within the console I issued the 'tree' command.

The first menuitem and the settingsitem have the correct label as defined in the xaml. Second item and third item have no label.

Dump from console tree command:

                                    [NavigationViewItem] label: "menuitem1"
                                      [NavigationViewItemPresenter] label: "NavigationViewItemPresenter"
                                        [Grid] label: "LayoutRoot"
                                          [Grid]
                                            [Rectangle] label: "SelectionIndicator"
                                          [Border] label: "RevealBorder"
                                          [Grid] label: "ContentGrid"
                                            [Viewbox > Border] label: "IconBox"
                                              [ContentPresenter > ... > TextBlock] label: "Icon"
                                            [ContentPresenter > ImplicitTextBlock] label: "ContentPresenter"
                                    [NavigationViewItemHeader > Grid]
                                      [Grid] label: "InnerHeaderGrid"
                                        [TextBlock] label: "HeaderText"
                                    [NavigationViewItem]
                                      [NavigationViewItemPresenter] label: "NavigationViewItemPresenter"
                                        [Grid] label: "LayoutRoot"
                                          [Grid]
                                            [Rectangle] label: "SelectionIndicator"
                                          [Border] label: "RevealBorder"
                                          [Grid] label: "ContentGrid"
                                            [Viewbox > Border] label: "IconBox"
                                              [ContentPresenter > ... > TextBlock] label: "Icon"
                                            [ContentPresenter > ImplicitTextBlock] label: "ContentPresenter"
                                    [NavigationViewItem]
                                      [NavigationViewItemPresenter] label: "NavigationViewItemPresenter"
                                        [Grid] label: "LayoutRoot"
                                          [Grid]
                                            [Rectangle] label: "SelectionIndicator"
                                          [Border] label: "RevealBorder"
                                          [Grid] label: "ContentGrid"
                                            [Viewbox > Border] label: "IconBox"
                                              [ContentPresenter > ... > TextBlock] label: "Icon"
                                            [ContentPresenter > ImplicitTextBlock] label: "ContentPresenter"
                              [NavigationViewItem] label: "SettingsNavPaneItem"
                                [NavigationViewItemPresenter] label: "NavigationViewItemPresenter"
                                  [Grid] label: "LayoutRoot"
                                    [Grid]
                                      [Rectangle] label: "SelectionIndicator"
                                    [Border] label: "RevealBorder"
                                    [Grid] label: "ContentGrid"
                                      [Viewbox > Border] label: "IconBox"
                                        [ContentPresenter > ... > TextBlock] label: "Icon"
                                      [ContentPresenter > ImplicitTextBlock] label: "ContentPresenter"
jabak picture jabak on 9 Jun 2020

Hmmm I think I've misunderstood how I to prepare the UI test runs. Running an UI test doesn't deploy the sample app it self. So if I make changes to the sample app I've have to debug it once before running any UI tests again. That's what threw me off.

jabak picture jabak on 9 Jun 2020
👍1

Getting closer. I've managed to flesh out working navigation code. For some reason the back button doesn't work (maybe that's a bug and my next contribution). So I decided to use the TogglePaneButton to get from one menu item to the next. The only thing left is to test if the correct page (Page 1, Page 2 or Page 3) is loaded when tapping the menu item. According to xamarin-uitest-sdk documentation AppQuery should be able to search for views whose text property contains for example "Page 2". But that doesn't work. Could be that the the harness is not fully implemented?

So I still need some way to verify that Page 1, Page 2 and Settings page gets loaded and displayed when the corresponding menuitem is tapped. Any suggestions?

Code that navigates:

Run("SamplesApp.Samples.NavigationViewSample.NavigationViewSample");

_app.WaitForElement(_app.Marked("nvSample"));

var secondMenuItem = _app.Marked("menuitem2");
secondMenuItem.FastTap();

var togglePaneButton = _app.Marked("TogglePaneButton");
togglePaneButton.FastTap();

var settingsItem = _app.Marked("SettingsNavPaneItem");
settingsItem.FastTap();

togglePaneButton.FastTap();

var firstMenuItem = _app.Marked("menuitem1");
firstMenuItem.FastTap();

jabak picture jabak on 9 Jun 2020

Easiest is to just put a distinctive x:Name on one of the elements in the page, eg <TextBlock x:Name="Page2NavViewContent" Text="Page 2"/>, then call _app.WaitForElement("Page2NavViewContent") in the test.

WaitForElement() will time out and fail the test if the element isn't there.

davidjohnoliver picture davidjohnoliver on 10 Jun 2020
👍1

Okay... finally got the PR done. But it failed a Conventional Commits check??

jabak picture jabak on 11 Jun 2020

Okay I completely screwed up my local fork consequently also the pull request. Tried to follow some guidance on how to amend the commit comments in order to correct them so they would pass Conventional Commits. But it just made things worse and I just couldn't unravel things.

So I'll just start all over again.

Lesson learned: be patient with git and github and try not to do something you don't fully understand.

jabak picture jabak on 12 Jun 2020

Can this be closed?

francoistanguay picture francoistanguay on 29 Aug 2020

In my opinion yes.

jabak picture jabak on 30 Aug 2020
Was this page helpful?
0 / 5 - 0 ratings

Related issues

MartinZikmund picture MartinZikmund  路  4Comments
MatFillion picture MatFillion  路  4Comments
carlh98 picture carlh98  路  3Comments
JanabiSoft picture JanabiSoft  路  4Comments
jeromelaban picture jeromelaban  路  3Comments