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

Refactor fix for 2689 to use attached properties #2762

Merged
merged 4 commits into from
Jul 8, 2022

Conversation

nicolaihenriksen
Copy link
Contributor

@nicolaihenriksen nicolaihenriksen commented Jul 5, 2022

Refactoring of previous fix for #2689

Added attached properties for UniformCornerRadius and Style. These properties are inherited and can thus be used to manipulate a Card located inside of another UIElement (eg. the Flipper). I decided to put them in a CardAssist file to align with the other attached properties in the toolkit. It could be moved to the Card type if that is more desirable.

Added more unit- and UI tests for this as well. Also fixed a bug in the original elevated card style (UniformCornerRadius binding) caught by these tests.

Added attached properties for UniformCornerRadius and Style. These properties are inherited and can thus be used to manipulate a Card located inside of another UIElement (eg. the Flipper).
Fallback to default(Style) if the TargetType is not Card.
@ElieTaillard ElieTaillard requested a review from Keboo July 7, 2022 18:42
Copy link
Member

@Keboo Keboo left a comment

Choose a reason for hiding this comment

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

Looks pretty close, just a couple changes.

@@ -0,0 +1,34 @@
namespace MaterialDesignThemes.Wpf;

public static class CardAssist
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 rename this to FlipperAssist since the consumption is designed for flippers not cards, even though the target of the property is a card.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, I'll rename it.

MaterialDesignThemes.Wpf/Flipper.cs Show resolved Hide resolved
@Keboo Keboo added enhancement release notes Items are likely to be highlighted in the release notes. labels Jul 8, 2022
@Keboo Keboo added this to the 4.6.0 milestone Jul 8, 2022
@Keboo Keboo added the visual breaking change Items here have affected the visual look of controls. label Jul 8, 2022
…d property)

You mentioned on stream that fact that derived types would not work for the CardStyle because of the guard which is completely correct. So I loosened up the requirements by using IsAssignableFrom() instead.
Rename operation did not pick up the string literal in these tests
@Keboo Keboo merged commit 78c4aa3 into MaterialDesignInXAML:master Jul 8, 2022
@nicolaihenriksen nicolaihenriksen deleted the refactor2689 branch July 15, 2022 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement release notes Items are likely to be highlighted in the release notes. visual breaking change Items here have affected the visual look of controls.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants