Skip to content
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

Conversation

Felix-Dev
Copy link
Contributor

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:

navviewpane_issue_fix

@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label Apr 2, 2020
@ranjeshj ranjeshj added area-NavigationView NavView control team-Controls Issue for the Controls team labels Apr 2, 2020
@ranjeshj ranjeshj requested a review from ojhad April 2, 2020 11:23
Copy link
Collaborator

@marcelwgn marcelwgn left a 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.

@Felix-Dev
Copy link
Contributor Author

@chingucoding Good point. Added an interaction test now.

@ranjeshj ranjeshj removed the needs-triage Issue needs to be triaged by the area owners label Apr 2, 2020
@ranjeshj
Copy link
Contributor

ranjeshj commented Apr 3, 2020

@Felix-Dev can you please merge with master which includes some stability fixes for the PR pipeline ?

@Felix-Dev
Copy link
Contributor Author

@ranjeshj Done.

@ranjeshj
Copy link
Contributor

ranjeshj commented Apr 3, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@StephenLPeters StephenLPeters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@StephenLPeters StephenLPeters merged commit 81fa8b0 into microsoft:master Apr 3, 2020
@Felix-Dev Felix-Dev deleted the user/Felix-Dev/navigationview-light-dismiss-issue branch April 3, 2020 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-NavigationView NavView control team-Controls Issue for the Controls team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NavigationView: Pane light dismiss does not adjust item width
5 participants