-
Notifications
You must be signed in to change notification settings - Fork 669
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
NavigationView: Fix SelectedItem not being unset when no NavigationViewItem is selected #2329
NavigationView: Fix SelectedItem not being unset when no NavigationViewItem is selected #2329
Conversation
@ranjeshj VS seems to suggest you worked on this test so I'll ask you: Is the |
Co-Authored-By: Marcel Wagner <marcel.alex.wagner@outlook.com>
I think the idea is that we verify that the selection is also correct when selectionchanged got raised. |
@chingucoding I just don't think we need to check whether the selection is correct twice. If we keep the check in the SelectionChanged event handler I don't think we need the other check (as we fail if event was not raised). |
We check if the selection is correct after we changed it and we also check if the selection is correct inside the SelectionChanged event. Techincally there could be a missmatch for example when we raise SelectionChanged to early. |
That does look redundant. One thing this might be checking is that the timing is correct - that the selection state has been updated before raising the event and checking the state in the event handler gives us the expected values. If that is not covered by any of the other tests, then a comment might be useful. If that is being covered elsewhere, we can remove the check. Thanks. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
I kept the SelectionChanged event handler ValidateSelection checks and removed the specific |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Description
Fix issue where NavView's SelectedItem property was not set to null when all navigation view items are no longer selected.
Motivation and Context
Fixes #957
How Has This Been Tested?
Added API test.