Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[iOS] Allow observable source update while CollectionView is not visible #13678

Merged
merged 3 commits into from
Feb 9, 2021

Conversation

hartez
Copy link
Contributor

@hartez hartez commented Feb 5, 2021

Description of Change

Fix timing issue where CollectionView items are added to ObservableSource before CollectionView is visible, and CollectionView is later made visible via Binding.

Issues Resolved

API Changes

None

Platforms Affected

  • iOS

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

Testing Procedure

Automated UI test.

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

CollectionView updates while CollectionView is hidden;
fixes #13126
@hartez hartez added this to To Fix in 5.0.0 SR 3 via automation Feb 5, 2021
@hartez hartez moved this from To Fix to PR Needs Review in 5.0.0 SR 3 Feb 5, 2021
@Tommigun1980
Copy link

Fantastic @hartez! Is there a package with this fix I could test?
Thank you so much!

Only force layout on transition from invisible to visible
Move cell size cache to layout and clear cache on size change (e.g., rotation)
Use ItemsView size for constraints when possible;
@rmarinho
Copy link
Member

rmarinho commented Feb 7, 2021

@hartez
Copy link
Contributor Author

hartez commented Feb 7, 2021

UWP and Android test failures are utterly unrelated. iOS 14 failures are the known ListView issues.

@Tommigun1980
Copy link

Tommigun1980 commented Feb 8, 2021

@Tommigun1980 you can try the packages inside the artifacts folder / Nuget

https://dev.azure.com/xamarin/public/_build/results?buildId=34759&view=artifacts&pathAsName=false&type=publishedArtifacts

https://dev.azure.com/xamarin/6fd3d886-57a5-4e31-8db7-52a1b47c07a8/_apis/build/builds/34759/artifacts?artifactName=nuget&api-version=6.0&%24format=zip

Thank you.

Tested with this package and the issue was unfortunately not fixed in my app.

I still have to do the following after populating the CollectionView in order for its items to become visible:

var items = this.myCollectionView.ItemsSource;
this.myCollectionView.ItemsSource = null;
this.myCollectionView.ItemsSource = items;

If I don't do the above, it will display the EmptyView. A hot reload also makes the contents draw properly.

@timvandenhof
Copy link

@Tommigun1980 you can try the packages inside the artifacts folder / Nuget
https://dev.azure.com/xamarin/public/_build/results?buildId=34759&view=artifacts&pathAsName=false&type=publishedArtifacts
https://dev.azure.com/xamarin/6fd3d886-57a5-4e31-8db7-52a1b47c07a8/_apis/build/builds/34759/artifacts?artifactName=nuget&api-version=6.0&%24format=zip

Thank you.

Tested with this package and the issue was unfortunately not fixed in my app.

I still have to do the following after populating the CollectionView in order for its items to become visible:

var items = this.myCollectionView.ItemsSource;
this.myCollectionView.ItemsSource = null;
this.myCollectionView.ItemsSource = items;

If I don't do the above, it will display the EmptyView. A hot reload also makes the contents draw properly.

We are experiencing the same (or similar) issue, with the same exception. We are using a view with two tabs, where each tabs has a separate view model with separate ObservableCollection. Collection view has grouped items, with sticky list header behaviour disabled.

Downloaded the above NuGet packages and updated locally to verify this issue has been resolved.
This appears not to be the case for us:

