Microsoft-ui-xaml: Data-bound value not adjusted to fall into range for NumberBox

[MartinZikmund]

Describe the bug

When NumberBox has a Minimum/Maximum range set and the data-bound Value does not fall within this range, one of the boundary values is displayed in the control, but the boundary value is not propagated to the view model.

Steps to reproduce the bug
XAML:

<winui:NumberBox Minimum="2" Maximum="6" Value="{x:Bind Test, Mode=TwoWay}" />

C#:

public float Test {get; set;} = 0;

Now add a button that displays the value of Test in debug output. Notice that although number box shows "2", the output value is still 0, unless user modifies the value in any way.

Expected behavior

I would expect that the view model value would update accordingly when the "invalid" value is first bound.

Version Info

NuGet package version:

Microsoft.UI.Xaml 2.4.0

| Windows 10 version | Saw the problem? |
| :--------------------------------- | :-------------------- |
| Insider Build (xxxxx) | |
| May 2020 Update (19041) | Yes |
| November 2019 Update (18363) | |
| May 2019 Update (18362) | |
| October 2018 Update (17763) | |
| April 2018 Update (17134) | |
| Fall Creators Update (16299) | |
| Creators Update (15063) | |

| Device form factor | Saw the problem? |
| :----------------- | :--------------- |
| Desktop | Yes |
| Xbox | |
| Surface Hub | |
| IoT | |

Additional context

I have noticed the Slider control behaves the same way as well.

[MartinZikmund]

If the suggested behaviour would be preferred, I would be happy to try to implement this fix :-) .

[StephenLPeters]

@teaP fyi

[StephenLPeters]

@SavoySchuler FYI

[StephenLPeters]

@MartinZikmund This does seem like a bug, if you'd like to contribute the fix that would be awesome!

[chingucoding]

@MartinZikmund Would you still like to look into this, or would you be fine with somebody else tackling this?

[MartinZikmund]

@chingucoding Oh, I didn't notice the original response. I will be happy to do this (if it is not a time critical issue right now)

[chingucoding]

Right, yes. I don't think it's that time critical. If you need help with setting up the repro or getting started, feel free to ask!

[StephenLPeters]

Right, yes. I don't think it's not that time critical. If you need help with setting up the repro or getting started, feel free to ask!

I don't think its time critical

I think there was a mistaken double negative. @MartinZikmund go for it, looking foreword to the PR.

[chingucoding]

Oh right, yes there a negation to much in there, you are right @StephenLPeters !

[MartinZikmund]

I have looked into the issue and it seems this might be a problem with how Binding works under the covers. If the dependency property value is changed within the PropertyChangedCallback (which happens here during CoerceValue), the new value is not propagated back to the source in case of TwoWay binding. This seems like a platform bug. Simple unit test to see this scenario:

[TestMethod]
public void DataBoundValueIsCoerced()
{
    NumberBox numberBox = null;
    RunOnUIThread.Execute(() =>
    {
        numberBox = new NumberBox() {                    
            Minimum = 3,
            Maximum = 16
        };
        Content = numberBox;

        var dataBindingSource = new DataBindingSource();
        var binding = new Binding();            
        binding.Mode = BindingMode.TwoWay;
        binding.Source = dataBindingSource;
        binding.Path = new PropertyPath(nameof(DataBindingSource.Value));
        numberBox.SetBinding(NumberBox.ValueProperty, binding);

        Verify.AreEqual(numberBox.Minimum, dataBindingSource.Value);
        Verify.AreEqual(numberBox.Value, dataBindingSource.Value);
    });
}

I am not sure there is a way to work around this other than making some fix to how Binding works internally.

[StephenLPeters]

I have looked into the issue and it seems this might be a problem with how Binding works under the covers. If the dependency property value is changed within the PropertyChangedCallback (which happens here during CoerceValue), the new value is not propagated back to the source in case of TwoWay binding. This seems like a platform bug. Simple unit test to see this scenario:

[TestMethod]
public void DataBoundValueIsCoerced()
{
    NumberBox numberBox = null;
    RunOnUIThread.Execute(() =>
    {
        numberBox = new NumberBox() {                    
            Minimum = 3,
            Maximum = 16
        };
        Content = numberBox;

        var dataBindingSource = new DataBindingSource();
        var binding = new Binding();            
        binding.Mode = BindingMode.TwoWay;
        binding.Source = dataBindingSource;
        binding.Path = new PropertyPath(nameof(DataBindingSource.Value));
        numberBox.SetBinding(NumberBox.ValueProperty, binding);

        Verify.AreEqual(numberBox.Minimum, dataBindingSource.Value);
        Verify.AreEqual(numberBox.Value, dataBindingSource.Value);
    });
}

I am not sure there is a way to work around this other than making some fix to how Binding works internally.

Interesting, @MikeHillberg and/or @chrisglein do you have more incites on this behavior?