Avalonia icon indicating copy to clipboard operation
Avalonia copied to clipboard

ListBox selection not working correctly with CompileBinding

Open mysteryx93 opened this issue 1 year ago • 6 comments

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.

mysteryx93 avatar Jun 26 '24 17:06 mysteryx93

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.

stevemonaco avatar Jun 26 '24 22:06 stevemonaco

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.

mysteryx93 avatar Jun 26 '24 22:06 mysteryx93

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.

maxkatz6 avatar Jun 27 '24 01:06 maxkatz6

There are no errors, from what I can tell

mysteryx93 avatar Jun 27 '24 02:06 mysteryx93

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.

stevemonaco avatar Jun 27 '24 13:06 stevemonaco

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

maxkatz6 avatar Jun 27 '24 21:06 maxkatz6