Skip to content

Commit

Permalink
[ios-sdk] Remove dependency on [UITableView allowsMultipleSelection]
Browse files Browse the repository at this point in the history
Summary:
We were using an API that was iOS 5.0+ only, and we want to support 4.3. We were only
half-relying on it anyway to determine whether to select or deselect an item. Now we
rely on the FBGraphObjectTableSelection class to know whether an item is selected or not,
rather than relying on the UITableView. This also fixes a bug I discovered while fixing
this one, where in non-multi-select mode, the friend picker would not correctly deselect
an item that had scrolled far off the screen (because the UITableView returned nil in
response to cellForRowAtIndexPath, causing us to skip the deselect).

In multi-select mode, we ignore deselect events entirely and treat select as a toggle,
much as before, except that we do not rely on calling cellForRowAtIndexPath.

In single-select mode, we do handle deselect events to ensure that only one item at a time
is selected. (We also treat select as a toggle, so selecting the same item twice deselects it.)

Test Plan:
- Ran the FriendPickerSample with the picker in both single and multi-select modes.
- Selected and unselected lots of items, both near each other and far apart in the list,
verifying that the selection state was as-expected each time.

Revert Plan:

Reviewers: jacl, mmarucheck, gregschechte, ayden

Reviewed By: jacl

CC: platform-diffs@lists

Differential Revision: https://phabricator.fb.com/D493114

Task ID: 1121147
  • Loading branch information
Chris Lang committed Jun 13, 2012
1 parent b67aec4 commit 07bd2cf
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 15 deletions.
6 changes: 3 additions & 3 deletions src/FBFriendPickerViewController.m
Expand Up @@ -159,8 +159,8 @@ - (BOOL)allowsMultipleSelection
- (void)setAllowsMultipleSelection:(BOOL)allowsMultipleSelection
{
_allowsMultipleSelection = allowsMultipleSelection;
if (self.isViewLoaded) {
self.tableView.allowsMultipleSelection = allowsMultipleSelection;
if (self.selectionManager) {
self.selectionManager.allowsMultipleSelection = allowsMultipleSelection;
}
}

Expand Down Expand Up @@ -218,7 +218,7 @@ - (void)viewDidLoad
[spinner release];
}

self.tableView.allowsMultipleSelection = self.allowsMultipleSelection;
self.selectionManager.allowsMultipleSelection = self.allowsMultipleSelection;
self.tableView.delegate = self.selectionManager;
[self.dataSource bindTableView:self.tableView];
self.loader.tableView = self.tableView;
Expand Down
1 change: 1 addition & 0 deletions src/FBGraphObjectTableSelection.h
Expand Up @@ -23,6 +23,7 @@

@property (nonatomic, assign) id<FBGraphObjectSelectionChangedDelegate> delegate;
@property (nonatomic, retain, readonly) NSArray *selection;
@property (nonatomic) BOOL allowsMultipleSelection;

- (id)initWithDataSource:(FBGraphObjectTableDataSource *)dataSource;

Expand Down
40 changes: 29 additions & 11 deletions src/FBGraphObjectTableSelection.m
Expand Up @@ -35,6 +35,7 @@ @implementation FBGraphObjectTableSelection
@synthesize dataSource = _dataSource;
@synthesize delegate = _delegate;
@synthesize selection = _selection;
@synthesize allowsMultipleSelection = _allowMultipleSelection;

- (id)initWithDataSource:(FBGraphObjectTableDataSource *)dataSource
{
Expand All @@ -44,7 +45,8 @@ - (id)initWithDataSource:(FBGraphObjectTableDataSource *)dataSource
dataSource.selectionDelegate = self;

self.dataSource = dataSource;

self.allowsMultipleSelection = YES;

NSArray *selection = [[NSArray alloc] init];
self.selection = selection;
[selection release];
Expand Down Expand Up @@ -98,33 +100,49 @@ - (void)selectionChanged
}
}

- (BOOL)selectionIncludesItem:(id<FBGraphObject>)item
{
return [FBUtility graphObjectInArray:self.selection withSameIDAs:item] != nil;
}

#pragma mark - FBGraphObjectSelectionDelegate

- (BOOL)graphObjectTableDataSource:(FBGraphObjectTableDataSource *)dataSource
selectionIncludesItem:(id<FBGraphObject>)item
{
return [FBUtility graphObjectInArray:self.selection withSameIDAs:item] != nil;
return [self selectionIncludesItem:item];
}

#pragma mark - UITableViewDelegate

- (void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath *)indexPath
{
UITableViewCell *cell = [tableView cellForRowAtIndexPath:indexPath];
if (cell) {
FBGraphObject *item = [self.dataSource itemAtIndexPath:indexPath];
if (cell.accessoryType == UITableViewCellAccessoryNone) {
[self selectItem:item cell:cell];
} else {
[self deselectItem:item cell:cell];
}
// cell may be nil, which is okay, it will pick up the right selected state when it is created.

FBGraphObject *item = [self.dataSource itemAtIndexPath:indexPath];

// We want to support multi-select on iOS <5.0, so rather than rely on the table view's notion
// of selection, just treat this as a toggle. If it is already selected, deselect it, and vice versa.
// Note: we only care about allowMultipleSelection in didDeselect, as that's what has to worry about
// de-selecting the previously-selected item, if any.
if (![self selectionIncludesItem:item]) {
[self selectItem:item cell:cell];
} else {
[self deselectItem:item cell:cell];
}
}

- (void)tableView:(UITableView *)tableView didDeselectRowAtIndexPath:(NSIndexPath *)indexPath
{
UITableViewCell *cell = [tableView cellForRowAtIndexPath:indexPath];
if (cell) {
if (self.allowsMultipleSelection == NO) {
// Only deselect if we are not allowing multi select. Otherwise, the user will manually
// deselect this item by clicking on it again.

// cell may be nil, which is okay, it will pick up the right selected state when it is created.
UITableViewCell *cell = [tableView cellForRowAtIndexPath:indexPath];
NSLog(@"didDeselect: indexPath = %@", indexPath.description);

FBGraphObject *item = [self.dataSource itemAtIndexPath:indexPath];
[self deselectItem:item cell:cell];
}
Expand Down
1 change: 0 additions & 1 deletion src/FBPlacePickerViewController.m
Expand Up @@ -206,7 +206,6 @@ - (void)viewDidLoad

if (!self.tableView) {
UITableView *tableView = [[UITableView alloc] initWithFrame:bounds];
tableView.allowsMultipleSelection = NO;
tableView.autoresizingMask =
UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight;

Expand Down

0 comments on commit 07bd2cf

Please sign in to comment.