NavigationViewItem's IsSelected property doesn't update to true when NavigationView's SelectedItem is updated in the View Model. This is causing VisualStates to not work as expected.

[Suriman]

Current behavior

When the default NavigationView's SelectedItem is setted in the View Model, the SelectionChanged event is invoked correctly but:

• When obtaining the SelectedItemContainer (through event arguments), the IsSelected value is still false.
  Once you change tab, then it works as spected (IsSelected value is setted to true).
• The SelectedItemContainer (through event arguments) is null for WPF & WASM. Only Windows is returning the container.
  A workaround in Windows is to set the IsSelected value to true when the SelectionChanged event is invoked.
• This behavior causes that VisualStates don´t work as expected.

Expected behavior

IsSelected value should be setted true when setting the NavigationView's SelectedItem through the View Model. This should solve VisualStates current behavior.

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

How to reproduce it:
1 - Open the attached project and compile it.
2 - Execute the Windows, WPF & WASM projects and debug the SelectionChanged event in MainPage's code behind (MainPage.xaml.cs).
There you can see that the event is invoked but the property IsSelected is not setted to true.
With WPF & WASM you should see that the SelectedItemContainer is returning null.

Workaround

Works on UWP/WinUI: Yes, but with the workaround.

Works on UWP/WinUI

Yes

Environment

Uno.UI / Uno.UI.WebAssembly / Uno.UI.Skia

NuGet package version(s)

NuGet package(s): Uno.WinUI, Uno.Material.WinUI
NuGet package version(s): Uno.WinUI 5.0.48, Uno.Material.WinUI 4.0.6

Affected platforms

WebAssembly, Skia (WPF), Windows

IDE

Visual Studio 2022

IDE version

17.8.1

Relevant plugins

No response

Anything else we need to know?

NavigationViewItemIsSelectedValue.zip

[ramezgerges]

I can't reproduce this. The container is not null. The container has IsSelected = False inside the event handler but this is the case on both Windows (WinUI) and our Uno targets (WPF, Wasm, etc.). We attempt to match the behaviour on WinUI as closely as we can, so if you see the same behaviour (IsSelected = False) on both Windows and WPF, it's not a bug. @Suriman can you please check again and revise the issue?

[ADD-Noureddine-Maachi]

You are right about not being able to reproduce it. I remember having reproduced it well...
I'll check it out and tell you something. Thanks in advance.

[ramezgerges]

Closing for now. We can reopen if there are new findings.

[ADD-Noureddine-Maachi]

Reviewing it, we have seen that it seems to be a WinUI bug and we have reported it to Microsoft. You can follow the progress here.

[agneszitte]

@ADD-Noureddine-Maachi based on the details here microsoft/microsoft-ui-xaml#9185 (comment), are you still experiencing this issue lately?

[ADD-Noureddine-Maachi]

@agneszitte This issue is still happening. I will try to prepare an update repo for you so you can see it better. The main issue here is, setting the SelectedItem from a NavigationOption from the view model simulating a 5 seconds background work, does not select anything at all.

[Suriman]

Here a file to reproduce the issue:
NavigationViewItemIsSelectedIssue.zip

[ramezgerges]

Edit: The suggestions in this comments are outdated. Further investigation showed that our behaviour is actually similar to WinUI, it's just that the timing is supposed to be non-deterministic and it just so happens that most of the time, the sequence of interleaving threads will behave a certain way. You should follow Nick's advice below and mostly ignore this comment. The only important part is to not set the DataContext yourself.

@Suriman Hello. I can repro the problem. It seems to be a timing problem between Loaded and setting the DataContext by the navigation feature.

• You're not supposed to set DataContext yourself. It will be done for you by Uno.Extension's navigation feature. For more, see https://platform.uno/blog/uno-extensions-navigation-guide.
• Part of the navigation logic sets the DataContext to null at some point to ensure a "fresh" view model. This shouldn't be a problem because you're not supposed to be creating the view model and setting it as the data context yourself. This happens on both WinUI and Uno.
• Later, the DataContext is correctly set to a newly constructed MainViewModel instance. This makes the DataContext that you created irrelevant, since it gets replaced with a new instance. This also happens on both Uno and WinUI. The only difference is the timing. On Uno, I'm seeing
  • DataContext set to null
  • a new MainViewModel is created
  • Loaded
  • DataContext set to the MainViewModel
    whereas on WinUI, I'm seeing
  • DataContext set to null
  • a new MainViewModel is created
  • DataContext set to a MainViewModel
  • Loaded
• Oddly enough, if I create a new uno app with dotnet new unoapp -o navigationdatacontextnullrepro -preset "recommended" -platforms "windows" -platforms "desktop" -theme-service False -vscode False -presentation "mvvm" -di -toolkit False -dsp False -pub "navigationdatacontextnullrepro", I see the same problem on WinUI too. The only difference I'm seeing is that the repro in this issue uses a parameterless constructor in MainViewModel, while the newly created app I tried uses a public MainViewModel( IStringLocalizer localizer, IOptions<AppConfig> appInfo, INavigator navigator) constructor.

So TL;DR is

• Do not construct your own view models and don't set the DataContext manually.
• To work around the timing inconsistency, enqueue the logic of Loaded on the dispatcher, like this:

    private void Page_Loaded(object sender, RoutedEventArgs e)
    {
        DispatcherQueue.TryEnqueue(async () =>
        {
            await ViewModel.LoadAsync();
        });
    }

[nickrandolph]

@Suriman with Navigation in Uno.Extensions we wanted to try to avoid developers needing to hook lifecycle events such as Loaded and NavigatedTo but I understand the desire to trigger asynchronous loading. For this, my suggestion would be to capture the DataContextChanged event, where the DataContext will be set to the instance of your viewmodel once it's been created.

[Suriman]

@nickrandolph, we do not use Uno.Extensions in our product, we use Prism.Regions, that's why we think that this has nothing to do with the issue.

We have updated our project so it can be easier to reproduce the issue. Now, we have introduced a button which does the load work (see the .zip file attached to this comment).

Inside the execution code of its Command, you will see that there is now a commented line of code (a delay of 1 milisecond). If you uncomment it, rerun it and click at the button, you will see that the NavigationView control correctly sets the SelectedItem as expected.

We think that the control is not waiting to have loaded all the options from the ItemsSource before selecting the SelectedItem.

NavigationViewItemIsSelectedIssue_v2.zip

[ramezgerges]

@Suriman I've tested the repro and it reproduces on both the WinUI and Desktop targets, so it's not really an Uno bug. Regardless, I've investigated and the problem is that the NavigationView (and its internal ItemsRepeater) need a layout cycle to actually materialize the items from the items source, so when you set the items source and then directly set the selected items, it doesn't work because the there aren't any materialized items to select yet. So we need a way to insert a layout cycle between setting the MenuItemsSource and the SelectedItem. You can do that by adding a delay like you did, or more cleanly by enqueuing the setting of the SelectedItem on the dispatcher queue, like so:

[RelayCommand]
    private async Task LoadNavigationOtionsAsync()
    {
        IReadOnlyCollection<NavigationOption> allowedNavigationOptions = await GetAllowedNavigationOptionsAsync();

        NavigationOptions = new ObservableCollection<NavigationOption>(allowedNavigationOptions);

        DispatcherQueue.GetForCurrentThread().TryEnqueue(() => SelectedNavigationOption = NavigationOptions.First());
    }

Alternatively, you can add a reference to the NavigationView instance in the view model and call UpdateLayout on it, i.e.

[RelayCommand]
    private async Task LoadNavigationOtionsAsync()
    {
        IReadOnlyCollection<NavigationOption> allowedNavigationOptions = await GetAllowedNavigationOptionsAsync();

        NavigationOptions = new ObservableCollection<NavigationOption>(allowedNavigationOptions);

        navigationView.UpdateLayout();

        SelectedNavigationOption = NavigationOptions.First();
    }

[ADD-David-Antolin]

@ramezgerges First option is not feasible because the ViewModel is UI Framework agnostic so it cannot use DispatcherQueue that only exists on WinUI/Uno Platform. The second one breaks the MVVM pattern. The ViewModel cannot have a reference to a View element. I see two options:

1. Microsoft solves it inside the control code. If SelectedItem is set while there is an ItemSource but not all items materialized, the control private logic should wait till they are before attempting to select it (or try select again after they are materialized).
2. Creating a Behavior<NavigationView> that does the middle man (bind SelectedItem to the behavior instead of the NavigationView) and implement one of the solutions that you proposed but inside the View layer.

[ramezgerges]

@ADD-Noureddine-Maachi When you say "Microsoft solves it", do you mean that this repro works correctly for you on WinUI? If
it does, then it's a problem. If it doesn't, then I'm a bit confused.

[agneszitte]

@ADD-Noureddine-Maachi When you say "Microsoft solves it", do you mean that this repro works correctly for you on WinUI? If it does, then it's a problem. If it doesn't, then I'm a bit confused.

Hi @ADD-Noureddine-Maachi, I hope you're doing well! Have you had a chance to review @ramezgerges' question?
Your feedback would be greatly appreciated. Thank you in advance!

[ADD-David-Antolin]

@ramezgerges I was the one that answered you. The problem affects WinUI too, because that I said that it is Microsoft who should solve it, because the NavigationView can control internally if an item is being selected while an ItemSource is being materialized. Any other solution would be suboptimal.

[jeromelaban]

Thanks @ADD-Noureddine-Maachi. If it is indeed a Microsoft behavior issue, there will need to be a change directly in the WinUI repository that we can replicate afterwards.

[jeromelaban]

Let's create an issue on the microsoft side, we'll track it with Microsoft.