-
Notifications
You must be signed in to change notification settings - Fork 742
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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. #14668
Comments
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? |
You are right about not being able to reproduce it. I remember having reproduced it well... |
Closing for now. We can reopen if there are new findings. |
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. |
@ADD-Noureddine-Maachi based on the details here microsoft/microsoft-ui-xaml#9185 (comment), are you still experiencing this issue lately? |
@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 |
Here a file to reproduce the issue: |
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
So TL;DR is
private void Page_Loaded(object sender, RoutedEventArgs e)
{
DispatcherQueue.TryEnqueue(async () =>
{
await ViewModel.LoadAsync();
});
} |
@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. |
@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. |
@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 [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 [RelayCommand]
private async Task LoadNavigationOtionsAsync()
{
IReadOnlyCollection<NavigationOption> allowedNavigationOptions = await GetAllowedNavigationOptionsAsync();
NavigationOptions = new ObservableCollection<NavigationOption>(allowedNavigationOptions);
navigationView.UpdateLayout();
SelectedNavigationOption = NavigationOptions.First();
} |
@ramezgerges First option is not feasible because the ViewModel is UI Framework agnostic so it cannot use
|
@ADD-Noureddine-Maachi When you say "Microsoft solves it", do you mean that this repro works correctly for you on WinUI? If |
Hi @ADD-Noureddine-Maachi, I hope you're doing well! Have you had a chance to review @ramezgerges' question? |
@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. |
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. |
Let's create an issue on the microsoft side, we'll track it with Microsoft. |
Current behavior
When the default NavigationView's SelectedItem is setted in the View Model, the SelectionChanged event is invoked correctly but:
Once you change tab, then it works as spected (IsSelected value is setted to true).
A workaround in Windows is to set the IsSelected value to true when the SelectionChanged event is invoked.
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
The text was updated successfully, but these errors were encountered: