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!

[marcelwgn]

@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)

[marcelwgn]

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.

[marcelwgn]

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?

[marcelwgn]

@StephenLPeters @ranjeshj Given that this seems like an issue with binding, does this need to wait until WinUI 3?

[codeaphex]

I think I got this but in reverse.
I'm trying to implement and infinite spinner functionality (e.x. when hitting max value, set it to min value + 1x change value).

NumberBox Config:

<muxc:NumberBox x:Name="MinuteNumberBox"      
                        Value="{Binding Minutes, Mode=TwoWay}"
                        SpinButtonPlacementMode="Inline"
                        SmallChange="15"
                        LargeChange="15"
                        Minimum="-15"
                        Maximum="60"
                    />

ViewModel Property (using MVVM Community Toolkit):

        private double _minutes;
        public double Minutes
        {
            get { return _minutes; }
            set
            {
                if (value == 60)
                {
                    value = 0;
                }
                if (value == -15)
                {
                    value = 45;
                }
                SetProperty(ref _minutes, value);
            }
        }

When I hit Min/Max the Property is updated (e.x. 60 becomes 0) but the change is not propagated to the view.
If I add an extra button and bind it to this command the values are correctly updated.

  RelayCommand _add15Command;
  public RelayCommand Add15Command
  {
      get
      {
          return _add15Command ??=
             new RelayCommand(
                  () => Minutes += 15
                  ); ;
      }
  }

[github-actions]

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 5 days.