Add New NavView VisualStateGroup #4659

karkarl · 2021-03-26T06:13:46Z

Description

Add PaneOverlayGroup to control PaneOverlay and PaneNotOverlay vistual states
Set background to transparent when in compact closed, and acrylic when in compact overlay
Lift SplitView from WUXC to add cornerRadius templateBindings to PaneRoot
Set OverlayCornerRadius to SplitView during PaneOverlay state
Update NavView pane border to have rounded corners on overlay

Motivation and Context

How Has This Been Tested?

Visual verification

Screenshots (if appropriate):

dev/CommonStyles/Common_themeresources.xaml

karkarl · 2021-03-26T23:07:05Z

/azp run

azure-pipelines · 2021-03-26T23:07:16Z

Azure Pipelines successfully started running 1 pipeline(s).

karkarl · 2021-03-29T18:30:41Z

/azp run

azure-pipelines · 2021-03-29T18:30:53Z

Azure Pipelines successfully started running 1 pipeline(s).

karkarl · 2021-03-31T17:06:59Z

/azp run

azure-pipelines · 2021-03-31T17:07:11Z

Azure Pipelines successfully started running 1 pipeline(s).

dev/NavigationView/NavigationView.xaml

StephenLPeters · 2021-03-31T18:57:40Z

dev/NavigationView/NavigationView.xaml

+                                        </Storyboard>
+                                    </VisualTransition>
+                                </VisualStateGroup.Transitions>
+                                <VisualState x:Name="PaneOverlay">


These VisualState names don't seem correct to me. @ranjeshj and @MikeHillberg for naming, maybe PaneOverlaying and PaneNotOverlaying

dev/NavigationView/NavigationView.xaml

dev/NavigationView/NavigationView.cpp

karkarl · 2021-03-31T23:20:30Z

/azp run

azure-pipelines · 2021-03-31T23:20:41Z

Azure Pipelines successfully started running 1 pipeline(s).

ranjeshj · 2021-04-01T02:54:09Z

@karenbtlai you need to merge in master to fix the key mismatch errors.

karkarl · 2021-04-01T15:52:13Z

/azp run

azure-pipelines · 2021-04-01T15:52:23Z

Azure Pipelines successfully started running 1 pipeline(s).

karkarl · 2021-04-01T17:28:04Z

/azp run

azure-pipelines · 2021-04-01T17:28:16Z

Azure Pipelines successfully started running 1 pipeline(s).

licanhua · 2021-04-01T22:10:01Z

dev/NavigationView/NavigationView.cpp

+                if (splitView.DisplayMode() == winrt::SplitViewDisplayMode::CompactOverlay || splitView.DisplayMode() == winrt::SplitViewDisplayMode::Overlay)
+                {
+                    winrt::VisualStateManager::GoToState(*this, L"PaneNotOverlay", true /*useTransitions*/);
+                }


It's very comfusion here.
From your code, Likely PaneNotOverlay and PaneOverlay only happens when DisplayMode is CompactOverlay or Overlay.
So we should have 3 visual states other than 2:

DisplayMode is not CompactOverlay or Overlay

DisplayMode is ?Overlay, and Pane is closed

DisplayMode is ?Overlay, and Pane is opened

Don't use PaneNotOverlay/Overlay, it's easy to mess up with the term: CompactOverlay or Overlay in splitview setting.

It's better to have the LeftNav in the name, for example: Pane???OnLeftNavigationView

Added you to the email thread discussing the naming

StephenLPeters · 2021-04-01T22:11:07Z

static constexpr auto c_shadowCasterEaseOutStoryboard = L"ShadowCasterEaseOutStoryboard"sv;

Looks like these storyboards aren't referenced from the code behind anymore. Can we delete this? If so, can we remove the name from the storyboard definition as well? (We can only remove the name if we haven't published the storyboard in a release yet)

Refers to: dev/NavigationView/NavigationView.cpp:101 in 3e2d182. [](commit_id = 3e2d182, deletion_comment = False)

StephenLPeters · 2021-04-01T22:13:01Z

            UnsetDropShadow();

Don't we need to keep the shadow around until the transition animation has finished?

Refers to: dev/NavigationView/NavigationView.cpp:4086 in 3e2d182. [](commit_id = 3e2d182, deletion_comment = False)

dev/NavigationView/NavigationView.xaml

karkarl · 2021-04-02T19:27:21Z

            UnsetDropShadow();
Don't we need to keep the shadow around until the transition animation has finished?

Refers to: dev/NavigationView/NavigationView.cpp:4086 in 3e2d182. [](commit_id = 3e2d182, deletion_comment = False)

You're right. I reverted the bit with the event listener.

karkarl · 2021-04-02T19:27:45Z

/azp run

azure-pipelines · 2021-04-02T19:27:55Z

Azure Pipelines successfully started running 1 pipeline(s).

dev/NavigationView/NavigationView.cpp

karkarl · 2021-04-02T23:54:01Z

/azp run

azure-pipelines · 2021-04-02T23:54:28Z

Azure Pipelines successfully started running 1 pipeline(s).

StephenLPeters

karkarl · 2021-04-05T17:50:49Z

/azp run

azure-pipelines · 2021-04-05T17:51:00Z

Azure Pipelines successfully started running 1 pipeline(s).

licanhua · 2021-04-05T20:17:34Z

dev/NavigationView/NavigationView.xaml

+                                        </Storyboard>
+                                    </VisualTransition>
+                                </VisualStateGroup.Transitions>
+                                <VisualState x:Name="PaneOverlaying" />


Please do a testing that if the default NavigationVIew is set to minimal or compact in Xaml, will PaneNotOverlaying
code still called?
Because your default state is PaneOverlaying, and I want to make sure PaneNotOverlaying working in some default settings.

Yes it does. I confirmed it with different test pages where minimal and compact is the default.

ghost · 2021-05-03T20:04:35Z

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

Handy links:

ghost added the needs-triage Issue needs to be triaged by the area owners label Mar 26, 2021

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 Mar 26, 2021

ranjeshj requested review from teaP and YuliKl March 26, 2021 15:43

ranjeshj reviewed Mar 26, 2021

View reviewed changes

dev/CommonStyles/Common_themeresources.xaml Outdated Show resolved Hide resolved

StephenLPeters reviewed Mar 31, 2021

View reviewed changes

dev/NavigationView/NavigationView.xaml Show resolved Hide resolved

StephenLPeters reviewed Mar 31, 2021

View reviewed changes

dev/NavigationView/NavigationView.xaml Outdated Show resolved Hide resolved

StephenLPeters reviewed Mar 31, 2021

View reviewed changes

dev/NavigationView/NavigationView.xaml Outdated Show resolved Hide resolved

StephenLPeters reviewed Mar 31, 2021

View reviewed changes

dev/NavigationView/NavigationView.cpp Outdated Show resolved Hide resolved

karkarl added 9 commits April 1, 2021 08:48

add OverlayStateGroup and logic

08c2102

lift SplitView and add cornerradius

21a0f51

update border to have cornerRadius on overlay

cadf4ad

move shadow storyboards into new visualStateGroup

63ad9c9

update verification files

6fe13fd

fix failing tests

6a0f74c

changes from comments

866a673

changes from comments

b83cbc2

fix formatting

47f4a3a

karkarl force-pushed the user/karenlai/NavViewOverlayStateGroup branch from dacd169 to 47f4a3a Compare April 1, 2021 15:51

fix failing tests

3e2d182

licanhua reviewed Apr 1, 2021

View reviewed changes

StephenLPeters reviewed Apr 1, 2021

View reviewed changes

dev/NavigationView/NavigationView.xaml Outdated Show resolved Hide resolved

karkarl added 2 commits April 2, 2021 10:46

remove shadowCaster storyboard references

be083c5

add back eventlistener for easeOutStoryboard

e2d0871

StephenLPeters reviewed Apr 2, 2021

View reviewed changes

dev/NavigationView/NavigationView.cpp Show resolved Hide resolved

remove unused tracker_ref

f1cc6b8

StephenLPeters approved these changes Apr 3, 2021

