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

Update CornerRadiusFilterConverter to work around a TemplateBinding bug #1239

Merged
merged 4 commits into from
Aug 30, 2019

Conversation

kaiguo
Copy link
Contributor

@kaiguo kaiguo commented Aug 29, 2019

We hit an issue that binding expression like below does not work in C++/CX apps.

RadiusX="{Binding RelativeSource={RelativeSource TemplatedParent}, Path=CornerRadius.TopLeft}"

Updated CornerRadiusFilterConverter so we can get a double typed CornerRadius value for a specific corner, then updated binding expressions to use converter instead. New expression using converter looks like this

RadiusX="{Binding CornerRadius, RelativeSource={RelativeSource TemplatedParent}, Converter={StaticResource TopLeftCornerRadiusDoubleValueConverter}}"

Also added a CornerRadius test page in TestAppCX to test out the changes.

@kaiguo kaiguo requested a review from jevansaks August 29, 2019 22:20
@kaiguo kaiguo requested a review from a team as a code owner August 29, 2019 22:20
@kaiguo
Copy link
Contributor Author

kaiguo commented Aug 29, 2019

@MikeHillberg We added some new values to the CornerRadiusFilterKind enum in order to make this work. "TopLeftValue" and "BottomRightValue" will filter out the value and return the result in double type.

public void CornerRadiusTest()
{
var button = new Button(FindElement.ByName("GetCheckBoxRectangleCornerRadiusValue"));
button.Click();
Copy link
Member

Choose a reason for hiding this comment

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

Recommend using Invoke() instead of Click() (which is mouse input) for better speed and reliability

Copy link
Member

@jevansaks jevansaks left a comment

Choose a reason for hiding this comment

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

Thanks! Approved with minor suggestion.

@jevansaks jevansaks added the auto merge This PR will be merged once all checks pass label Aug 30, 2019
@jevansaks jevansaks added this to the WinUI 2.2 milestone Aug 30, 2019
@jevansaks
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@msft-github-bot msft-github-bot merged commit c663faa into master Aug 30, 2019
@msft-github-bot msft-github-bot deleted the user/kaiguo/corner-radius-converter-updates branch August 30, 2019 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto merge This PR will be merged once all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants