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

Add a MirroredWhenRightToLeft API to AnimatedIcon #5023

Merged
merged 4 commits into from
May 21, 2021

Conversation

StephenLPeters
Copy link
Contributor

Some animated icons should be mirrored when the flow direction is RTL, like the BackButton. Some should not, like the checkmark. This PR adds an API to animated icon that allows opting into mirroring on a per animated icon basis. The default is to not mirror, which is a change in behavior.

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label May 15, 2021
@StephenLPeters
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@beervoley beervoley added area-AnimatedIcon team-Controls Issue for the Controls team labels May 17, 2021
@@ -22,13 +22,17 @@ unsealed runtimeclass AnimatedIcon : Windows.UI.Xaml.Controls.IconElement
[MUX_PROPERTY_CHANGED_CALLBACK(TRUE)]
IconSource FallbackIconSource{ get; set; };

[MUX_PROPERTY_CHANGED_CALLBACK(TRUE)]
Boolean MirroredWhenRightToLeft{ get; set; };
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to update any of our usages to set this property to non default ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Some Icons have specific Right to Left versions which are not just mirrors. Is there a good way to swap out the Animated Icon and Fallback, for those cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ranjeshj yes, we will.

@mdtauk interesting point, this would be doable by the app author, but is not currently easily supported by AnimatedIcon (or any of our icons).

dev/AnimatedIcon/AnimatedIcon.cpp Show resolved Hide resolved
@ranjeshj ranjeshj removed the needs-triage Issue needs to be triaged by the area owners label May 18, 2021
@StephenLPeters
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StephenLPeters StephenLPeters dismissed jevansaks’s stale review May 21, 2021 18:05

Resolved the issue.

@StephenLPeters StephenLPeters merged commit f4d781d into main May 21, 2021
@StephenLPeters StephenLPeters deleted the user/stpete/AnimatedIconMirrored branch May 21, 2021 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-AnimatedIcon team-Controls Issue for the Controls team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants