Fix Incorrect usages of UnsetValue

[Rosuavio]

Fixes #3311 Remove incorrect usages of DependencyProperty.UnsetValue and use it in values converters where appropriate.

In some places we are using DependencyProperty.UnsetValue where we are not supposed to.
DependencyProperty.UnsetValue is only supposed to be used as a returned value from IValueConverters that fail to convert there value. This pr removes it from where its not supposed to be used and uses it in value converters instead of throwing exceptions.

PR Type

What kind of change does this PR introduce?

• Bugfix

What is the current behavior?

What is the new behavior?

PR Checklist

Please check if your PR fulfills the following requirements:

• Tested code with current supported SDKs
• Pull Request has been submitted to the documentation repository instructions. Link:
• Sample in sample app has been added / updated (for bug fixes / features)
  • Icon has been created (if new sample) following the Thumbnail Style Guide and templates
• New major technical changes in the toolkit have or will be added to the Wiki e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
• Tests for the changes have been added (for bug fixes / features) (if applicable)
• Header has been added to all new source files (run build/UpdateHeaders.bat)
• Contains NO breaking changes

Other information

[ghost]

Thanks RosarioPulella for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

[Rosuavio]

I am letting #3943 fix the instance in ColorPicker.

[Rosuavio]

There are some locations where our usage of DependencyProperty.UnsetValue is questionable, this would be a good opportunity to check them.

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/68888d2d7f0eba1b1f671a7ba1acc775ac0a46ef/Microsoft.Toolkit.Uwp.Input.GazeInteraction/GazeTargetItem.cs#L45-L55

[Rosuavio]

Some instances in DataGrid where we ref UnsetValue, do they make sense?
https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/68888d2d7f0eba1b1f671a7ba1acc775ac0a46ef/Microsoft.Toolkit.Uwp.UI.Controls.DataGrid/DataGrid/DataGridComboBoxColumn.cs#L245-L255

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/68888d2d7f0eba1b1f671a7ba1acc775ac0a46ef/Microsoft.Toolkit.Uwp.UI.Controls.DataGrid/DataGrid/DataGridComboBoxColumn.cs#L292-L313

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/68888d2d7f0eba1b1f671a7ba1acc775ac0a46ef/Microsoft.Toolkit.Uwp.UI.Controls.DataGrid/DataGrid/DataGridTextColumn.cs#L173-L187

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/68888d2d7f0eba1b1f671a7ba1acc775ac0a46ef/Microsoft.Toolkit.Uwp.UI.Controls.DataGrid/DataGrid/DataGridTextColumn.cs#L215-L228

[Rosuavio]

Last one

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/68888d2d7f0eba1b1f671a7ba1acc775ac0a46ef/Microsoft.Toolkit.Uwp.UI.Controls.Input/TokenizingTextBox/TokenizingTextBoxItem.AutoSuggestBox.cs#L99-L122

[michael-hawker]

@RosarioPulella the ReadLocalValue cases are all fine and expected. That's like the one case where we do want to be checking against UnsetValue. 🙂 (Appreciate you finding them all though!)

Basically ReadLocalValue gets the raw value from the runtime itself as opposed to just trying to get the value from the property and having it return null. So ReadLocalValue may also return null if the developer set the property value to null, but would instead return UnsetValue if it wasn't set at all.

That's why we should adhere to the contract in the other places with PropertyMetadata and IValueConverter to ensure that the rest of the system works as intended.

Since it looks like it's just the one update here for ClipToBounds (outside the incoming ColorPicker updates), would you want to add having the ValueConverters returns UnsetValue as part of this PR too for #3311? (I think we have some Unit Tests for the converters, if not we should, so that could be a starting point as well to ensure we're not breaking anything when we switch?)

[Rosuavio]

would you want to add having the ValueConverters returns UnsetValue as part of this PR too for #3311?

Yep, I'll comb threw!