Skip to content

Commit

Permalink
[ios-sdk] Fix bug in paging loader that led to multiple activity indi…
Browse files Browse the repository at this point in the history
…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
Chris Lang committed Jul 11, 2012
1 parent 6aca821 commit 3b5ae9d
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 12 deletions.
4 changes: 2 additions & 2 deletions src/FBFriendPickerCacheDescriptor.m
Expand Up @@ -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];
Expand Down
5 changes: 3 additions & 2 deletions src/FBFriendPickerViewController.m
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions src/FBGraphObjectPagingLoader.h
Expand Up @@ -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;
Expand Down
13 changes: 10 additions & 3 deletions src/FBGraphObjectPagingLoader.m
Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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 {
Expand Down
5 changes: 3 additions & 2 deletions src/FBPlacePickerCacheDescriptor.m
Expand Up @@ -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];
Expand Down
4 changes: 3 additions & 1 deletion src/FBPlacePickerViewController.m
Expand Up @@ -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
Expand Down

0 comments on commit 3b5ae9d

Please sign in to comment.