Skip to content

Conversation

@JorisCleVR
Copy link
Contributor

ToggleButton now uses an EllipseClipConverter to create the Ellipse for clipping of button.

This fixes some binding errors like: System.Windows.Data Error: 2 : Cannot find governing FrameworkElement or FrameworkContentElement for target element. BindingExpression:Path=Width; DataItem=null; target element is 'EllipseGeometry' (HashCode=37186555); target property is 'RadiusX' (type 'Double')

@Keboo Keboo added this to the 5.3.0 milestone Jun 10, 2025
using System.Windows.Data;
using System.Windows.Media;

namespace MaterialDesignThemes.Wpf.Converters
Copy link
Member

Choose a reason for hiding this comment

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

nit: We have been favoring file-scoped namespaces

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed good to keep consistency in the code base: Fixed

@corvinsz
Copy link
Member

corvinsz commented Jun 11, 2025

Not that we need it, and I'm just asking out of curiosity:
Do we in general not add tests for the converters?
Currently there are tests for only like 5 converters.

At work we always add tests for every converter to (obviously) test it, and also to show intent of what the converter is supposed to do. Sometimes it could be hard to fit a converters purpose into it's name. Especially because converters are this well encapsulated/standalone "component" it seems like a nobrainer to add tests for it.

@nicolaihenriksen
Copy link
Contributor

@corvinsz I don't believe we have a rule/guideline stating that you must or must not add tests for them. Personally I have added a few tests when the logic grows to be non-trivial.

However, I like your comment about the tests indicating intent, because naming is hard! and I have often found myself scratching my head looking a converter name trying to decipher what its purpose in life is.

So in short, I think you're more than welcome to add tests, especially fast running "unit tests". The more safety net, the better IMO.

@corvinsz
Copy link
Member

Neat. After @Keboo has successfully converted and merged all the tests to TUnit - and I can spare some time - I can take a look at adding tests for converters.

@Keboo Keboo enabled auto-merge (squash) June 13, 2025 04:06
@Keboo Keboo merged commit 556b412 into MaterialDesignInXAML:master Jun 13, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants