-
Notifications
You must be signed in to change notification settings - Fork 706
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
Fix NavigationView light dismiss issue #2211
Fix NavigationView light dismiss issue #2211
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if we should a test for this. Since we apparently missed transitioning to a visual state, we might want to ensure that this behavior is reliable.
@chingucoding Good point. Added an interaction test now. |
@Felix-Dev can you please merge with master which includes some stability fixes for the PR pipeline ? |
@ranjeshj Done. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description
Fixes the NavigationView light dismiss issue where the NavigationViewItems would not work with the correct closed pane width.
Implementation details
I removed the check if the navigation pane (a SplitView pane) is closed. When we close the pane using the pane toggle button (or set SplitView.IsPaneOpen to false), we were already entering the ListSizeCompact visual state. In other words, splitView.IsPaneOpen() returned false. However, if the pane was light dismissed, splitView.IsPaneOpen() returned true at this point, thus not entering the ListSizeCompact visual state, causing the issue to manifest.
Now that the check is removed, we enter this visual state no matter how the navigation pane is closed . And since we are only calling this code in a handler for the event when the pane is closing (OnSplitViewPaneClosing) having removed this check should not introduce any other side-effects.
Motivation and Context
Fixes #2208
How Has This Been Tested?
Tested visually.
Screenshots
See the following GIF: