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

Adding styles for the system XAML CommandBarFlyoutCommandBar #4593

Merged
merged 7 commits into from
Apr 1, 2021

Conversation

llongley
Copy link
Member

Until we add the ability to retrieve the monitor bounds from a UI element, some apps are going to need to use the system XAML CommandBarFlyout to get that functionality, so this adds styles for that control's CommandBar in order to get the WinUI 2 experience from it.

While I was here, there were also two issues I needed to fix:

  • The x:Key'd version of the CommandBarFlyoutCommandBar style is defined in the default style XAML file, not the theme resource XAML file, which makes it so we can't use it in a Style's BasedOn property. I fixed that by moving it to the theme resource file.
  • I also found an issue where the corner radius erroneously instantly snaps back to its original state while the secondary commands popup is still animating closed due to not being properly animated in a visual transition. I added a visual transition to fix that.

…dBar styles were in the style XAMLs instead of the theme resource XAMLs, which made it so they couldn't be derived from.

- Fixed an issue where CornerRadius wasn't being properly animated.
- Added styles for the WUXC version of CommandBarFlyoutCommandBar.
@llongley llongley requested a review from jevansaks March 20, 2021 02:04
@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Mar 20, 2021
@llongley
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ranjeshj ranjeshj added area-Commanding AppBar, UICommand, MVVM, etc team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Mar 20, 2021
@ranjeshj ranjeshj requested a review from licanhua March 23, 2021 12:52
@ranjeshj
Copy link
Contributor

@llongley can you please resolve the conflicts ?

@llongley
Copy link
Member Author

Conflicts resolved.

@llongley
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@llongley
Copy link
Member Author

/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
Copy link
Contributor

Moving the named style from the xaml file to the resources file does have more effect than just making the resource exposable. It changes where the StaticResource invokations originate from, which changes the resource lookup behavior. This could potentially cause some resouce overrides which worked before this change to no longer work. But it seems like you need the based on behavior so I think we need to do this.

@llongley llongley merged commit bb55f85 into master Apr 1, 2021
@llongley llongley deleted the user/llongley/WUXCCommandBarFlyoutStyle branch April 1, 2021 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Commanding AppBar, UICommand, MVVM, etc team-Controls Issue for the Controls team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants