Skip to content
Browse files

[ios-sdk] Fix bug in paging loader that led to multiple activity indi…

…cators 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
  • Loading branch information...
1 parent 6aca821 commit 3b5ae9d5371461a24061a32cd801d40aeb26204b @clang13 clang13 committed Jul 10, 2012
View
4 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];
View
5 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
View
5 src/FBGraphObjectPagingLoader.h
@@ -36,10 +36,11 @@ typedef enum {
@property (nonatomic, retain) FBGraphObjectTableDataSource *dataSource;
@property (nonatomic, retain) FBSession *session;
@property (nonatomic, assign) id<FBGraphObjectPagingLoaderDelegate> 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;
View
13 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 {
View
5 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];
View
4 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

0 comments on commit 3b5ae9d

Please sign in to comment.
Something went wrong with that request. Please try again.