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 NavView issue with SelectionChanged being raised upon displaymode change #1956

Merged

Conversation

marcelwgn
Copy link
Collaborator

@marcelwgn marcelwgn commented Feb 8, 2020

Description

Changed code where we update the settings item to set the m_shouldIgnoreNextSelectionChange flag to true, so SelectionChanged ignores that selection change. Also added a check for m_shouldIgnoreNextSelectionChange in ChangeSelection() .

Motivation and Context

Fixes #148 , Fixes #1976, Fixes #1956

How Has This Been Tested?

Added new interaction test.

Screenshots (if appropriate):

@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label Feb 8, 2020
@marcelwgn marcelwgn changed the title Transfer changes from #1395 Fix NavView issue with SelectionChanged being raised upon displaymode change Feb 9, 2020
@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StephenLPeters StephenLPeters 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 Feb 11, 2020
@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ad1Dima
Copy link
Contributor

ad1Dima commented Feb 20, 2020

Hmm, my #1997 is also fixing #148 (by adding settings in FooterMenuItems)
I guess there may be conflict between this PR's

@ojhad
Copy link
Contributor

ojhad commented Mar 6, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StephenLPeters
Copy link
Contributor

@chingucoding looks like some of the tests are real... I can't imagine the acrylic brush failures are related but maybe the nav view failures put us in a bad state..

@marcelwgn
Copy link
Collaborator Author

@StephenLPeters , I fixed the bug causing the NavigationView test failures. Regarding the Acrylic tests, I guess it is because of the failing NavigationView tests.

Strangely, the Acrylic test failures also do not show up on the error list in the "Checks" tab of this PR, they just show up in the Azure portal (well at least on my machine/account).

@ranjeshj
Copy link
Contributor

ranjeshj commented Mar 7, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ranjeshj ranjeshj merged commit 2164f0a into microsoft:master Mar 9, 2020
@marcelwgn marcelwgn deleted the navview-displaymode-eventargsnull-bug branch April 7, 2020 18:03
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
7 participants