Make consistency behaviour when clicking on a unselected item or clicking on a selected item

[licanhua]

Fix #573
NavigationView has different logic for clicking on a selected item and click on unselected item.
We try to close the pane when clicking on unselected item which is implemented in ChangeSelection.
When clicked on a selected item, ChangeSelection is not called, but we have special logic in OnItemClick to know about it and raise ItemInvoked event to customer.
This fix would try to close pane for both cases, so I extract the code to ClosePaneIfNeccessaryAfterItemIsClicked, and call it in both ChangeSelection and OnItemClick.

[kaiguo]

The fix looks good me. I guess my question is why selected and unselected item clicks go to different function? Can they be handled together in ChangeSelection?

[licanhua]

The fix looks good me. I guess my question is why selected and unselected item clicks go to different function? Can they be handled together in ChangeSelection?
No, we can't.

Background:
Generally speaking, ItemInvoked and SelectionChanged event are raised from ListView, then NavigationView re-raise them to external customer.
In RS5, we introduced a lot of new features like SelectsOnInvoked, SelectionFollowFocus, and it makes the logic very complex. (see comments on NavigationView::OnSelectionChanged), during the refactoring, we delayed the itemInvoked to selectionchanged if it's not clicking on selecteditem. In this way, the mapping table is controlled inside SelectionChanged only, and it would be easy to read(reality is that I didn't see significant benefit).

So event looking like this:
Click on selected item -> ListView raise ItemInvoked -> NavView raise ItemInvoked
Click on un-selected item -> ListView raise ItemInvoked, and NavView ignore it -> ListView continue raise SelectionChanged event -> NavView handle selectionChanged -> NavView raise ItemInvoke -> NavView raise SelectionChanged.

When clicking on selected item, listview doesn't raise selectionchanged, so we can't move everything to ChangeSelection.

[kaiguo]

The fix looks good me. I guess my question is why selected and unselected item clicks go to different function? Can they be handled together in ChangeSelection?
No, we can't.

Background:
Generally speaking, ItemInvoked and SelectionChanged event are raised from ListView, then NavigationView re-raise them to external customer.
In RS5, we introduced a lot of new features like SelectsOnInvoked, SelectionFollowFocus, and it makes the logic very complex. (see comments on NavigationView::OnSelectionChanged), during the refactoring, we delayed the itemInvoked to selectionchanged if it's not clicking on selecteditem. In this way, the mapping table is controlled inside SelectionChanged only, and it would be easy to read(reality is that I didn't see significant benefit).

So event looking like this:
Click on selected item -> ListView raise ItemInvoked -> NavView raise ItemInvoked
Click on un-selected item -> ListView raise ItemInvoked, and NavView ignore it -> ListView continue raise SelectionChanged event -> NavView handle selectionChanged -> NavView raise ItemInvoke -> NavView raise SelectionChanged.

When clicking on selected item, listview doesn't raise selectionchanged, so we can't move everything to ChangeSelection.

Thanks for clarifying!

[msft-github-bot]

🎉Microsoft.UI.Xaml v2.1.190606001 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes
• Official Nuget Release