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

NavigationView incorrectly sizing items as compact on launch #2810

Merged
merged 1 commit into from
Jul 13, 2020

Conversation

ojhad
Copy link
Contributor

@ojhad ojhad commented Jul 1, 2020

Description

NavigationView was launching into expanded mode while the items were being sized as compact.
This is happening due to the pane being closed and opened during the set up. This should not be happening while internal logic is setting up the correct state. Used an existing flag to ensure this.

Motivation and Context

Fixes #2635

How Has This Been Tested?

Manual testing

@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label Jul 1, 2020
@marcelwgn
Copy link
Collaborator

Should we add a test for this?

@ranjeshj ranjeshj added area-NavigationView NavView control team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Jul 2, 2020
@StephenLPeters
Copy link
Contributor

StephenLPeters commented Jul 6, 2020

Should we add a test for this?

I don't think we have a consistent repro which would make effectively testing it hard. @ojhad

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ojhad
Copy link
Contributor Author

ojhad commented Jul 6, 2020

Should we add a test for this?

Not having a consistent repro bundled with the fact that this scenario is only hit in a specific configuration upon app launch makes the test setup for this require a lot more work than normal, which is why I left out writing a test for this for now.

@marcelwgn
Copy link
Collaborator

Not having a consistent repro bundled with the fact that this scenario is only hit in a specific configuration upon app launch makes the test setup for this require a lot more work than normal, which is why I left out writing a test for this for now.

Fair enough, if the test would require a lot of work or wouldn't be reliable anyway, I guess in this case a test won't be necessary or helpful.

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

if (IsPaneOpen() &&
DisplayMode() != winrt::NavigationViewDisplayMode::Expanded &&
!DoesNavigationViewItemHaveChildren(selectedContainer) &&
!m_shouldIgnoreNextSelectionChange)
Copy link
Contributor

@ranjeshj ranjeshj Jul 13, 2020

Choose a reason for hiding this comment

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

Is this an issue with selection model raising events when it should not be ? If so, lets file a bug for it.

@ranjeshj ranjeshj merged commit 791d14f into master Jul 13, 2020
@ranjeshj ranjeshj deleted the user/ojhad/incorrectitemsizingfix branch July 13, 2020 20:55
Kinnara added a commit to Kinnara/ModernWpf that referenced this pull request Jul 14, 2020
@msft-github-bot
Copy link
Collaborator

🎉Microsoft.UI.Xaml v2.5.0-prerelease.200812001 has been released which incorporates this pull request.:tada:

Handy links:

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 occasionally loads in Compact form while reporting IsPaneOpen==True
5 participants