Foundation.MonoTouchException: Objective-C exception thrown.  Name: NSInternalInconsistencyException Reason: Invalid update: invalid number of sections.  The number of sections contained in the collection view after the update (12) must be equal to the number of sections contained in the collection view before the update (12), plus or minus the number of sections inserted or deleted (1 inserted, 0 deleted).
Native stack trace:
	0   CoreFoundation                      0x00007fff20421af6 __exceptionPreprocess + 242
	1   libobjc.A.dylib                     0x00007fff20177e78 objc_exception_throw + 48
	2   CoreFoundation                      0x00007fff2042191f +[NSException raise:format:] + 0
	3   Foundation                          0x00007fff2077156a -[NSAssertionHandler handleFailureInMethod:object:file:lineNumber:description:] + 191
	4   UIKitCore                           0x00007fff23d92d24 -[UICollectionView _endItemAnimationsWithInvalidationContext:tentativelyForReordering:animator:collectionViewAnimator:] + 15731
	5   UIKitCore                           0x00007fff23d8efa0 -[UICollectionView _endItemAnimationsWithInvalidationContext:tentativelyForReordering:animator:] + 60
	6   UIKitCore                           0x00007fff23d8e58e -[UICollectionView _updateSections:updateAction:] + 454
	7   UIKitCore                           0x00007fff23d8e65e -[UICollectionView insertSections:] + 64
	8   UIKit                               0x0000000108ebd4e4 -[UICollectionViewAccessibility insertSections:] + 42
	9   MyApp.iOS                   0x00000001012df489 xamarin_dyn_objc_msgSend + 217
	10  ???                                 0x0000000109a444f0 0x0 + 4456727792

  at ObjCRuntime.Runtime.ThrowNSException (System.IntPtr ns_exception) [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/14.8.0.3/src/Xamarin.iOS/ObjCRuntime/Runtime.cs:407
  at ObjCRuntime.Runtime.throw_ns_exception (System.IntPtr exc) [0x00000] in /Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/runtime/Delegates.generated.cs:126
  at at (wrapper native-to-managed) ObjCRuntime.Runtime.throw_ns_exception(intptr)
  at at (wrapper managed-to-native) ObjCRuntime.Messaging.void_objc_msgSend_IntPtr(intptr,intptr,intptr)
  at UIKit.UICollectionView.InsertSections (Foundation.NSIndexSet sections) [0x0001b] in /Library/Frameworks/Xamarin.iOS.framework/Versions/14.8.0.3/src/Xamarin.iOS/UIKit/UICollectionView.g.cs:537
  at Xamarin.Forms.Platform.iOS.ObservableGroupedSource+<>c__DisplayClass24_0.<Add>b__0 () [0x00000] in D:\agent\1\s\Xamarin.Forms.Platform.iOS\CollectionView\ObservableGroupedSource.cs:207
  at Xamarin.Forms.Platform.iOS.ObservableGroupedSource.Update (System.Action update) [0x00013] in D:\agent\1\s\Xamarin.Forms.Platform.iOS\CollectionView\ObservableGroupedSource.cs:358
  at Xamarin.Forms.Platform.iOS.ObservableGroupedSource.Add (System.Collections.Specialized.NotifyCollectionChangedEventArgs args) [0x00068] in D:\agent\1\s\Xamarin.Forms.Platform.iOS\CollectionView\ObservableGroupedSource.cs:207
  at Xamarin.Forms.Platform.iOS.ObservableGroupedSource.CollectionChanged (System.Collections.Specialized.NotifyCollectionChangedEventArgs args) [0x00026] in D:\agent\1\s\Xamarin.Forms.Platform.iOS\CollectionView\ObservableGroupedSource.cs:147
  at Xamarin.Forms.Platform.iOS.ObservableGroupedSource+<>c__DisplayClass19_0.<CollectionChanged>b__0 () [0x00000] in D:\agent\1\s\Xamarin.Forms.Platform.iOS\CollectionView\ObservableGroupedSource.cs:134
  at Foundation.NSAsyncActionDispatcher.Apply () [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/14.8.0.3/src/Xamarin.iOS/Foundation/NSAction.cs:152
  at at (wrapper managed-to-native) UIKit.UIApplication.UIApplicationMain(int,string[],intptr,intptr)
  at UIKit.UIApplication.Main (System.String[] args, System.IntPtr principal, System.IntPtr delegate) [0x00005] in /Library/Frameworks/Xamarin.iOS.framework/Versions/14.8.0.3/src/Xamarin.iOS/UIKit/UIApplication.cs:86
  at UIKit.UIApplication.Main (System.String[] args, System.String principalClassName, System.String delegateClassName) [0x0000e] in /Library/Frameworks/Xamarin.iOS.framework/Versions/14.8.0.3/src/Xamarin.iOS/UIKit/UIApplication.cs:65
  at MyApp.iOS.Application.Main (System.String[] args) [0x00001] in /Users/MyUsername/Projects/MyApp/MyApp.iOS/Main.cs:16

Tried workarounds:

  • Make sure update on collection took place on UI thread.
  • In stead of .Clear(), use a while loop to remove items one by one before adding all new retrieved data.

Maybe the above information is useful for further troubleshooting.

@hartez
Copy link
Contributor Author

hartez commented Feb 8, 2021

@Tommigun1980 I thought it was odd that it didn't work, because I basically just copied your repro project from 13126 as my test case for this. But you're right - applying this nuget package to the repro project does not work.

However, if I apply the nuget package to your repro project and then comment out the using (this.Models.BeginMassUpdate()) in MyViewModel.cs, it does work.

So now I need to figure out what the XamarinUniversity.Infrastructure package's "OptimizedObservableCollection" is doing that's thwarting the ObservableItemsSource.

@hartez
Copy link
Contributor Author

hartez commented Feb 8, 2021

@timvandenhof Your issue sounds like a different problem; your situation is crashing (not just remaining blank) and you're using a grouped collection. Can you open an issue? And if you can provide a small repro project, that would help a lot. It sounds like your problem might be a grouped version of issue #7700.

@hartez
Copy link
Contributor Author

hartez commented Feb 8, 2021

@Tommigun1980 Okay, looking at OptimizedObservableCollection, it looks like when the mass update object is Disposed the collection raises a Reset event. Which should just trigger a full UICollectionView.ReloadData(), but I'm guessing there's a timing/race condition we're not accounting for. I'll add another test case for a reset and see what I can find.

@Tommigun1980
Copy link

@Tommigun1980 Okay, looking at OptimizedObservableCollection, it looks like when the mass update object is Disposed the collection raises a Reset event. Which should just trigger a full UICollectionView.ReloadData(), but I'm guessing there's a timing/race condition we're not accounting for. I'll add another test case for a reset and see what I can find.

Thank you @hartez.

Yeah the OptimizedObservableCollection suspends the updates when it is populated, and only sends out one Reset when done.
It’s effectively a “batch add”.

Thank you so much for fixing these!!

@rmarinho rmarinho merged commit a10c2c6 into 5.0.0 Feb 9, 2021
5.0.0 SR 3 automation moved this from PR Needs Review to Done Feb 9, 2021
@rmarinho rmarinho deleted the fix-13126 branch February 9, 2021 11:54
@samhouts samhouts added this to the 5.0.0 milestone Feb 12, 2021
@samhouts samhouts added i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often i/regression t/bug 🐛 labels Feb 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/collectionview i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often i/regression p/iOS 🍎 t/bug 🐛
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

[Bug] Regression: 5.0.0-pre5 often fails to draw dynamically loaded collection view content
6 participants