Search code examples
c#wpfmvvmcomboboxinotifypropertychanged

Why SelectedItem in combobox is not set with OnPropertyChanged()?


I have a very strange behavior of combobox and SelectedItem.

I want to have a message when selecting any other item in the combobox message show (That is not available yet), and setting [1] value in the SelectedItem

private static ObservableCollection<string> sourceTypeCollection = new ObservableCollection<string>() { "1", "2", "3", "4", "5", "6", "7", "8", "9" };
public ObservableCollection<string> SourceTypeCollection
{
    get => sourceTypeCollection;
    set => Set(ref sourceTypeCollection, value);
}


private string selectedValue = "2";
public string SelectedValue
{
    get => selectedValue;
    set
    {
        selectedValue = value;              
        if (selectedValue != SourceTypeCollection[1])
        {
            MessageBox.Show(value + " Is not available yet", "Source Info", MessageBoxButton.OK);
            selectedValue = SourceTypeCollection[1];                   
        }
        Set(ref selectedValue, value);
    }
}

<ComboBox     ItemsSource="{Binding SourceTypeCollection,  Mode=TwoWay, UpdateSourceTrigger=PropertyChanged}" 
                                                                           
                                          SelectedValue="{Binding SelectedValue, Mode=TwoWay, UpdateSourceTrigger=PropertyChanged}" 
                                          
                                          IsSynchronizedWithCurrentItem="True"                                         
                                          Height="30" FontSize="18" Width="150"
                                          VerticalAlignment="Top" Grid.Row="1"
                                          HorizontalAlignment="Center">
                                

                            </ComboBox>

But MVVM wpf Combobox shows the message, but still sets the selected value different from SourceTypeCollection[1]

What can be the problem?

public abstract class ViewModel : INotifyPropertyChanged, IDisposable
{
    public event PropertyChangedEventHandler PropertyChanged;

    protected virtual void OnPropertyChanged([CallerMemberName] string PropertyName = null)
    {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(PropertyName));
    }

    protected virtual bool Set<T>(ref T field, T value, [CallerMemberName] string PropertyName = null)
    {
        if (Equals(field, value)) return false;
        field = value;
        OnPropertyChanged(PropertyName);
        return true;
    }
}

Solution

  • I believe, but haven't put this one into a test project, that the problem is the implementation of Set. Your setter logic causes the property's backing field to hold only the value of SourceTypeCollection[1]. Any other value is coerced with in the setter.

    So when you call Set(ref selectedValue, value); the check for equality in the first line will always return true and the function always returns already in the first line (at max with the exception of the first call).
    So OnPropertyChanged is never invoked and the WPF-notification system doesn't re-evaluate the binding value. So you have a value in the combo box that is not equal to value of the binding.

    On the other hand the implementation of Set is basically correct and how everyone does it. It just doesn't fit in to your special coercion mechanism that actually doesn't allow any selection but one value.

    You (probably) could fix it like this (untested):

        public string SelectedValue
        {
            get => selectedValue;
            set
            {
                if (value!= SourceTypeCollection[1])
                {
                    MessageBox.Show(value + " Is not available yet", "Source Info", MessageBoxButton.OK);
                    Set(ref selectedValue, SourceTypeCollection[1], true);
                }
                else 
                {
                    Set(ref selectedValue, value);
                }
            }
        }    
       ...
        protected virtual bool Set<T>(ref T field, T value, bool forceNotify = false, [CallerMemberName] string PropertyName = null)
        {
            if (Equals(field, value))
            {
                 if (forceNotify) OnPropertyChanged(PropertyName);
                 return false;
            } 
            field = value;
            OnPropertyChanged(PropertyName);
            return true;
        }
    

    I wouldn't this consider good code, since the problem to my opinion is on the level of the UIX design. You don't put a list of choices into a combo if there is nothing you can actually choose.