Skip to content

Commit

Permalink
fix IGListExperimentFixCrashOnReloadObjects
Browse files Browse the repository at this point in the history
Summary:
There's a couple issues with `IGListExperimentFixCrashOnReloadObjects` (D47281472):
1. We're calling `_updateBackgroundView` in the completion block, which might be called many times for a single transaction.
2. On animated updates, the background view will appear after the animation, which doesn't look great. Btw, this happens if you update the count via an item-block update, but maybe we can fix this separatly to keep things simple.

Instead of calling `_updateBackgroundView` in the completion block, lets go back to calling it within `_updateWithData`, but not check `isInDataUpdateBlock` to keep the current behavior.

Reviewed By: fabiomassimo

Differential Revision: D50752236

fbshipit-source-id: c742d9d64a93f1f2e1a48d85f832720d9350c545
  • Loading branch information
Maxime Ollivier authored and facebook-github-bot committed Oct 31, 2023
1 parent ee9fe50 commit b3d4313
Showing 1 changed file with 18 additions and 18 deletions.
36 changes: 18 additions & 18 deletions Source/IGListKit/IGListAdapter.m
Original file line number Diff line number Diff line change
Expand Up @@ -427,13 +427,6 @@ - (void)performUpdatesAnimated:(BOOL)animated completion:(IGListUpdaterCompletio
return;
}

if (IGListExperimentEnabled(strongSelf.experiments, IGListExperimentFixCrashOnReloadObjects)) {
// All the backgroundView changes during the update will be disabled, since we don't know how
// many cells we'll really have until it's all done. As a follow up, we should probably have
// a centralized place to do this, and change the background before the animation starts.
[strongSelf _updateBackgroundViewShouldHide:![strongSelf _itemCountIsZero]];
}

// release the previous items
strongSelf.previousSectionMap = nil;
[strongSelf _notifyDidUpdate:IGListAdapterUpdateTypePerformUpdates animated:animated];
Expand Down Expand Up @@ -770,18 +763,16 @@ - (void)_updateWithData:(IGListTransitionData *)data {
[[map sectionControllerForObject:object] didUpdateToObject:object];
}

[self _updateBackgroundViewShouldHide:![self _itemCountIsZero]];
[self _updateBackgroundView];

// Should be the last thing called in this function.
_isInObjectUpdateTransaction = NO;
}

- (void)_updateBackgroundViewShouldHide:(BOOL)shouldHide {
if ([self isInDataUpdateBlock]) {
return; // will be called again when update block completes
}
- (void)_updateBackgroundView {
const BOOL shouldDisplay = [self _itemCountIsZero];

if (!shouldHide) {
if (shouldDisplay) {
UIView *backgroundView = [self.dataSource emptyViewForListAdapter:self];
// don't do anything if the client is using the same view
if (backgroundView != _collectionView.backgroundView) {
Expand All @@ -792,7 +783,7 @@ - (void)_updateBackgroundViewShouldHide:(BOOL)shouldHide {
}
}

_collectionView.backgroundView.hidden = shouldHide;
_collectionView.backgroundView.hidden = !shouldDisplay;
}

- (BOOL)_itemCountIsZero {
Expand Down Expand Up @@ -1298,7 +1289,7 @@ - (void)performBatchAnimated:(BOOL)animated updates:(void (^)(id<IGListBatchCont
updates(weakSelf);
weakSelf.legacyIsInDataUpdateBlock = NO;
} completion: ^(BOOL finished) {
[weakSelf _updateBackgroundViewShouldHide:![weakSelf _itemCountIsZero]];
[weakSelf _updateBackgroundView];
[weakSelf _notifyDidUpdate:IGListAdapterUpdateTypeItemUpdates animated:animated];
if (completion) {
completion(finished);
Expand Down Expand Up @@ -1409,7 +1400,10 @@ - (void)insertInSectionController:(IGListSectionController *)sectionController a

NSArray *indexPaths = [self indexPathsFromSectionController:sectionController indexes:indexes usePreviousIfInUpdateBlock:NO];
[self.updater insertItemsIntoCollectionView:collectionView indexPaths:indexPaths];
[self _updateBackgroundViewShouldHide:![self _itemCountIsZero]];

if (![self isInDataUpdateBlock]) {
[self _updateBackgroundView];
}
}

- (void)deleteInSectionController:(IGListSectionController *)sectionController atIndexes:(NSIndexSet *)indexes {
Expand All @@ -1425,7 +1419,10 @@ - (void)deleteInSectionController:(IGListSectionController *)sectionController a

NSArray *indexPaths = [self indexPathsFromSectionController:sectionController indexes:indexes usePreviousIfInUpdateBlock:YES];
[self.updater deleteItemsFromCollectionView:collectionView indexPaths:indexPaths];
[self _updateBackgroundViewShouldHide:![self _itemCountIsZero]];

if (![self isInDataUpdateBlock]) {
[self _updateBackgroundView];
}
}

- (void)invalidateLayoutInSectionController:(IGListSectionController *)sectionController atIndexes:(NSIndexSet *)indexes {
Expand Down Expand Up @@ -1479,7 +1476,10 @@ - (void)reloadSectionController:(IGListSectionController *)sectionController {

NSIndexSet *sections = [NSIndexSet indexSetWithIndex:section];
[self.updater reloadCollectionView:collectionView sections:sections];
[self _updateBackgroundViewShouldHide:![self _itemCountIsZero]];

if (![self isInDataUpdateBlock]) {
[self _updateBackgroundView];
}
}

- (void)moveSectionControllerInteractive:(IGListSectionController *)sectionController
Expand Down

0 comments on commit b3d4313

Please sign in to comment.