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

Pickers Inside CollectionView get SelectedItem Cleared on Scrolling #25842

Open
LeoJHarris opened this issue Nov 13, 2024 · 11 comments · May be fixed by #26775
Open

Pickers Inside CollectionView get SelectedItem Cleared on Scrolling #25842

LeoJHarris opened this issue Nov 13, 2024 · 11 comments · May be fixed by #26775
Labels
area-controls-collectionview CollectionView, CarouselView, IndicatorView p/2 Work that is important, but is currently not scheduled for release platform/iOS 🍎 s/needs-attention Issue has more information and needs another look s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working

Comments

@LeoJHarris
Copy link

LeoJHarris commented Nov 13, 2024

Description

Refer to the short gif below:

20241114_093914-ezgif.com-resize-video.mp4

Steps to Reproduce

No response

Link to public reproduction project repository

https://github.com/LeoJHarris/CollectionView_Picker_SelectedValueCleared.git

Version with bug

9.0.0 GA

Is this a regression from previous behavior?

No, this is something new

Last version that worked well

Unknown/Other

Affected platforms

iOS

Affected platform versions

iOS 18

Did you find any workaround?

ListView with Retain Element set to True

Relevant log output

@LeoJHarris LeoJHarris added the t/bug Something isn't working label Nov 13, 2024
@ninachen03 ninachen03 added platform/iOS 🍎 s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Nov 14, 2024
@ninachen03
Copy link

ninachen03 commented Nov 14, 2024

This issue has been verified using Visual Studio 17.13.0 Preview 1.0 (9.0.0-rc.2.24503.2 & 9.0.0). Can repro on iOS platform.

@kubaflo
Copy link
Contributor

kubaflo commented Nov 17, 2024

Hi, @LeoJHarris I see in your code that you don't have binding set on the SelectedItem property

<Picker
    Title="Select an item"
    Grid.Row="1"
    TextColor="Black">
    <Picker.ItemsSource>
        <x:Array Type="{x:Type x:String}">
            <x:String>Item 1</x:String>
            <x:String>Item 2</x:String>
            <x:String>Item 3</x:String>
        </x:Array>
    </Picker.ItemsSource>
</Picker>

Therefore the cell doesn't know how to restore to its previous state after it becomes visible again. Here's the code responsible for restoring cells:

Android:

if (!templateChanging)
{
// Same template, new data
View.BindingContext = itemBindingContext;
}

iOS:

if (PlatformHandler?.VirtualView is View view)
{
view.SetValueFromRenderer(BindableObject.BindingContextProperty, bindingContext);
}

As you can see on these videos collection view behaves strangely when binding is not applied correctly:

Incorrect binding Proper binding
Simulator.Screen.Recording.-.iPhone.16.Pro.Max.-.2024-11-17.at.02.06.49.mp4
Simulator.Screen.Recording.-.iPhone.16.Pro.Max.-.2024-11-17.at.02.02.55.mp4

This is an example of a viewModel that I've used

public class MainPageViewModel : BindableObject
{
    public class Item : BindableObject
    {
        private string _entryText;
        public string EntryText
        {
            get => _entryText;
            set
            {
                _entryText = value;
                OnPropertyChanged();
            }
        }

        private string _pickerSelection;
        public string PickerSelection
        {
            get => _pickerSelection;
            set
            {
                _pickerSelection = value;
                OnPropertyChanged();
            }
        }
    }

    private ObservableCollection<Item> _items = new ObservableCollection<Item>();
    public ObservableCollection<Item> Items
    {
        get => _items;
        set
        {
            _items = value;
            OnPropertyChanged();
        }
    }

    public MainPageViewModel()
    {
        Items = new ObservableCollection<Item>(Enumerable.Range(0, 20).Select(x => new Item()));
    }
}

Perhaps @rmarinho or @PureWeen can confirm if that's correct

@mattleibow mattleibow added area-controls-collectionview CollectionView, CarouselView, IndicatorView s/needs-info Issue needs more info from the author labels Nov 18, 2024
@mattleibow
Copy link
Member

@kubaflo thanks for the reply! Yes, you are correct. When scrolling the collection view, the cells are reused and if there is no binding then it will use whatever there was in there before.

@LeoJHarris is there a reason you are not binding? How are you planning on reading the data out of the controls?

@LeoJHarris
Copy link
Author

Hi all, sorry I haven't found the time to get back to to this but I didn't think it mattered to not set the binding, I have another application where the binding is set and we still have the same issue. I will update sample with MVVM and setting bindings tomorrow and update if the problem persists.

@dotnet-policy-service dotnet-policy-service bot added s/needs-attention Issue has more information and needs another look and removed s/needs-info Issue needs more info from the author labels Nov 19, 2024
@PureWeen PureWeen added s/needs-info Issue needs more info from the author and removed s/needs-attention Issue has more information and needs another look labels Nov 19, 2024
@LeoJHarris
Copy link
Author

Okay I've updated the sample project this time with pickers nested inside the CollectionView cell items using the proper bindings. Let me know if the setup is correct otherwise here is the end result.

Image

@dotnet-policy-service dotnet-policy-service bot added s/needs-attention Issue has more information and needs another look and removed s/needs-info Issue needs more info from the author labels Nov 19, 2024
@kubaflo
Copy link
Contributor

kubaflo commented Nov 22, 2024

@LeoJHarris okay, you're right. Actually it is not the iOS specific's bug. It happens on Android too, you just need to scroll more.

Screen.Recording.2024-11-20.at.01.45.53.mov

Basically it happens because you use a new itemsSource for each cell. So when a cell is reused, a new collection is being created and selected Index resets to its default value (-1) because of it.

I'm not sure if it is worth to overcomplicate the already overcomplicated picker's code to account for it. It is because I think it is not a likely situation that devs need a new itemsSource for each cell with picker.

In your case you can use either ListView or a stack view with bindable layout embedded inside a scroll view to fix it

Screen.Recording.2024-11-22.at.02.16.15.mov

However, if you really want to stick to collection view, I'd suggest creating one, common for all pickers, items source. Something like this for example:

<Picker
        Title="Select an item"
        Grid.Row="2"
        SelectedIndex="{Binding SelectedPickerIndex}"
        TextColor="Black">
        <Picker.ItemsSource>
                <x:Array Type="{x:Type x:String}">
                        <x:String>Item 1</x:String>
                        <x:String>Item 2</x:String>
                        <x:String>Item 3</x:String>
                </x:Array>
        </Picker.ItemsSource>
</Picker>

@mattleibow @PureWeen what do you think? Should it be closed?

@LeoJHarris
Copy link
Author

LeoJHarris commented Nov 22, 2024

@kubaflo Thanks for following up, I understand that we can possibly could use a ListView although I do feel there was a reason why we opted for a CollectionView which may come back to me, but what I don't understand was this was working as expected in Xamarin, we had been doing the same thing with a CollectionView in an existing production application without issues using Xamarin and never noticed this issue.

I know things are getting a bit complicated but that's just a consequence of our requirements, each picker depending on the selected item determine the content that is being displayed in the cell itself. I've deliberately omitted this detail from the sample so the sample doesn't paint the complete picture of how and why we do it this way.

@mattleibow
Copy link
Member

@kubaflo do you have that nice sample for android somewhere?

But I am also not sure I understand what you mean here. You said:

Basically it happens because you use a new itemsSource for each cell. So when a cell is reused, a new collection is being created and selected Index resets to its default value (-1) because of it.

As far as I can see, the whole structure is created at the start and then there are nested bindable objects. When swapping the BindingContext, it should not be resetting anything in the VM. So for the pickers, when you scroll the CV, the item is being set and when you swap the ItemsSource it should not be resetting the SelectedIndex.

You can image a world without any CV and you have some UI that has a picker. That picker is bound to VM1 with a source and selection. Both have some data. The you have VM2, with its own source and selection.

Changing the BC on the Picker should not update anything in either VM as it needs to wait for the BC to be finished setting. The code may now be seeing the source being reset, so it resets the selection, but maybe as part of that it should first make sure that the selection is preserved and then rather use that index instead of the -1.

I think this is a bug.

@mattleibow
Copy link
Member

Actually, is it still breaking in .NET 9? I copied the sample and it works fine on Android and Windows. I don't have my mac set up at the moment, but maybe you can have a check?

MauiApp21.zip

@mattleibow mattleibow added s/needs-info Issue needs more info from the author and removed s/needs-attention Issue has more information and needs another look labels Dec 5, 2024
@LeoJHarris
Copy link
Author

LeoJHarris commented Dec 11, 2024

@mattleibow I have tested on iOS with .NET 9 and the issue still remains, I will try android also

@dotnet-policy-service dotnet-policy-service bot added s/needs-attention Issue has more information and needs another look and removed s/needs-info Issue needs more info from the author labels Dec 11, 2024
@jfversluis jfversluis added s/needs-info Issue needs more info from the author and removed s/needs-attention Issue has more information and needs another look labels Dec 11, 2024
@jfversluis jfversluis added this to the .NET 9 Servicing milestone Dec 11, 2024
@jfversluis jfversluis added the p/2 Work that is important, but is currently not scheduled for release label Dec 11, 2024
@dotnet-policy-service dotnet-policy-service bot added the s/no-recent-activity Issue has had no recent activity label Dec 16, 2024
@bhuwancb99
Copy link

Is this issue fixed or still pending? because in latest version on iOS devices it's not working and picker items clear inside CollectionView when scroll the page.

@dotnet-policy-service dotnet-policy-service bot added s/needs-attention Issue has more information and needs another look and removed s/needs-info Issue needs more info from the author s/no-recent-activity Issue has had no recent activity labels Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-controls-collectionview CollectionView, CarouselView, IndicatorView p/2 Work that is important, but is currently not scheduled for release platform/iOS 🍎 s/needs-attention Issue has more information and needs another look s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants