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

Made changes in test repo for making Setter.Value the ContentProperty for Setter #321

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

anjali-wpf
Copy link
Member

@anjali-wpf anjali-wpf commented Jul 5, 2024

Fixes Issue #84

Description

Made changes in test repo for making Setter.Value the ContentProperty for Setter

Customer Impact

None

Regression

None

Testing

Added test case

Risk

NA

Microsoft Reviewers: Open in CodeFlow

@anjali-wpf anjali-wpf changed the title Setter changes Made changes in test repo for making Setter.Value the ContentProperty for Setter Jul 5, 2024
@@ -71,6 +71,7 @@
<PropertyToSkip PropertyName="LCID" Owner="CultureInfo" />
<!-- With DWM turned on, Button.BorderBrush changes-->
<PropertyToSkip PropertyName="BorderBrush" Owner="Button" />
<PropertyToSkip PropertyName="Value" Owner="Setter" />
Copy link
Member

Choose a reason for hiding this comment

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

I would like to understand how we are using the PropertiesToSkip.xaml to skip the properties for the comparison and why are we making a change here. The rationale for the question is that we were checking it earlier and now we are skipping this check, so skipping needs better reasoning.

@@ -301,6 +303,59 @@ private static bool CompareXmlNode(XmlNode node1, XmlNode node2)
return true;
}

private static bool IsContentProperty(XmlNode xmlNode)
Copy link
Member

Choose a reason for hiding this comment

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

I am not getting the implementation of this function. As of now what I can get is that we are reading the propertiesToSkip.xml and working on that check only. There should be more checks in my understanding to validate if it is a ContentProperty or not.

@anjali-wpf anjali-wpf marked this pull request as ready for review July 22, 2024 07:39
@anjali-wpf anjali-wpf requested a review from a team as a code owner July 22, 2024 07:39
@anjali-wpf anjali-wpf removed the draft label Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants