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

MVVM ObservableCollection SelectedItem = null = Exception #8329

Closed
gfmoore opened this issue Jun 25, 2022 · 21 comments
Closed

MVVM ObservableCollection SelectedItem = null = Exception #8329

gfmoore opened this issue Jun 25, 2022 · 21 comments
Labels
area-controls-collectionview CollectionView, CarouselView, IndicatorView p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint platform/android 🤖 t/bug Something isn't working

Comments

@gfmoore
Copy link

gfmoore commented Jun 25, 2022

Description

Following the guidelines in the docs:[(https://docs.microsoft.com/en-us/dotnet/maui/user-interface/controls/collectionview/selection#clear-selections)] Using community mvvm

Clear selections
The SelectedItem and SelectedItems properties can be cleared by setting them, or the objects they bind to, to null.

So I have a MainPage and a ViewModel for it

  [ObservableProperty]
  private Friend selectedItem;

...

 public ICommand SelectionChangedCommand => new Command<Object>(async (Object e) =>
  {
    Console.WriteLine($"Selection made {SelectedItem.FName} {SelectedItem.LName}");
    **SelectedItem = null;**
    //INavigation navigation = App.Current.MainPage.Navigation;

  });
     <CollectionView ItemsSource="{Binding FriendsOC}"
                      SelectionMode="Single"
                      SelectedItem="{Binding SelectedItem}"
                      SelectionChangedCommand="{Binding SelectionChangedCommand}"
                      SelectionChangedCommandParameter="{Binding .}" >

        <CollectionView.ItemTemplate>
          <DataTemplate>

    ...

I select the item and bang!

I expect the selected item highlight to be cleared. It will be done at some later point, but everytime I do it it crashes

image

Steps to Reproduce

As above

Version with bug

6.0.400 (current)

Last version that worked well

Unknown/Other

Affected platforms

Android, I was not able test on other platforms

Affected platform versions

Pixel 5 API 30

Did you find any workaround?

Nope

Relevant log output

No response

@gfmoore gfmoore added s/needs-verification Indicates that this issue needs initial verification before further triage will happen t/bug Something isn't working labels Jun 25, 2022
@BinaryAssault
Copy link

Not sure if it'll fix it, but did you try using the source code generator?

instead of

 public ICommand SelectionChangedCommand => new Command<Object>(async (Object e) =>
  {
    Console.WriteLine($"Selection made {SelectedItem.FName} {SelectedItem.LName}");

  });

Use

[ICommand] // notice the word command is not included. 
 public async void SelectionChanged(Friend friend)
  {
    Console.WriteLine($"Selection made {SelectedItem.FName} {SelectedItem.LName}");
    SelectedItem = null;
  }

@gfmoore
Copy link
Author

gfmoore commented Jun 25, 2022

I'm getting an error on the [ICommand] decoration/attribute? saying

Compiler Error CS0653
Cannot apply attribute class 'class' because it is abstract

An abstract custom attribute class cannot be used as an attribute.

I'm also wondering how the xaml would call this since my binding is to SelectionChangedCommand?

Did you note I'm using MVVM and the community helper stuff? :)

I liked the idea and the contribution though :)

@BinaryAssault
Copy link

I'm getting an error on the [ICommand] decoration/attribute? saying

Compiler Error CS0653 Cannot apply attribute class 'class' because it is abstract

An abstract custom attribute class cannot be used as an attribute.

I'm also wondering how the xaml would call this since my binding is to SelectionChangedCommand?

Did you note I'm using MVVM and the community helper stuff? :)

I liked the idea and the contribution though :)

The code generator will create an ICommand named SelectedChangedCommand. You must make your class partial to use it properly.

Regardless of my suggestion, I think you should post more code as there seems to be more going on than your original comment.

@gfmoore
Copy link
Author

gfmoore commented Jun 26, 2022

namespace Census.ViewModels;

public partial class CensusViewModel : ObservableObject
{
...
  [ObservableProperty]
  private ObservableCollection<Friend> friendsOC = new();
...
  [ObservableProperty]
  private Friend selectedItem;
...

  //respond to item select in list of friends
  public ICommand SelectionChangedCommand => new Command<Object>(async (Object e) =>
  {
    Console.WriteLine($"Selection made {SelectedItem.FName} {SelectedItem.LName}");

    INavigation navigation = App.Current.MainPage.Navigation;
    await navigation.PushModalAsync(new DetailPage(SelectedItem));

    //how do I clear the selection, esp whilst the modal page is being displayed.
    SelectedItem = null  //<--Bang!
  });

...

}


...

  <CollectionView ItemsSource="{Binding FriendsOC}"
                  SelectionMode="Single"
                  SelectedItem="{Binding SelectedItem}"
                  SelectionChangedCommand="{Binding SelectionChangedCommand}"
                  SelectionChangedCommandParameter="{Binding .}" >

    <CollectionView.ItemTemplate>
      <DataTemplate>
        <Grid ColumnDefinitions="*, *, 20">
          <Label Grid.Column="0"
                 Text="{Binding FName}"
                 FontSize="20"
                 TextColor="Yellow" />
          <Label Grid.Column="1"
                 Text="{Binding LName}"
                 FontSize="20"
                 TextColor="Yellow" />
          <Label Grid.Column="2"
                 Text="{Binding GroupId}"
                 FontSize="20"
                 TextColor="Green" />
        </Grid>
      </DataTemplate>
    </CollectionView.ItemTemplate>
  </CollectionView>

</Frame>

<Frame Grid.Row="2"
       CornerRadius="10"
       BorderColor="blue"
       BackgroundColor="Black"
       HeightRequest="60"
       Margin="0,0,0,0"
       Padding="0,2,0,0">

...


Hope this helps :)


On tapping the line item
System.NullReferenceException: 'Object reference not set to an instance of an object.'


btw I wish I knew what to do with these runtime exceptions as I have no idea where t happens, what line it's happening on or how to trace through it except to use breakpoints and it happens on the setting SelectedItem to null


Edit: I'm looking into this [ICommand] attribute. I should point out that I'm on CommunityToolkit.Mvvm 8.0.0 preview 4 which I needed for something else - a picker issue I think. 

I don't understand why it won't work. That's odd. I've been wondering if there was a command equivalent to [ObservableProperty], so that's progress. :)

@gfmoore
Copy link
Author

gfmoore commented Jun 26, 2022

Another thought is that I am (attempting) to use dependency injection (following tutorials) My program works, but could there be a side effect?

MauiProgram.cs

namespace Census;

public static class MauiProgram
{
	public static MauiApp CreateMauiApp()
	{
		var builder = MauiApp.CreateBuilder();
		builder
			.UseMauiApp<App>()
			.ConfigureFonts(fonts =>
			{
				fonts.AddFont("OpenSans-Regular.ttf", "OpenSansRegular");
				fonts.AddFont("OpenSans-Semibold.ttf", "OpenSansSemibold");
			});

		//dependency injection
		builder.Services.AddTransient<MainPage>();
		builder.Services.AddTransient<DetailPage>();
		builder.Services.AddTransient<CensusViewModel>();
		builder.Services.AddTransient<CensusDetailViewModel>();

		return builder.Build();
	}
}

@gfmoore
Copy link
Author

gfmoore commented Jun 26, 2022

I'm reading the documentation and indeed something very odd is going on in that it isn't accepting the [ICommand] yet accepts the [ObservableProperty] - doesn't make sense.

It's as though it isn't linked to the library. For instance [ObservableProperty] is in green on my VSStudio community 2022 edition dark mode, whilst ICommand is in yellow (which I think indicates that it thinks it's one of mine?)

I'm using GlobalUsing.cs and have
global using CommunityToolkit.Mvvm;
global using CommunityToolkit.Mvvm.ComponentModel;

I think I only need the ComponentModel.

I also tried moving the using to the ViewModel, but no joy.

I'm missing something!!

@gfmoore
Copy link
Author

gfmoore commented Jun 26, 2022

I've added a basic repo at https://github.com/gfmoore/TestMVVMHelper.App

It doesn't recognise the [ICommand].

Am I using the correct nuget package?

[Conscious of moving away from the main issue, sorry! :( ]

@gfmoore
Copy link
Author

gfmoore commented Jun 26, 2022

Ahhhh According to this https://egvijayanand.in/2022/04/22/mvvm-made-easy/
[ICommand] has been renamed [RelayCommand] and now [this bit] works. Now back to the main issue.
Also need: using CommunityToolkit.Mvvm.Input;
(For CommunityToolkit.Mvvm 8.0.0 preview4)

@gfmoore
Copy link
Author

gfmoore commented Jun 26, 2022

Man, this is a lot of trouble for something so simple. The error does not seem to be with the SelectedItem = null; after all (with my original code, the new RelayCommand is bringing up some other error. I hadn't noticed because the selection did get cleared, but somewhere in my code I must now be using the SelectedItem and it's now null. I now need to trace this error.

Sorry for the trouble and I'll close this down. Very useful for the use of [RelayCommand] though (if I can get it to work properly in my code.)

Thanks for your help :)

@gfmoore
Copy link
Author

gfmoore commented Jun 26, 2022

Nope, spoke too soon, still getting exception. Somewhere the SelectedItem is being used, but it's not in this ViewModel that I can see.

@gfmoore
Copy link
Author

gfmoore commented Jun 26, 2022

I'm going to definitely close this down. There seems to be nothing wrong with the SelectedItem = null; in a little test app I made https://github.com/gfmoore/TestMVVMHelper.App (for now). It works fine. Somehow setting the SelectedItem to null is causing a side effect somewhere else, but I have no idea how to trace the exception. I think it might be something to do with navigating to another page and in the plumbing since I disabled navigating whilst getting this working.

I noted that: "CollectionView will throw an exception if its ItemsSource is updated off the UI thread."

Clutching at straws, but perhaps setting SelectedItem to null is doing that? I don't know.

Thanks anyway.

@gfmoore gfmoore closed this as completed Jun 26, 2022
@gfmoore
Copy link
Author

gfmoore commented Jun 27, 2022

Sorry, but I am reopening this (again) as I am not solving the issue. But I have further information which may lead to insights.

If I do

SelectedItem = null

My app crashes with : System.NullReferenceException: 'Object reference not set to an instance of an object.'

Something needs a real object attached to SelectedItem - but I'm not using it anywhere. I have removed my code that references a details page and it just runs as a standard AppShell to MainPage.

After a sudden divine insight!!! (yes I prayed... :) ) I set SelectedItem to be a new empty object.

SelectedItem = new Friend();

It seemed to work, but when I set a breakpoint on this line I noted that it seemed to be going through the command - probably for each of the 140 odd items in my ObservableCollection. Was it searching hrough the bound ObservableCollection?

On a hunch I set my SelectedItem to the first item in my list

await Task.Delay(2000);
SelectedItem = FriendsOC[0];

I added a delay between the selection and this line and yep, the item selected was highlighted and after 2 seconds the first item was highlighted.

I did the same for the 10th item in the list

SelectedItem = FriendsOC[11];

and indeed it does!!!

So my hypothesis is that setting SelectedItem to null is causing the CollectionView code to cycle through the bound ObservableCollection, but it can't because it is null, hence the System.NullReferenceException: 'Object reference not set to an instance of an object.' error.

This has taken me umpteen hours to debug, not helped by the fact that ViusalStudio is not reporting where the exception is arising and I don't know how to set that - probably in some debug options parameter???

So what next?

@gfmoore gfmoore reopened this Jun 27, 2022
@BinaryAssault
Copy link

Can you post your code?

@rachelkang
Copy link
Member

Hi, @gfmoore - Thank you so much for opening this issue and for all your detailed updates! It seems your code has evolved many times already and I unfortunately don't seem to have access to the link you shared at https://github.com/gfmoore/TestMVVMHelper.App. Would you please share an updated GitHub repo we can access with your most recent code so that we can investigate this issue further? Thanks :)

@rachelkang rachelkang added area-controls-collectionview CollectionView, CarouselView, IndicatorView platform/android 🤖 fatal and removed s/needs-verification Indicates that this issue needs initial verification before further triage will happen labels Jun 27, 2022
@gfmoore
Copy link
Author

gfmoore commented Jun 27, 2022

Okay, sorry I hadn't realised my test app was private - it's public now https://github.com/gfmoore/TestMVVMHelper.App/settings It's very simple.

My main app is public, but you'll need a bit of work to make it operate - creating a couple of json files for friends and groups and then putting them in the downloads folder of the emulator or real device. I can't supply my real data, but at least you can see my code.

If it comes really necessary to get it working, I'll do what I need to :)

https://github.com/gfmoore/Census

EDIT: Of course you could directly populate the FriendsOC observable collection with some dummy names. (same with groups if needed). You don't need to import anything - though part of the code may access the sqldata. If I get time I'll try and make something cut down work.

I appreciate the interest.

@gfmoore
Copy link
Author

gfmoore commented Jun 27, 2022

I've just uploaded a better! test using objects rather than a string and it works fine. Must be something in my Census code. https://github.com/gfmoore/TestMVVMHelper.App/settings

@rachelkang
Copy link
Member

Hi, @gfmoore - are you saying that the original issue you have reported has been resolved?

@gfmoore
Copy link
Author

gfmoore commented Jun 27, 2022

No, not at all. I'm saying that my original issue still remains, but I tried to create a simple test to see if the issue was my app or the CollectionView. Using a simple string the CollectionView in my test seemed to work. So I assumed my code had an error, but once I'd removed the plumblng for navigating to a different page the issue still remained in my census code. There was no other entry for using SelectedItem in my census code. I did some experiments as described.

I've no uploaded a new simplified test ( a few minutes ago) and at first everything seemed ok, but I've just now managed to duplicate the error that appears in my more complex census code.

If I put a breakpoint on line 37, select a monkey and then continue I get a break on the null reference.

Hope that makes sense.

@gfmoore
Copy link
Author

gfmoore commented Jun 27, 2022

Oh man, I think I've figured it.

If I remove the async in the public void SelectionChanged the break occurs on the Console.WriteLine statement EVEN THOUGH that occurs first.

Asynchronous code is yet again biting me.

I'm assuming then that the Console.WriteLine is taking longer to do it's stuff and the SelectedItem is set to null before Console.WriteLine can access the properties. So bang.

Not sure how to deal with it though.

EDIT: Uhmm, yet if I leave Console.WriteLine in the data gets written to the output window?!!

EDIT2: Is it something to do with things running on different threads?

@gfmoore
Copy link
Author

gfmoore commented Jun 27, 2022

Is it because when SelectedItem is set to null this triggers a reentry into the SelectionChanged event/command and now that SelectedItem is null the console.writeline fails.

Solution:

  [RelayCommand]
  public async void SelectionChanged()
  {
    if (SelectedItem == null) return;

    //do whatever here e.g.
    await Task.Delay(3000);

   SelectedItem = null; // to remove highlight
}

My problem is solved (I very much hope so anyway!!!) :sigh

@gfmoore gfmoore closed this as completed Jun 27, 2022
@rachelkang
Copy link
Member

Yay!! I'm glad you've figured it out :)

@samhouts samhouts added the p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint label Jul 12, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Aug 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-collectionview CollectionView, CarouselView, IndicatorView p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint platform/android 🤖 t/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants