From b3d431335588b2da550414dab411953d9ed4cbcf Mon Sep 17 00:00:00 2001 From: Maxime Ollivier Date: Tue, 31 Oct 2023 13:22:24 -0700 Subject: [PATCH] fix IGListExperimentFixCrashOnReloadObjects 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 --- Source/IGListKit/IGListAdapter.m | 36 ++++++++++++++++---------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/Source/IGListKit/IGListAdapter.m b/Source/IGListKit/IGListAdapter.m index fedb02dea..b32d8d574 100644 --- a/Source/IGListKit/IGListAdapter.m +++ b/Source/IGListKit/IGListAdapter.m @@ -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]; @@ -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) { @@ -792,7 +783,7 @@ - (void)_updateBackgroundViewShouldHide:(BOOL)shouldHide { } } - _collectionView.backgroundView.hidden = shouldHide; + _collectionView.backgroundView.hidden = !shouldDisplay; } - (BOOL)_itemCountIsZero { @@ -1298,7 +1289,7 @@ - (void)performBatchAnimated:(BOOL)animated updates:(void (^)(id