View reviewed changes

karkarl added 2 commits April 5, 2021 10:41

cover all display modes with PaneOverlayGroup

118ef29

update visualState names

62369f0

licanhua reviewed Apr 5, 2021

View reviewed changes

karkarl merged commit aadd350 into master Apr 5, 2021

karkarl deleted the user/karenlai/NavViewOverlayStateGroup branch April 5, 2021 21:47

Kinnara added a commit to Kinnara/ModernWpf that referenced this pull request Dec 6, 2023

Port microsoft/microsoft-ui-xaml#4659

d23f225

Kinnara added a commit to Kinnara/ModernWpf that referenced this pull request Dec 6, 2023

Port microsoft/microsoft-ui-xaml#4659 (#639)

691d74d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add New NavView VisualStateGroup #4659

Add New NavView VisualStateGroup #4659

karkarl commented Mar 26, 2021 •

edited

Loading

karkarl commented Mar 26, 2021

azure-pipelines bot commented Mar 26, 2021

karkarl commented Mar 29, 2021

azure-pipelines bot commented Mar 29, 2021

karkarl commented Mar 31, 2021

azure-pipelines bot commented Mar 31, 2021

StephenLPeters Mar 31, 2021

karkarl commented Mar 31, 2021

azure-pipelines bot commented Mar 31, 2021

ranjeshj commented Apr 1, 2021

karkarl commented Apr 1, 2021

azure-pipelines bot commented Apr 1, 2021

karkarl commented Apr 1, 2021

azure-pipelines bot commented Apr 1, 2021

licanhua Apr 1, 2021

karkarl Apr 3, 2021

StephenLPeters commented Apr 1, 2021 •

edited

Loading

StephenLPeters commented Apr 1, 2021

karkarl commented Apr 2, 2021 •

edited

Loading

karkarl commented Apr 2, 2021

azure-pipelines bot commented Apr 2, 2021

karkarl commented Apr 2, 2021

azure-pipelines bot commented Apr 2, 2021

StephenLPeters left a comment

karkarl commented Apr 5, 2021

azure-pipelines bot commented Apr 5, 2021

licanhua Apr 5, 2021

karkarl Apr 5, 2021

ghost commented May 3, 2021

Add New NavView VisualStateGroup #4659

Add New NavView VisualStateGroup #4659

Conversation

karkarl commented Mar 26, 2021 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

karkarl commented Mar 26, 2021

azure-pipelines bot commented Mar 26, 2021

karkarl commented Mar 29, 2021

azure-pipelines bot commented Mar 29, 2021

karkarl commented Mar 31, 2021

azure-pipelines bot commented Mar 31, 2021

StephenLPeters Mar 31, 2021

Choose a reason for hiding this comment

karkarl commented Mar 31, 2021

azure-pipelines bot commented Mar 31, 2021

ranjeshj commented Apr 1, 2021

karkarl commented Apr 1, 2021

azure-pipelines bot commented Apr 1, 2021

karkarl commented Apr 1, 2021

azure-pipelines bot commented Apr 1, 2021

licanhua Apr 1, 2021

Choose a reason for hiding this comment

karkarl Apr 3, 2021

Choose a reason for hiding this comment

StephenLPeters commented Apr 1, 2021 • edited Loading

StephenLPeters commented Apr 1, 2021

karkarl commented Apr 2, 2021 • edited Loading

karkarl commented Apr 2, 2021

azure-pipelines bot commented Apr 2, 2021

karkarl commented Apr 2, 2021

azure-pipelines bot commented Apr 2, 2021

StephenLPeters left a comment

Choose a reason for hiding this comment

karkarl commented Apr 5, 2021

azure-pipelines bot commented Apr 5, 2021

licanhua Apr 5, 2021

Choose a reason for hiding this comment

karkarl Apr 5, 2021

Choose a reason for hiding this comment

ghost commented May 3, 2021

karkarl commented Mar 26, 2021 •

edited

Loading

StephenLPeters commented Apr 1, 2021 •

edited

Loading

karkarl commented Apr 2, 2021 •

edited

Loading