Cards become invisible until XAML is hot reloaded if using OptimizedObservableCollection

[Tommigun1980]

Hi and thanks for a great component!

I noticed that if you use OptimizedObservableCollection (https://github.com/xamarinhq/xamu-infrastructure/blob/598f97d6f5cc68fc603873e913ccc9089dbc60df/src/XamU.Core/Collections/OptimizedObservableCollection.cs) with BeginMassUpdate, the deck is drawn as empty. Everything goes back to normal is the XAML is hot reloaded, but that is of course not shippable.

this.MyDeck = new OptimizedObservableCollection<MyCardType>()
{
    EmptyCard
};

var newCards = ... ; // get from server

using (this.MyDeck.BeginMassUpdate())
    this.MyDeck.AddRange(newCards);

<swipeCardView:SwipeCardView ItemsSource="{Binding MyDeck}">

Removing BeginMassUpdate() makes the problem go away. OptimizedObservableCollection is part of Microsoft's Xamarin University library and is quite convenient; it turns off sending events at start of BeginMassUpdate() and when exiting the using clause it sends reset event for the entire collection (which is of course faster than doing tons of updates). I haven't had problems using OptimizedObservableCollection with other components so I'm hopeful to get a fix for this :)

https://github.com/xamarinhq/xamu-infrastructure/blob/598f97d6f5cc68fc603873e913ccc9089dbc60df/src/XamU.Core/Collections/OptimizedObservableCollection.cs shows that OptimizedObservableCollection sends the following events when the operation is done:

            public void Dispose ()
            {
                parent.shouldRaiseNotifications = true;
                parent.OnPropertyChanged (new PropertyChangedEventArgs ("Count"));
                parent.OnPropertyChanged (new PropertyChangedEventArgs ("Item[]"));
                parent.OnCollectionChanged (new NotifyCollectionChangedEventArgs (NotifyCollectionChangedAction.Reset));
            }

Maybe the order of events is something SwipeCardView doesn't understand?

Thanks so much!

[Tommigun1980]

I just noticed that even without an OptimizedObservableCollection there's a problem.

If you just add cards to the deck at any point and then swipe, the card below won't be shown until the swipe has finished. So I think that altering the deck in any way seems to create problems.

Please advice on a workaround if at all possible. I tried to force refresh the layout of the deck when it's topped up with more cards, but it didn't help.

I think the problem may have to do with me removing cards from the deck, ie I reset it at certain points, and looking at https://github.com/markolazic88/SwipeCardView/blob/master/src/MLToolkit.Forms.SwipeCardView/SwipeCardView.cs seems to not support removing cards?

Thanks.

[Tommigun1980]

Looking at the sources I can also see that the CollectionChanged listener leaks if the item template property is changed.

And it looks like NotifyCollectionChangedAction.Reset exits early, which is why OptimizedObservableCollection doesn't work.

None of these are blockers for me except that it doesn't seem to support removing cards. I've tried to force reset it by hacking around with changing the item template but I just can't get it to work.

[Tommigun1980]

Ok I was kinda able to get it to work by making absolutely insane hacks, but the deck flashes when doing this and this also leaks memory due to the issue I reported above:

    // this resets _cards and the internal layout, but leaks the CollectionChanged listener added in OnItemsSourcePropertyChanged
    var oldTemplate = this.cards.ItemTemplate;
    this.cards.ItemTemplate = null;
    this.cards.ItemTemplate = oldTemplate;

    // this sends a NotifyCollectionChangedAction.Reset that OnItemSourceCollectionChanged handles
    using (this.vm.MyDeck.BeginMassUpdate()) { }

    // this sends an edit event that makes OnItemSourceCollectionChanged call Setup()
    this.vm.MyDeck[0] = this.vm.MyDeck[0];

Needless to say this is not the best way of handling things. Is there any chance the component could be updated to support removing elements, or at least add a Reset() method? OnItemsSourcePropertyChanged should also call this Reset() internally. But best would of course be if it understood adding/removing items natively. Thanks.

[markolazic88]

Thanks for pointing this out. This library definitely needs better collection changes handling. My initial goal with the library was to show that it was possible to create Tinder-like UI with Xamarin, which meant just adding new images on the end of collections. "Going back one card" feature also waits for the major changes on collections handling logic

[Tommigun1980]

Thanks for merging the pull request.

I opened a new pull request, https://github.com/markolazic88/SwipeCardView/pull/24, which fixes the flickering issues and makes the control disposable (with cleanup of registered listeners). I still didn't add proper handling of removing cards, but at least now if the deck is reset it will look fine.