From 3b5ae9d5371461a24061a32cd801d40aeb26204b Mon Sep 17 00:00:00 2001 From: Chris Lang Date: Tue, 10 Jul 2012 18:18:00 -0700 Subject: [PATCH] [ios-sdk] Fix bug in paging loader that led to multiple activity indicators in friend picker. Summary: FBGraphObjectPagingLoader was setting a dataNeededDelegate on its data source even if it was not in the AsNeeded paging mode. This was fooling the data source into thinking it should display a spinner in the bottom-most table cell to indicate data was being fetched. Controls that aren't in AsNeeded mode may have other UI to indicate when they load data (as friend picker does), so this spinner was redundant. Fixed FBGraphObjectPagingLoader to only set a dataNeededDelegate if it actually should; in turn, this pointed out that pagingMode should not be a writable property on the object, as there is little hope of things working correctly if the pagingMode is changed during the lifetime of an object. Changed it into a readonly property set at init-time. Test Plan: - Ran FriendPickerSample - Ran PlacesPickerSample - Ran Scrumptious - Ran HelloFacebook Revert Plan: Reviewers: jacl, mmarucheck, gregschechte, ayden Reviewed By: jacl CC: msdkexp@, platform-diffs@lists Differential Revision: https://phabricator.fb.com/D516326 --- src/FBFriendPickerCacheDescriptor.m | 4 ++-- src/FBFriendPickerViewController.m | 5 +++-- src/FBGraphObjectPagingLoader.h | 5 +++-- src/FBGraphObjectPagingLoader.m | 13 ++++++++++--- src/FBPlacePickerCacheDescriptor.m | 5 +++-- src/FBPlacePickerViewController.m | 4 +++- 6 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/FBFriendPickerCacheDescriptor.m b/src/FBFriendPickerCacheDescriptor.m index c08bdd148d..a029f6ebf7 100644 --- a/src/FBFriendPickerCacheDescriptor.m +++ b/src/FBFriendPickerCacheDescriptor.m @@ -101,12 +101,12 @@ - (void)prefetchAndCacheForSession:(FBSession*)session { } self.loader.delegate = nil; - self.loader = [[FBGraphObjectPagingLoader alloc] initWithDataSource:datasource]; + self.loader = [[FBGraphObjectPagingLoader alloc] initWithDataSource:datasource + pagingMode:FBGraphObjectPagingModeImmediateViewless]; self.loader.session = session; [self.loader release]; self.loader.delegate = self; - self.loader.pagingMode = FBGraphObjectPagingModeImmediateViewless; // make sure we are around to handle the delegate call [self retain]; diff --git a/src/FBFriendPickerViewController.m b/src/FBFriendPickerViewController.m index e43dd9e4d9..278fe7baa9 100644 --- a/src/FBFriendPickerViewController.m +++ b/src/FBFriendPickerViewController.m @@ -112,8 +112,9 @@ - (void)initialize selectionManager.delegate = self; // Paging loader - self.loader = [[FBGraphObjectPagingLoader alloc] initWithDataSource:self.dataSource]; - self.loader.pagingMode = FBGraphObjectPagingModeImmediate; + self.loader = [[FBGraphObjectPagingLoader alloc] initWithDataSource:self.dataSource + pagingMode:FBGraphObjectPagingModeImmediate]; + [_loader release]; self.loader.delegate = self; // Self diff --git a/src/FBGraphObjectPagingLoader.h b/src/FBGraphObjectPagingLoader.h index a650e86e89..00fd434fdd 100644 --- a/src/FBGraphObjectPagingLoader.h +++ b/src/FBGraphObjectPagingLoader.h @@ -36,10 +36,11 @@ typedef enum { @property (nonatomic, retain) FBGraphObjectTableDataSource *dataSource; @property (nonatomic, retain) FBSession *session; @property (nonatomic, assign) id delegate; -@property (nonatomic) FBGraphObjectPagingMode pagingMode; +@property (nonatomic, readonly) FBGraphObjectPagingMode pagingMode; @property (nonatomic, readonly) BOOL isResultFromCache; -- (id)initWithDataSource:(FBGraphObjectTableDataSource*)aDataSource; +- (id)initWithDataSource:(FBGraphObjectTableDataSource*)aDataSource + pagingMode:(FBGraphObjectPagingMode)pagingMode; - (void)startLoadingWithRequest:(FBRequest*)request cacheIdentity:(NSString*)cacheIdentity skipRoundtripIfCached:(BOOL)skipRoundtripIfCached; diff --git a/src/FBGraphObjectPagingLoader.m b/src/FBGraphObjectPagingLoader.m index fe09bffac7..4d5db3fb09 100644 --- a/src/FBGraphObjectPagingLoader.m +++ b/src/FBGraphObjectPagingLoader.m @@ -25,6 +25,7 @@ @interface FBGraphObjectPagingLoader () @property (nonatomic, retain) FBRequestConnection *connection; @property (nonatomic, copy) NSString *cacheIdentity; @property (nonatomic, assign) BOOL skipRoundtripIfCached; +@property (nonatomic) FBGraphObjectPagingMode pagingMode; - (void)followNextLink; - (void)requestCompleted:(FBRequestConnection *)connection @@ -49,10 +50,12 @@ @implementation FBGraphObjectPagingLoader #pragma mark Lifecycle methods -- (id)initWithDataSource:(FBGraphObjectTableDataSource*)aDataSource { +- (id)initWithDataSource:(FBGraphObjectTableDataSource*)aDataSource + pagingMode:(FBGraphObjectPagingMode)pagingMode;{ if (self = [super init]) { + // Note that pagingMode must be set before dataSource. + self.pagingMode = pagingMode; self.dataSource = aDataSource; - self.pagingMode = FBGraphObjectPagingModeAsNeeded; _isResultFromCache = NO; } return self; @@ -75,7 +78,11 @@ - (void)setDataSource:(FBGraphObjectTableDataSource *)dataSource { [dataSource retain]; [_dataSource release]; _dataSource = dataSource; - _dataSource.dataNeededDelegate = self; + if (self.pagingMode == FBGraphObjectPagingModeAsNeeded) { + _dataSource.dataNeededDelegate = self; + } else { + _dataSource.dataNeededDelegate = nil; + } } - (void)setTableView:(UITableView*)tableView { diff --git a/src/FBPlacePickerCacheDescriptor.m b/src/FBPlacePickerCacheDescriptor.m index 5bf10f842a..337cd230c5 100644 --- a/src/FBPlacePickerCacheDescriptor.m +++ b/src/FBPlacePickerCacheDescriptor.m @@ -83,12 +83,13 @@ - (void)prefetchAndCacheForSession:(FBSession*)session { session:session]; self.loader.delegate = nil; - self.loader = [[FBGraphObjectPagingLoader alloc] initWithDataSource:datasource]; + self.loader = [[FBGraphObjectPagingLoader alloc] + initWithDataSource:datasource + pagingMode:FBGraphObjectPagingModeImmediateViewless]; self.loader.session = session; [self.loader release]; self.loader.delegate = self; - self.loader.pagingMode = FBGraphObjectPagingModeImmediateViewless; // make sure we are around to handle the delegate call [self retain]; diff --git a/src/FBPlacePickerViewController.m b/src/FBPlacePickerViewController.m index 364c831a63..a428b37356 100644 --- a/src/FBPlacePickerViewController.m +++ b/src/FBPlacePickerViewController.m @@ -120,7 +120,9 @@ - (void)initialize selectionManager.delegate = self; // Paging loader - self.loader = [[FBGraphObjectPagingLoader alloc] initWithDataSource:self.dataSource]; + self.loader = [[FBGraphObjectPagingLoader alloc] initWithDataSource:self.dataSource + pagingMode:FBGraphObjectPagingModeAsNeeded]; + [_loader release]; self.loader.delegate = self; // Self