AvaloniaUI / Avalonia

Develop Desktop, Embedded, Mobile and WebAssembly apps with C# and XAML. The most popular .NET UI client technology
https://avaloniaui.net
MIT License
24.6k stars 2.13k forks source link

ListBox selection not working correctly with CompileBinding #16144

Open mysteryx93 opened 2 weeks ago

mysteryx93 commented 2 weeks ago

Describe the bug

When CompileBinding is enabled, ListBox selections aren't working as expected, but work perfectly fine when disabled.

To Reproduce

BindingBug.zip Here is a simple repro project.

On the left are Playlists, on the right are Folders. Click "+" above Playlists twice to add 2 items. Add a folder in each. Works fine.

Now, edit MainView and remove all x:CompileBindings references. Try again.

This time, selecting a Playlist on the left doesn't change the displayed Folder on the right.

Expected behavior

It should behave like when CompileBinding is disabled.

Avalonia version

11.1.0-rc1

OS

Linux

Additional context

I believe I reported this a long while ago but didn't have time to investigate further and just disabled CompileBinding. Seeing it is still an issue in 11.1.0-rc1, I''m finally providing the simple repo.

Now it's causing me an issue because disabling CompileBinding is causing a problem with Trimming.

stevemonaco commented 2 weeks ago

It's a large repro to sort through, but it's related to interfaces in MainViewModel.

public ICollectionView<IPlaylistViewModel> Playlists { get; } = new CollectionView<IPlaylistViewModel>(); // Does not work

public CollectionView<IPlaylistViewModel> Playlists { get; } = new CollectionView<IPlaylistViewModel>(); // Works

And the ListBox bindings for quick reference:

<ListBox ItemsSource="{Binding Playlists.Source}" SelectedItem="{Binding Playlists.CurrentItem}">

Edit: This is because ICollectionView<T> doesn't have a setter for CurrentItem, but CollectionView<T> does. This doesn't appear like a bug for compiled bindings, but can hear what the team thinks.

mysteryx93 commented 2 weeks ago

Woah, simply adding a setter to that interface property makes the app work indeed! Very unintuitive solution to the problem.

It is still a strange behavior that it breaks only for compiled binding and works with regular bindings.

I just tested another bug I was having with compiled bindings with generic base class, that one seems to have been fixed. Compiled bindings overall working better than before, but should avoid those hard-to-pinpoint failures.

maxkatz6 commented 2 weeks ago

Very unintuitive solution to the problem

Depends on the error message. If it says that setter is missing, then it all good and there is no issue. If not, we should make it more clear.

mysteryx93 commented 2 weeks ago

There are no errors, from what I can tell

stevemonaco commented 2 weeks ago

I've simplified the repro, However, it doesn't show the ReflectionBinding vs CompiledBinding part: ReflectionBinding works with the concrete type assigned to the property whereas CompiledBinding must work with the property type (ie. the interface).

public partial class MainWindowViewModel : ViewModelBase
{
    public ObservableCollection<int> Items { get; }

    private int _selectedItem;
    public int SelectedItem
    {
        get => _selectedItem;
        set => SetProperty(ref _selectedItem, value); // Still compiles even when removed
    }

    public MainWindowViewModel()
    {
        Items = [1, 2, 3, 4, 5];
        SelectedItem = Items.First();
    }
}
<Grid RowDefinitions="*,auto">
    <ListBox ItemsSource="{Binding Items}" SelectedItem="{Binding SelectedItem}">
        <ListBox.ItemTemplate>
            <DataTemplate>
                <TextBlock Text="{Binding}" />
            </DataTemplate>
        </ListBox.ItemTemplate>
    </ListBox>

    <TextBlock Grid.Row="1" Text="{Binding SelectedItem}" />
</Grid>

The issue is that there seems to be no logged sanity check for the BindingMode. SelectedItemProperty has a default of TwoWay which isn't overridden here. The get-only version fails silently at runtime. Even on LogEventLevel.Verbose, there isn't a Binding or Property area error logged.

I think this should be a compile-time failure and force the dev to specify OneTime or OneWay (if not the AvaloniaProperty's default) when binding to get-only properties, but that's a breaking change.

maxkatz6 commented 2 weeks ago

@stevemonaco validation in compile time is problematic, because we can't get BindingMode during compilation, unless it's specified explicitly.