ComboBox SelectionChanged fires before SelectedItem is updated

mrlacey commented 3 years ago

Current behavior

For a ComboBox, the SelectionChanged event fires before a bound value to SelectedItem is updated. This means it's not possible to find out the selected item from inside the event fired when selection changes.

Expected behavior

SelectionChanged should fire before SelectedItem does - just like it does in UWP.

How to reproduce it (as minimally and precisely as possible)

this xaml

<Page
    x:Class="SelectionChangedEventOrder.MainPage"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    xmlns:local="using:SelectionChangedEventOrder"
    xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
    xmlns:i="using:Microsoft.Xaml.Interactivity"
    xmlns:ic="using:Microsoft.Xaml.Interactions.Core" 
    mc:Ignorable="d">

    <Grid Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">

        <ComboBox
            HorizontalAlignment="Stretch"
            VerticalAlignment="Center"
            ItemsSource="{x:Bind ViewModel.ListOfNames}"
            SelectedItem="{x:Bind ViewModel.SelectedName, Mode=TwoWay}"
            SelectionChangedTrigger="Always">
            <i:Interaction.Behaviors>
                <ic:EventTriggerBehavior EventName="SelectionChanged">
                    <ic:InvokeCommandAction Command="{x:Bind ViewModel.SelectionChangedCommand}"/>
                </ic:EventTriggerBehavior>
            </i:Interaction.Behaviors>
        </ComboBox>
    </Grid>
</Page>

this code-behind

    public sealed partial class MainPage : Page
    {
        public MainViewModel ViewModel { get; set; } = new MainViewModel();

        public MainPage()
        {
            this.InitializeComponent();
        }

        private void ComboBox_SelectionChanged(object sender, SelectionChangedEventArgs e)
        {
            System.Diagnostics.Debug.WriteLine("SelectionChanged eventhandler called");
            System.Diagnostics.Debug.WriteLine($"SelectionName is {ViewModel.SelectedName}");
        }
    }

this ViewModel

namespace SelectionChangedEventOrder
{
    public class MainViewModel : INotifyPropertyChanged
    {
        private ObservableCollection<string> _listOfNames;

        private string _selectedName;

        public MainViewModel()
        {
            ListOfNames = new ObservableCollection<string>(new[] { "Andy", "Betty" , "Carl", "Diane", "Eric", "Francesca", "Gary" });

            SelectionChangedCommand = new RelayCommand(() => { SelectionChanged(); });
        }

        public string SelectedName
        {
            get => _selectedName;

            set
            {
                SetProperty(ref _selectedName, value);
                Debug.WriteLine($"SelectedName set as {value}");
            }
        }

        public ObservableCollection<string> ListOfNames
        {
            get => _listOfNames;
            set => SetProperty(ref _listOfNames, value);
        }

        public ICommand SelectionChangedCommand { get; }

        public void SelectionChanged()
        {
            Debug.WriteLine($"SelectedName is {SelectedName}");
            Debug.WriteLine("SelectionChanged called");
        }

        public event PropertyChangedEventHandler PropertyChanged;

        private bool SetProperty<T>(ref T field, T newValue, [CallerMemberName] string propertyName = null)
        {
            if (EqualityComparer<T>.Default.Equals(field, newValue))
            {
                return false;
            }

            field = newValue;

            this.PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));

            return true;
        }
    }

    public class RelayCommand : ICommand
    {
        private readonly Action execute;
        public event EventHandler CanExecuteChanged;
        public RelayCommand(Action execute)
        {
            this.execute = execute;
        }
        public bool CanExecute(object parameter)
        {
            return true;
        }

        public void Execute(object parameter)
        {
            this.execute();
        }
    }
}

zipped repro project

Workaround

None - for the MVVM purist :( Can get at the right value in code-behind

Environment

Nuget Package:

[x] Uno.UI / Uno.UI.WebAssembly / Uno.UI.Skia v3.6.6
[ ] Uno.WinUI / Uno.WinUI.WebAssembly / Uno.WinUI.Skia
[ ] Uno.SourceGenerationTasks
[ ] Uno.UI.RemoteControl / Uno.WinUI.RemoteControl
[x] Other: Uno.Microsoft.Xaml.Behaviors.Interactivity / Uno.Microsoft.Xaml.Behaviors.Uwp.Managed v2.0.3-uno.92

Nuget Package Version(s):

Affected platform(s):

[x] iOS
[x] Android
[x] WebAssembly
[ ] WebAssembly renderers for Xamarin.Forms
[ ] macOS
[ ] Skia
- [ ] WPF
- [ ] GTK (Linux)
- [ ] Tizen
[ ] Windows
[ ] Build tasks
[ ] Solution Templates

IDE:

[ ] Visual Studio 2017 (version: )
[x] Visual Studio 2019 (version: 16.9.4)
[ ] Visual Studio for Mac (version: )
[ ] Rider Windows (version: )
[ ] Rider macOS (version: )
[ ] Visual Studio Code (version: )

Relevant plugins:

[ ] Resharper (version: )

Anything else we need to know?

This appears to be the same issue as previously reported (and fixed) for ListView as #534

mrlacey commented 3 years ago

Given that this was previously addressed for ListView (in #534) can this be solved at a lower level so everything that inherits from Selector gets the fix? (i.e. solve this for FlipView too)

jeromelaban commented 3 years ago

Selector is already a base class, it's likely to be something else.

Xiaoy312 commented 3 years ago

The same issue is also present on ListView (or, any derived of Selector).

As a temporary workaround, you may bind to SelectedValue instead, which is updated before SelectionChanged fires. (This should be valid for all derived of Selector.)

unoplatform / uno