[Payment Request] Picker view + showcase integration + egtests

ios/chrome/browser/payments/payment_request_picker_view_controller.mm

Index: ios/chrome/browser/payments/payment_request_picker_view_controller.mm
diff --git a/ios/chrome/browser/payments/payment_request_picker_view_controller.mm b/ios/chrome/browser/payments/payment_request_picker_view_controller.mm
new file mode 100644
index 0000000000000000000000000000000000000000..7131852bc0d4f89a9adf467ce20c7440ff7b8e70
--- /dev/null
+++ b/ios/chrome/browser/payments/payment_request_picker_view_controller.mm
@@ -0,0 +1,224 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "ios/chrome/browser/payments/payment_request_picker_view_controller.h"
+
+#import "base/logging.h"
+#import "ios/chrome/browser/payments/payment_request_picker_row.h"
+#import "ios/third_party/material_components_ios/src/components/CollectionCells/src/MaterialCollectionCells.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+NSString* const kPaymentRequestPickerRowAccessibilityID =
+    @"kPaymentRequestPickerRowAccessibilityID";
+NSString* const kPaymentRequestPickerSelectedRowAccessibilityID =
+    @"kPaymentRequestPickerSelectedRowAccessibilityID";
+NSString* const kPaymentRequestPickerSearchBarAccessibilityID =
+    @"kPaymentRequestPickerSearchBarAccessibilityID";
+
+namespace {
+NSString* const kPaymentRequestPickerViewControllerAccessibilityID =
+    @"kPaymentRequestPickerViewControllerAccessibilityID";
+

[lpromero, 2017/03/29 11:38:15]

Remove empty line.

[Moe, 2017/03/29 17:04:09]

Done.

+}  // namespace
+
+@interface PaymentRequestPickerViewController ()<UITableViewDataSource,
+                                                 UITableViewDelegate,

[lpromero, 2017/03/29 11:38:15]

This is already done by the base class.

[Moe, 2017/03/29 17:04:09]

Done.

+                                                 UISearchResultsUpdating>
+
+// Search controller that contains search bar.
+@property(nonatomic, strong) UISearchController* searchController;
+
+// Full data set displayed when tableView is not filtered.
+@property(nonatomic, strong) NSArray<PickerRow*>* allRows;
+
+// Displayed rows in the tableView.
+@property(nonatomic, strong) NSArray<PickerRow*>* displayedRows;
+
+// Selected row.
+@property(nonatomic, strong) PickerRow* selectedRow;
+
+@end
+
+@implementation PaymentRequestPickerViewController
+@synthesize searchController = _searchController;
+@synthesize allRows = _allRows;
+@synthesize displayedRows = _displayedRows;
+@synthesize selectedRow = _selectedRow;
+@synthesize delegate = _delegate;
+
+- (instancetype)initWithRows:(NSArray<PickerRow*>*)rows
+                    selected:(PickerRow*)selectedRow {
+  self = [super initWithStyle:UITableViewStylePlain];
+  if (self) {
+    self.allRows = [rows sortedArrayUsingComparator:^NSComparisonResult(
+                             PickerRow* row1, PickerRow* row2) {
+      return [row1.label localizedCaseInsensitiveCompare:row2.label];
+    }];
+    self.selectedRow = selectedRow;
+    // Default to displaying all the rows.
+    self.displayedRows = self.allRows;
+  }
+  return self;
+}
+
+#pragma mark - UIViewController
+
+- (void)viewDidLoad {
+  [super viewDidLoad];
+
+  self.tableView.rowHeight = MDCCellDefaultOneLineHeight;

[lpromero, 2017/03/29 11:38:15]

I'd suggest just setting your own value here, not depending on MDC. The imported
header can be removed then.

[Moe, 2017/03/29 17:04:09]

Done.

+  self.tableView.accessibilityIdentifier =
+      kPaymentRequestPickerViewControllerAccessibilityID;
+
+  self.searchController =
+      [[UISearchController alloc] initWithSearchResultsController:nil];
+  self.searchController.searchResultsUpdater = self;
+  self.searchController.dimsBackgroundDuringPresentation = NO;
+  self.searchController.searchBar.accessibilityIdentifier =
+      kPaymentRequestPickerSearchBarAccessibilityID;
+  self.tableView.tableHeaderView = self.searchController.searchBar;
+
+  // Presentation of searchController will walk up the view controller hierarchy
+  // until it finds the root view controller or one that defines a presentation
+  // context. Make this class the presentation context so that the search
+  // controller does not present on top of the navigation controller.
+  self.definesPresentationContext = YES;
+}
+
+#pragma mark - UITableViewDataSource
+
+- (NSArray<NSString*>*)sectionIndexTitlesForTableView:(UITableView*)tableView {
+  return [self sectionTitles];
+}
+
+- (NSInteger)numberOfSectionsInTableView:(UITableView*)tableView {
+  return [[self sectionTitles] count];
+}
+
+- (NSString*)tableView:(UITableView*)tableView
+    titleForHeaderInSection:(NSInteger)section {
+  return [[self sectionTitles] objectAtIndex:section];
+}
+
+- (NSInteger)tableView:(UITableView*)tableView
+    numberOfRowsInSection:(NSInteger)section {
+  return [[self rowsInSection:section] count];
+}
+
+- (UITableViewCell*)tableView:(UITableView*)tableView
+        cellForRowAtIndexPath:(NSIndexPath*)indexPath {
+  UITableViewCell* cell = [tableView dequeueReusableCellWithIdentifier:@"cell"];
+  if (!cell) {
+    cell = [[UITableViewCell alloc] initWithStyle:UITableViewCellStyleSubtitle
+                                  reuseIdentifier:@"cell"];
+  }
+  PickerRow* row =
+      [[self rowsInSection:indexPath.section] objectAtIndex:indexPath.row];
+  cell.textLabel.text = row.label;
+  cell.accessoryType = (row == self.selectedRow)
+                           ? UITableViewCellAccessoryCheckmark
+                           : UITableViewCellAccessoryNone;
+  cell.accessibilityLabel = row.label;
+  cell.accessibilityIdentifier =

[lpromero, 2017/03/29 11:38:15]

If you remove the need for the 2 different IDs, you can set just one, around
line 117, since it won't change.

[Moe, 2017/03/29 17:04:09]

Done.

+      (row == self.selectedRow)
+          ? kPaymentRequestPickerSelectedRowAccessibilityID
+          : kPaymentRequestPickerRowAccessibilityID;
+  return cell;
+}
+
+#pragma mark - UITableViewDelegate
+
+- (void)tableView:(UITableView*)tableView
+    didSelectRowAtIndexPath:(NSIndexPath*)indexPath {
+  if (self.selectedRow) {
+    NSIndexPath* oldSelectedIndexPath = [self indexPathForRow:self.selectedRow];
+    self.selectedRow = nil;
+    // Reload the previously selected row.
+    [self.tableView reloadRowsAtIndexPaths:@[ oldSelectedIndexPath ]
+                          withRowAnimation:UITableViewRowAnimationFade];
+  }
+
+  self.selectedRow =
+      [[self rowsInSection:indexPath.section] objectAtIndex:indexPath.row];
+  NSIndexPath* newSelectedIndexPath = [self indexPathForRow:self.selectedRow];

[lpromero, 2017/03/29 11:38:15]

Isn't this equal to indexPath?

[Moe, 2017/03/29 17:04:09]

Oops. Done.

+  // Reload the newly selected row.
+  [self.tableView reloadRowsAtIndexPaths:@[ newSelectedIndexPath ]
+                        withRowAnimation:UITableViewRowAnimationFade];
+
+  [_delegate paymentRequestPickerViewController:self
+                                   didSelectRow:self.selectedRow];
+}
+
+#pragma mark - UISearchResultsUpdating
+
+- (void)updateSearchResultsForSearchController:
+    (UISearchController*)searchController {
+  [self filterContentForSearchText:searchController.searchBar.text];
+}
+
+#pragma mark - Private
+
+// Returns the indexPath for |row| by calculating its section and its index
+// within the section.
+- (NSIndexPath*)indexPathForRow:(PickerRow*)row {
+  NSString* sectionTitle = [self sectionTitleForRow:row];
+  NSInteger section = [[self sectionTitles] indexOfObject:sectionTitle];
+  DCHECK(section != NSNotFound);
+
+  NSInteger indexInSection = [[self rowsInSection:section] indexOfObject:row];
+  DCHECK(indexInSection != NSNotFound);
+
+  return [NSIndexPath indexPathForRow:indexInSection inSection:section];
+}
+
+// Returns the titles for the displayed sections in the tableView.
+- (NSArray<NSString*>*)sectionTitles {
+  NSMutableOrderedSet<NSString*>* sectionTitles =
+      [[NSMutableOrderedSet alloc] init];
+  [self.displayedRows
+      enumerateObjectsUsingBlock:^(PickerRow* row, NSUInteger idx, BOOL* stop) {
+        [sectionTitles addObject:[self sectionTitleForRow:row]];
+      }];
+  return [sectionTitles array];

[lpromero, 2017/03/29 11:38:15]

Should you cache this array? (You'll need to invalidate when displayedRows
changes.)
I guess if the list scrolls smoothly already on old devices, we can avoid
optimizing.

[Moe, 2017/03/29 17:04:09]

I'm now keeping a map of section titles to the section rows. This method now
returns the sorted keys which is pretty lightweight.

+}
+
+// Returns the displayed rows in the given section.
+- (NSArray<PickerRow*>*)rowsInSection:(NSInteger)section {
+  NSArray<NSString*>* sectionTitles = [self sectionTitles];
+  DCHECK(section >= 0 && section < static_cast<NSInteger>(sectionTitles.count));
+
+  NSString* sectionTitle = [sectionTitles objectAtIndex:section];
+
+  // Find the rows in the section with the title |sectionTitle| based on the
+  // labels of the rows. The search is case-insensitive and ignores diacritics.
+  NSPredicate* prediacate = [NSPredicate

[lpromero, 2017/03/29 11:38:15]

*predicate

[Moe, 2017/03/29 17:04:09]

Done.

+      predicateWithFormat:@"label BEGINSWITH[cd] %@", sectionTitle];
+  return [self.displayedRows filteredArrayUsingPredicate:prediacate];

[lpromero, 2017/03/29 11:38:15]

idem, should this be cached? It is called several times, and at least once per
cell being shown on screen.

[Moe, 2017/03/29 17:04:09]

with the map of section titles to the section rows, this is now a very
lightweight method. 

+}
+
+// Returns the title for the section the given row gets added to. The section
+// title for a row is the capitalized first letter of the label for that row.
+- (NSString*)sectionTitleForRow:(PickerRow*)row {
+  return [[row.label substringToIndex:1] uppercaseString];
+}
+
+// Filters |allRows| for |searchText| and reloads the tableView. If |searchText|
+// is empty, tableView will be loaded with |allRows|.
+- (void)filterContentForSearchText:(NSString*)searchText {
+  self.displayedRows = self.allRows;
+
+  if (searchText.length != 0) {
+    // The search is case-insensitive and ignores diacritics.
+    NSPredicate* prediacate =
+        [NSPredicate predicateWithFormat:@"label CONTAINS[cd] %@", searchText];
+    self.displayedRows = [self.allRows filteredArrayUsingPredicate:prediacate];
+  }
+
+  [self.tableView reloadData];

[lpromero, 2017/03/29 11:38:15]

Showcase handles that more nicely, as it animates the cells in and out:

- (void)filterContentForSearchText:(NSString*)searchText {
  NSArray<showcase::ModelRow*>* newFilteredRows = self.allRows;

  if (![self.searchController.searchBar.text isEqualToString:@""]) {
    // The search is case-insensitive and searches both displayed texts.
    NSIndexSet* matchingRows =
        [self.allRows indexesOfObjectsPassingTest:^BOOL(
                          showcase::ModelRow* row, NSUInteger idx, BOOL* stop) {
          return [row[showcase::kClassForDisplayKey]
                     localizedCaseInsensitiveContainsString:searchText] ||
                 [row[showcase::kUseCaseKey]
                     localizedCaseInsensitiveContainsString:searchText];
        }];
    newFilteredRows = [self.allRows objectsAtIndexes:matchingRows];
  }

  NSArray<NSIndexPath*>* indexPathsToInsert =
      [self indexPathsToInsertFromArray:self.displayedRows
                                toArray:newFilteredRows
                       indexPathSection:0];
  NSArray<NSIndexPath*>* indexPathsToDelete =
      [self indexPathsToDeleteFromArray:self.displayedRows
                                toArray:newFilteredRows
                       indexPathSection:0];

  [self.tableView beginUpdates];
  [self.tableView insertRowsAtIndexPaths:indexPathsToInsert
                        withRowAnimation:UITableViewRowAnimationFade];
  [self.tableView deleteRowsAtIndexPaths:indexPathsToDelete
                        withRowAnimation:UITableViewRowAnimationFade];
  self.displayedRows = newFilteredRows;
  [self.tableView endUpdates];
}

[Moe, 2017/03/29 17:04:09]

Since we have sections here and they would have to get removed or added
accordingly, I imagine the logic to animate the cells in and out would be a lot
more involved. This screen will be used for picking a country when editing a
credit card. It simply isn't important enough to justify the effort.  

+}
+
+@end