Add/Edit contacts in group list

* properly populate phone fields
* Fix appearence for CNContactViewController
* "Add Contact" vs "Show Contact"

// FREEBIE

Signal/src/ViewControllers/OWSTableViewController.m

@@ -277,6 +277,7 @@ NSString * const kOWSTableCellIdentifier = @"kOWSTableCellIdentifier";
 - (void)setContents:(OWSTableContents *)contents
 {
     OWSAssert(contents);
+    AssertIsOnMainThread();
 
     _contents = contents;
 

Signal/src/ViewControllers/ShowGroupMembersViewController.m

@@ -18,9 +18,11 @@
 #import <SignalServiceKit/TSGroupModel.h>
 #import <SignalServiceKit/TSGroupThread.h>
 
+@import ContactsUI;
+
 NS_ASSUME_NONNULL_BEGIN
 
-@interface ShowGroupMembersViewController () <ContactsViewHelperDelegate>
+@interface ShowGroupMembersViewController () <ContactsViewHelperDelegate, CNContactViewControllerDelegate>
 
 @property (nonatomic, readonly) TSGroupThread *thread;
 @property (nonatomic, readonly) ContactsViewHelper *contactsViewHelper;
@@ -78,13 +80,25 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (void)viewDidLoad {
     [super viewDidLoad];
-    [self.navigationController.navigationBar setTranslucent:NO];
+
+    // HACK otherwise CNContactViewController Navbar is shown as black.
+    // RADAR rdar://28433898 http://www.openradar.me/28433898
+    // CNContactViewController incompatible with opaque navigation bar
+    [self.navigationController.navigationBar setTranslucent:YES];
 
     self.title = _thread.groupModel.groupName;
 
     [self updateTableContents];
 }
 
+- (void)viewWillAppear:(BOOL)animated
+{
+    [super viewWillAppear:animated];
+
+    // In case we're dismissing a CNContactViewController which requires default system appearance
+    [UIUtil applySignalAppearence];
+}
+
 #pragma mark - Table Contents
 
 - (void)updateTableContents
@@ -143,13 +157,15 @@ NS_ASSUME_NONNULL_BEGIN
     UIAlertController *actionSheetController =
         [UIAlertController alertControllerWithTitle:nil message:nil preferredStyle:UIAlertControllerStyleActionSheet];
 
-    [actionSheetController
+    NSString *contactInfoTitle = signalAccount
-        addAction:[UIAlertAction actionWithTitle:NSLocalizedString(@"GROUP_MEMBERS_VIEW_CONTACT_INFO",
+        ? NSLocalizedString(@"GROUP_MEMBERS_VIEW_CONTACT_INFO", @"Button label for the 'show contact info' button")
-                                                     @"Button label for the 'show contact info' button")
+        : NSLocalizedString(
-                                           style:UIAlertActionStyleDefault
+              @"GROUP_MEMBERS_ADD_CONTACT_INFO", @"Button label to add information to an unknown contact");
-                                         handler:^(UIAlertAction *_Nonnull action) {
+    [actionSheetController addAction:[UIAlertAction actionWithTitle:contactInfoTitle
-                                             [self showContactInfoViewForRecipientId:recipientId];
+                                                              style:UIAlertActionStyleDefault
-                                         }]];
+                                                            handler:^(UIAlertAction *_Nonnull action) {
+                                                                [self showContactInfoViewForRecipientId:recipientId];
+                                                            }]];
 
     BOOL isBlocked;
     if (signalAccount) {
@@ -280,37 +296,43 @@ NS_ASSUME_NONNULL_BEGIN
         [self presentViewController:alertController animated:YES completion:nil];
         return;
     }
-
+    
+    CNContactViewController *_Nullable contactViewController;
     if (signalAccount) {
-        // FIXME This is broken until converted to Contacts framework.
+        CNContact *_Nullable cnContact = signalAccount.contact.cnContact;
-        ABPersonViewController *view = [[ABPersonViewController alloc] init];
+        if (cnContact) {
-
+            contactViewController = [CNContactViewController viewControllerForContact:cnContact];
-        ABAddressBookRef addressBookRef = ABAddressBookCreateWithOptions(NULL, nil);
+        }
-        // Assume person is already defined.
+    }
-        view.displayedPerson = ABAddressBookGetPersonWithRecordID(addressBookRef, signalAccount.contact.recordID);
-        view.allowsActions = NO;
-        view.allowsEditing = YES;
 
-        [self.navigationController pushViewController:view animated:YES];
+    if (!contactViewController) {
-    } else {
+        CNMutableContact *newContact = [CNMutableContact new];
-        // FIXME This is broken until converted to Contacts framework.
+        CNPhoneNumber *phoneNumber = [CNPhoneNumber phoneNumberWithStringValue:recipientId];
-        ABUnknownPersonViewController *view = [[ABUnknownPersonViewController alloc] init];
+        CNLabeledValue<CNPhoneNumber *> *labeledPhoneNumber = [CNLabeledValue labeledValueWithLabel:CNLabelPhoneNumberMain
+                                                                             value:phoneNumber];
+        newContact.phoneNumbers = @[labeledPhoneNumber];
+        
+        contactViewController = [CNContactViewController viewControllerForNewContact:newContact];
+    }
 
-        ABRecordRef aContact = ABPersonCreate();
+    contactViewController.delegate = self;
-        CFErrorRef anError   = NULL;
+    contactViewController.allowsActions = NO;
+    contactViewController.allowsEditing = YES;
+    contactViewController.navigationItem.leftBarButtonItem =
+        [[UIBarButtonItem alloc] initWithTitle:NSLocalizedString(@"TXT_CANCEL_TITLE", nil)
+                                         style:UIBarButtonItemStylePlain
+                                        target:self
+                                        action:@selector(dismissPressed)];
 
-        ABMultiValueRef phone = ABMultiValueCreateMutable(kABMultiStringPropertyType);
+    UINavigationController *navigationController =
-        ABMultiValueAddValueAndLabel(phone, (__bridge CFTypeRef)recipientId, kABPersonPhoneMainLabel, NULL);
+        [[UINavigationController alloc] initWithRootViewController:contactViewController];
+    [self presentViewController:navigationController animated:YES completion:nil];
 
-        ABRecordSetValue(aContact, kABPersonPhoneProperty, phone, &anError);
-        CFRelease(phone);
 
-        if (!anError && aContact) {
+    // HACK otherwise CNContactViewController Navbar is shown as black.
-            view.displayedPerson = aContact; // Assume person is already defined.
+    // RADAR rdar://28433898 http://www.openradar.me/28433898
-            view.allowsAddingToAddressBook = YES;
+    // CNContactViewController incompatible with opaque navigation bar
-            [self.navigationController pushViewController:view animated:YES];
+    [UIUtil applyDefaultSystemAppearence];
-        }
-    }
 }
 
 - (void)showConversationViewForRecipientId:(NSString *)recipientId
@@ -325,10 +347,20 @@ NS_ASSUME_NONNULL_BEGIN
     [Environment callUserWithIdentifier:recipientId];
 }
 
+- (void)dismissPressed
+{
+    DDLogDebug(@"%@ %s", self.tag, __PRETTY_FUNCTION__);
+    [self dismissViewControllerAnimated:YES completion:nil];
+}
+
 #pragma mark - ContactsViewHelperDelegate
 
 - (void)contactsViewHelperDidUpdateContacts
 {
+    // FIXME new names are not immediately chown.
+    // doing some debugging, it seems that even though [self.tableView reloadData] is called
+    // we don't see subsequent invocations of `cellForIndexpath:` unless the items are scrolled off screen.
+    
     [self updateTableContents];
 }
 
@@ -337,6 +369,27 @@ NS_ASSUME_NONNULL_BEGIN
     return YES;
 }
 
+#pragma mark - CNContactViewControllerDelegate
+
+- (void)contactViewController:(CNContactViewController *)viewController
+       didCompleteWithContact:(nullable CNContact *)contact
+{
+    DDLogDebug(@"%@ done editing contact.", self.tag);
+    [self dismissViewControllerAnimated:YES completion:nil];
+}
+
+#pragma mark - Logging
+
++ (NSString *)tag
+{
+    return [NSString stringWithFormat:@"[%@]", self.class];
+}
+
+- (NSString *)tag
+{
+    return self.class.tag;
+}
+
 @end
 
 NS_ASSUME_NONNULL_END

Signal/src/contact/SystemContactsFetcher.swift

@@ -4,6 +4,7 @@
 
 import Foundation
 import Contacts
+import ContactsUI
 
 @objc protocol SystemContactsFetcherDelegate: class {
     func systemContactsFetcher(_ systemContactsFetcher: SystemContactsFetcher, updatedContacts contacts: [Contact])
@@ -36,7 +37,8 @@ class SystemContactsFetcher: NSObject {
         CNContactFormatter.descriptorForRequiredKeys(for: .fullName),
         CNContactThumbnailImageDataKey as CNKeyDescriptor, // TODO full image instead of thumbnail?
         CNContactPhoneNumbersKey as CNKeyDescriptor,
-        CNContactEmailAddressesKey as CNKeyDescriptor
+        CNContactEmailAddressesKey as CNKeyDescriptor,
+        CNContactViewController.descriptorForRequiredKeys()
     ]
 
     /**
@@ -106,14 +108,11 @@ class SystemContactsFetcher: NSObject {
 
         systemContactsHaveBeenRequestedAtLeastOnce = true
 
-        let contactStore = self.contactStore
-        let allowedContactKeys = self.allowedContactKeys
-
         DispatchQueue.global().async {
             var systemContacts = [CNContact]()
             do {
-                let contactFetchRequest = CNContactFetchRequest(keysToFetch: allowedContactKeys)
+                let contactFetchRequest = CNContactFetchRequest(keysToFetch: self.allowedContactKeys)
-                try contactStore.enumerateContacts(with: contactFetchRequest) { (contact, _) -> Void in
+                try self.contactStore.enumerateContacts(with: contactFetchRequest) { (contact, _) -> Void in
                     systemContacts.append(contact)
                 }
             } catch let error as NSError {

Signal/src/util/UIUtil.m

@@ -33,6 +33,7 @@
 {
     [[UIApplication sharedApplication] setStatusBarStyle:UIStatusBarStyleDefault];
     [[UINavigationBar appearance] setBarStyle:UIBarStyleDefault];
+    [[UINavigationBar appearance] setTintColor:[UIColor blackColor]];
     [[UIBarButtonItem appearance] setTintColor:[UIColor blackColor]];
     [[UINavigationBar appearance] setTitleTextAttributes:@{
                                                            NSForegroundColorAttributeName : [UIColor blackColor],

Signal/translations/en.lproj/Localizable.strings

@@ -520,6 +520,9 @@
 /* No comment provided by engineer. */
 "GROUP_MEMBER_LEFT" = " %@ left the group. ";
 
+/* Button label to add information to an unknown contact */
+"GROUP_MEMBERS_ADD_CONTACT_INFO" = "Add Contact";
+
 /* Button label for the 'call group member' button */
 "GROUP_MEMBERS_CALL" = "Call";