Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Combobox SelectedItem sends extra set value #4461

Closed
Kermalis opened this issue Aug 9, 2020 · 3 comments · Fixed by #4659
Closed

Combobox SelectedItem sends extra set value #4461

Kermalis opened this issue Aug 9, 2020 · 3 comments · Fixed by #4659
Labels

Comments

@Kermalis
Copy link
Contributor

Kermalis commented Aug 9, 2020

Intro

I have this model:

    public sealed class TestModel : INotifyPropertyChanged, IDisposable
    {
        private void OnPropertyChanged(string property)
        {
            PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(property));
        }
        public event PropertyChangedEventHandler PropertyChanged;

        public MyEnum Value
        {
            get => Obj.Value;
            set
            {
                if (value != Obj.Value) // Crash happens here because Obj is null
                {
                    Obj.Value = value;
                    OnPropertyChanged(nameof(Value));
                }
            }
        }

        public static IEnumerable<MyEnum> SelectableValues { get; } = Utils.GetEnumValues<MyEnum>();

        internal MyObj Obj;

        internal ObjectEventModel(MyObj obj)
        {
            Obj = obj;
        }

        public void Dispose()
        {
            Obj = null;
            PropertyChanged = null;
        }
    }

And this is the xaml that uses this model

<ComboBox Items="{Binding SelectedObject.SelectableValues}" SelectedItem="{Binding SelectedObject.Value}" />

My DataContext has an ObservableCollection<TestModel> which holds the models. The control has a ComboBox to select the current TestModel (which is then stored in SelectedObject)

<ComboBox Items="{Binding Objects}" SelectedItem="{Binding SelectedObject}">

The problem

When I run the following code:

            foreach (TestModel obj in Objects)
            {
                obj.Dispose();
            }
            SelectedObject = null; // Crash in SelectedObject.Value.set
            Objects.Clear();

OR

            foreach (TestModel obj in Objects)
            {
                obj.Dispose();
            }
            Objects.Clear(); // Crash in SelectedObject.Value.set

I get a crash in SelectedObject.Value.set because the ComboBox tried to set the Value property to the first item in the ComboBox despite it no longer being bound. If I do not set Obj to null in TestModel.Dispose(), then there's no crash, but I now have a value I did not select stored in Obj.Value (always the first item in TestModel.SelectableValues)

Expected behavior

ComboBox should not set the value since it is now unbound. No other controls are doing it (NumericUpDown, etc) on the same model.
Personally, given how many problems I've encountered with ComboBox, we should just port the ComboBox from WPF at this point since I'm assuming it's way more functional

@Kermalis
Copy link
Contributor Author

Kermalis commented Aug 9, 2020

The issue goes away if you bind with x:Static like so:

<ComboBox Items="{x:Static uimodels:TestModel.SelectableValues}" SelectedItem="{Binding SelectedObject.Value}" />

So the problem is probably in how Items is handled upon being unbound

@grokys
Copy link
Member

grokys commented Aug 10, 2020

I've tried reproducing this but I don't think I have enough information to reproduce. Here's the code I tried to reproduce it with which was adapted from your repro here:

https://gist.github.com/grokys/722d90147f8b3a6632d5b1bcb362e074

However, it doesn't cause the crash as the issue says. Could I have a full minimal repro please?

@Kermalis
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants