Skip to content

Conversation

United600
Copy link
Collaborator

@United600 United600 commented Sep 24, 2025

Minor refactor and improvement to the ThicknessFiltersConverter class and its related enum.

  • Adds new ThicknessFilterKinds combinations
  • Renames the Convert method to Extract
  • Sets the Extract method to static for direct XAML usage
  • Changes the Extract behavior, the None value now sets all fields to 0
  • Improves the Convert method by returning DependencyProperty.UnsetValue instead of null for unsupported values
  • Sets the Filters property default value back to None
  • Removes the partial modifier, as it will no longer be needed for UWP modern NET migration

add more thickness filters combinations

rename convert method
@United600 United600 requested a review from Copilot September 24, 2025 19:17
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Sep 24, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the ThicknessFiltersConverter class to improve its API design and behavior. The changes focus on adding new thickness filter combinations, improving method naming, and making the converter more suitable for XAML usage.

  • Adds new ThicknessFilterKinds enum values (Horizontal, Vertical, All) for better filtering options
  • Renames Convert method to Extract and makes it static for direct XAML usage
  • Changes behavior to return DependencyProperty.UnsetValue instead of null for invalid inputs

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines -149 to +160
return null;
return DependencyProperty.UnsetValue;
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not aware of the DependencyProperty.UnsetValue. Is there a functional difference between the two?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same here, I only learned about it from this toolkit converter. Not sure about the difference, we could just add #nullable enable to the file and call it a day. I'm not sure which one is the "correct".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants