From 40dead89e5badd2b1b4397c1ea08f662ef7cb9e6 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Fri, 5 May 2017 17:14:12 -0400 Subject: [PATCH 1/9] don't crash invite flow when contacts disabled // FREEBIE --- Signal/src/ViewControllers/ContactsPicker.swift | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Signal/src/ViewControllers/ContactsPicker.swift b/Signal/src/ViewControllers/ContactsPicker.swift index 1c8297e45..068c6f04d 100644 --- a/Signal/src/ViewControllers/ContactsPicker.swift +++ b/Signal/src/ViewControllers/ContactsPicker.swift @@ -274,7 +274,15 @@ open class ContactsPicker: UIViewController, UITableViewDelegate, UITableViewDat open func tableView(_ tableView: UITableView, titleForHeaderInSection section: Int) -> String? { let dataSource = filteredSections + guard dataSource.count >= section else { + return nil + } + if dataSource[section].count > 0 { + guard collation.sectionTitles.count >= section else { + return nil + } + return collation.sectionTitles[section] } else { return nil From bf5b6d1e6363a4d4d52bddb2d980e32c36e471d6 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Fri, 5 May 2017 17:34:37 -0400 Subject: [PATCH 2/9] Invite Flow when "no contact" TODO: we should probably just prevent people from getting to the invite flow when their contacts aren't shared, but still it seems good to fix these crashes. // FREEBIE --- .../src/ViewControllers/ContactsPicker.swift | 29 +++++++++++++------ .../MessageComposeTableViewController.m | 6 +++- .../translations/en.lproj/Localizable.strings | 15 +++++----- 3 files changed, 32 insertions(+), 18 deletions(-) diff --git a/Signal/src/ViewControllers/ContactsPicker.swift b/Signal/src/ViewControllers/ContactsPicker.swift index 068c6f04d..a1bdcba71 100644 --- a/Signal/src/ViewControllers/ContactsPicker.swift +++ b/Signal/src/ViewControllers/ContactsPicker.swift @@ -145,20 +145,27 @@ open class ContactsPicker: UIViewController, UITableViewDelegate, UITableViewDat func getContacts(onError errorHandler: @escaping (_ error: Error) -> Void) { switch CNContactStore.authorizationStatus(for: CNEntityType.contacts) { case CNAuthorizationStatus.denied, CNAuthorizationStatus.restricted: + let title = NSLocalizedString("INVITE_FLOW_REQUIRES_CONTACT_ACCESS_TITLE", comment: "Alert title when contacts disabled while trying to invite contacts to signal") + let body = NSLocalizedString("INVITE_FLOW_REQUIRES_CONTACT_ACCESS_BODY", comment: "Alert body when contacts disabled while trying to invite contacts to signal") - let title = NSLocalizedString("AB_PERMISSION_MISSING_TITLE", comment: "Alert title when contacts disabled") - let body = NSLocalizedString("ADDRESSBOOK_RESTRICTED_ALERT_BODY", comment: "Alert body when contacts disabled") let alert = UIAlertController(title: title, message: body, preferredStyle: UIAlertControllerStyle.alert) - let dismissText = NSLocalizedString("DISMISS_BUTTON_TEXT", comment:"") + let dismissText = NSLocalizedString("TXT_CANCEL_TITLE", comment:"") - let okAction = UIAlertAction(title: dismissText, style: UIAlertActionStyle.default, handler: { _ in + let cancelAction = UIAlertAction(title: dismissText, style: .cancel, handler: { _ in let error = NSError(domain: "contactsPickerErrorDomain", code: 1, userInfo: [NSLocalizedDescriptionKey: "No Contacts Access"]) self.contactsPickerDelegate?.contactsPicker(self, didContactFetchFailed: error) errorHandler(error) self.dismiss(animated: true, completion: nil) }) - alert.addAction(okAction) + alert.addAction(cancelAction) + + let settingsText = NSLocalizedString("OPEN_SETTINGS_BUTTON", comment:"Button text which opens the settings app") + let openSettingsAction = UIAlertAction(title: settingsText, style: .default, handler: { (_) in + UIApplication.shared.openSystemSettings() + }) + alert.addAction(openSettingsAction) + self.present(alert, animated: true, completion: nil) case CNAuthorizationStatus.notDetermined: @@ -208,6 +215,10 @@ open class ContactsPicker: UIViewController, UITableViewDelegate, UITableViewDat open func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int { let dataSource = filteredSections + guard section < dataSource.count else { + return 0 + } + return dataSource[section].count } @@ -274,15 +285,15 @@ open class ContactsPicker: UIViewController, UITableViewDelegate, UITableViewDat open func tableView(_ tableView: UITableView, titleForHeaderInSection section: Int) -> String? { let dataSource = filteredSections - guard dataSource.count >= section else { + guard section < dataSource.count else { return nil } - + if dataSource[section].count > 0 { - guard collation.sectionTitles.count >= section else { + guard section < collation.sectionTitles.count else { return nil } - + return collation.sectionTitles[section] } else { return nil diff --git a/Signal/src/ViewControllers/MessageComposeTableViewController.m b/Signal/src/ViewControllers/MessageComposeTableViewController.m index 94a95f40a..00cb744d6 100644 --- a/Signal/src/ViewControllers/MessageComposeTableViewController.m +++ b/Signal/src/ViewControllers/MessageComposeTableViewController.m @@ -151,7 +151,7 @@ NS_ASSUME_NONNULL_BEGIN UIButton *inviteContactsButton = [UIButton buttonWithType:UIButtonTypeCustom]; [inviteContactsButton setTitle:NSLocalizedString(@"INVITE_FRIENDS_CONTACT_TABLE_BUTTON", - "Text for button at the top of the contact picker") + "Label for the cell that presents the 'invite contacts' workflow.") forState:UIControlStateNormal]; [inviteContactsButton setTitleColor:[UIColor ows_materialBlueColor] forState:UIControlStateNormal]; [inviteContactsButton.titleLabel setFont:[UIFont ows_regularFontWithSize:17.f]]; @@ -420,6 +420,10 @@ NS_ASSUME_NONNULL_BEGIN - (void)showNoContactsModeIfNecessary { + if (!self.contactsViewHelper.contactsManager.isSystemContactsAuthorized) { + return; + } + BOOL hasNoContacts = self.contactsViewHelper.signalAccounts.count < 1; self.isNoContactsModeActive = (hasNoContacts && ![[Environment preferences] hasDeclinedNoContactsView]); } diff --git a/Signal/translations/en.lproj/Localizable.strings b/Signal/translations/en.lproj/Localizable.strings index 6d2f1589b..ab6db1a7e 100644 --- a/Signal/translations/en.lproj/Localizable.strings +++ b/Signal/translations/en.lproj/Localizable.strings @@ -1,9 +1,6 @@ /* Button text to dismiss missing contacts permission alert */ "AB_PERMISSION_MISSING_ACTION_NOT_NOW" = "Not Now"; -/* Alert title when contacts disabled */ -"AB_PERMISSION_MISSING_TITLE" = "Sorry!"; - /* Action sheet item */ "ACCEPT_NEW_IDENTITY_ACTION" = "Accept new safety number"; @@ -22,9 +19,6 @@ /* Title for the 'add group member' view. */ "ADD_GROUP_MEMBER_VIEW_TITLE" = "Add Member"; -/* Alert body when contacts disabled */ -"ADDRESSBOOK_RESTRICTED_ALERT_BODY" = "Signal requires access to your contacts. Access to contacts is restricted. Signal will close. You can disable the restriction temporarily to let Signal access your contacts by going the Settings app >> General >> Restrictions >> Contacts >> Allow Changes."; - /* The label for the 'discard' button in alerts and action sheets. */ "ALERT_DISCARD_BUTTON" = "Discard"; @@ -580,8 +574,13 @@ /* No comment provided by engineer. */ "INCOMING_INCOMPLETE_CALL" = "Incomplete incoming call from"; -/* Label for the cell that presents the 'invite contacts' workflow. - Text for button at the top of the contact picker */ +/* Alert body when contacts disabled while trying to invite contacts to signal */ +"INVITE_FLOW_REQUIRES_CONTACT_ACCESS_BODY" = "To invite your contacts, you need to allow Signal access to your contacts in the Settings app."; + +/* Alert title when contacts disabled while trying to invite contacts to signal */ +"INVITE_FLOW_REQUIRES_CONTACT_ACCESS_TITLE" = "Enable Contact Access"; + +/* Label for the cell that presents the 'invite contacts' workflow. */ "INVITE_FRIENDS_CONTACT_TABLE_BUTTON" = "Invite Friends to Signal"; /* Search */ From 0b6962cdd08c49f81c72a5fb70438c1d6306987b Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Fri, 5 May 2017 18:05:14 -0400 Subject: [PATCH 3/9] contacts reminder in compose view // FREEBIE --- .../MessageComposeTableViewController.m | 58 +++++++++++++------ .../ViewControllers/SignalsViewController.m | 3 +- Signal/src/views/ReminderView.swift | 3 +- .../translations/en.lproj/Localizable.strings | 5 +- 4 files changed, 49 insertions(+), 20 deletions(-) diff --git a/Signal/src/ViewControllers/MessageComposeTableViewController.m b/Signal/src/ViewControllers/MessageComposeTableViewController.m index 00cb744d6..181363447 100644 --- a/Signal/src/ViewControllers/MessageComposeTableViewController.m +++ b/Signal/src/ViewControllers/MessageComposeTableViewController.m @@ -34,6 +34,7 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, readonly) OWSTableViewController *tableViewController; @property (nonatomic, readonly) UISearchBar *searchBar; +@property (nonatomic, readonly) NSLayoutConstraint *hideContactsPermissionReminderViewConstraint; // A list of possible phone numbers parsed from the search text as // E164 values. @@ -59,6 +60,18 @@ NS_ASSUME_NONNULL_BEGIN _contactsViewHelper.delegate = self; _nonContactAccountSet = [NSMutableSet set]; + ReminderView *contactsPermissionReminderView = [[ReminderView alloc] + initWithText:NSLocalizedString(@"COMPOSE_SCREEN_MISSING_CONTACTS_PERMISSION", + @"Multiline label explaining why compose-screen contact picker is empty.") + tapAction:^{ + [[UIApplication sharedApplication] openSystemSettings]; + }]; + [self.view addSubview:contactsPermissionReminderView]; + [contactsPermissionReminderView autoPinWidthToSuperview]; + [contactsPermissionReminderView autoPinEdgeToSuperviewMargin:ALEdgeTop]; + _hideContactsPermissionReminderViewConstraint = + [contactsPermissionReminderView autoSetDimension:ALDimensionHeight toSize:0]; + self.navigationItem.leftBarButtonItem = [[UIBarButtonItem alloc] initWithBarButtonSystemItem:UIBarButtonSystemItemStop target:self @@ -87,7 +100,8 @@ NS_ASSUME_NONNULL_BEGIN _tableViewController.tableViewStyle = UITableViewStylePlain; [self.view addSubview:self.tableViewController.view]; [_tableViewController.view autoPinWidthToSuperview]; - [_tableViewController.view autoPinEdgeToSuperviewEdge:ALEdgeTop]; + + [_tableViewController.view autoPinEdge:ALEdgeTop toEdge:ALEdgeBottom ofView:contactsPermissionReminderView]; [_tableViewController.view autoPinToBottomLayoutGuideOfViewController:self withInset:0]; _tableViewController.tableView.tableHeaderView = searchBar; @@ -101,6 +115,11 @@ NS_ASSUME_NONNULL_BEGIN [self updateTableContents]; } +- (void)showContactsPermissionReminder:(BOOL)flag +{ + _hideContactsPermissionReminderViewConstraint.active = !flag; +} + - (UIView *)createNoSignalContactsView { UIView *view = [UIView new]; @@ -357,18 +376,20 @@ NS_ASSUME_NONNULL_BEGIN if (!hasSearchText && helper.signalAccounts.count < 1) { // No Contacts - [section addItem:[OWSTableItem itemWithCustomCellBlock:^{ - UITableViewCell *cell = [UITableViewCell new]; - cell.textLabel.text = NSLocalizedString( - @"SETTINGS_BLOCK_LIST_NO_CONTACTS", @"A label that indicates the user has no Signal contacts."); - cell.textLabel.font = [UIFont ows_regularFontWithSize:15.f]; - cell.textLabel.textColor = [UIColor colorWithWhite:0.5f alpha:1.f]; - cell.textLabel.textAlignment = NSTextAlignmentCenter; - cell.selectionStyle = UITableViewCellSelectionStyleNone; - return cell; + if (self.contactsViewHelper.contactsManager.isSystemContactsAuthorized) { + [section addItem:[OWSTableItem itemWithCustomCellBlock:^{ + UITableViewCell *cell = [UITableViewCell new]; + cell.textLabel.text = NSLocalizedString( + @"SETTINGS_BLOCK_LIST_NO_CONTACTS", @"A label that indicates the user has no Signal contacts."); + cell.textLabel.font = [UIFont ows_regularFontWithSize:15.f]; + cell.textLabel.textColor = [UIColor colorWithWhite:0.5f alpha:1.f]; + cell.textLabel.textAlignment = NSTextAlignmentCenter; + cell.selectionStyle = UITableViewCellSelectionStyleNone; + return cell; + } + customRowHeight:kActionCellHeight + actionBlock:nil]]; } - customRowHeight:kActionCellHeight - actionBlock:nil]]; } if (hasSearchText && !hasSearchResults) { @@ -420,12 +441,15 @@ NS_ASSUME_NONNULL_BEGIN - (void)showNoContactsModeIfNecessary { - if (!self.contactsViewHelper.contactsManager.isSystemContactsAuthorized) { - return; + if (self.contactsViewHelper.contactsManager.isSystemContactsAuthorized) { + BOOL hasNoContacts = self.contactsViewHelper.signalAccounts.count < 1; + self.isNoContactsModeActive = (hasNoContacts && ![[Environment preferences] hasDeclinedNoContactsView]); + [self showContactsPermissionReminder:NO]; + } else { + // don't show "no signal contacts", show "no contact access" + self.isNoContactsModeActive = NO; + [self showContactsPermissionReminder:YES]; } - - BOOL hasNoContacts = self.contactsViewHelper.signalAccounts.count < 1; - self.isNoContactsModeActive = (hasNoContacts && ![[Environment preferences] hasDeclinedNoContactsView]); } - (void)setIsNoContactsModeActive:(BOOL)isNoContactsModeActive diff --git a/Signal/src/ViewControllers/SignalsViewController.m b/Signal/src/ViewControllers/SignalsViewController.m index 18c7f5a44..dc2b3d012 100644 --- a/Signal/src/ViewControllers/SignalsViewController.m +++ b/Signal/src/ViewControllers/SignalsViewController.m @@ -168,7 +168,8 @@ NSString *const SignalsViewControllerSegueShowIncomingCall = @"ShowIncomingCallS @"SETTINGS_BUTTON_ACCESSIBILITY", @"Accessibility hint for the settings button"); - self.missingContactsPermissionView.text = NSLocalizedString(@"INBOX_VIEW_MISSING_CONTACTS_PERMISSION", @"Multi line label explainging how to show names instead of phone numbers in your inbox"); + self.missingContactsPermissionView.text = NSLocalizedString(@"INBOX_VIEW_MISSING_CONTACTS_PERMISSION", + @"Multiline label explaining how to show names instead of phone numbers in your inbox"); self.missingContactsPermissionView.tapAction = ^{ [[UIApplication sharedApplication] openSystemSettings]; }; diff --git a/Signal/src/views/ReminderView.swift b/Signal/src/views/ReminderView.swift index f0875a2cf..71309dd71 100644 --- a/Signal/src/views/ReminderView.swift +++ b/Signal/src/views/ReminderView.swift @@ -41,8 +41,9 @@ class ReminderView: UIView { setupSubviews() } - convenience init(tapAction: @escaping () -> Void) { + convenience init(text: String, tapAction: @escaping () -> Void) { self.init(frame: .zero) + self.text = text self.tapAction = tapAction } diff --git a/Signal/translations/en.lproj/Localizable.strings b/Signal/translations/en.lproj/Localizable.strings index ab6db1a7e..f986e986a 100644 --- a/Signal/translations/en.lproj/Localizable.strings +++ b/Signal/translations/en.lproj/Localizable.strings @@ -217,6 +217,9 @@ /* Activity Sheet label */ "COMPARE_SAFETY_NUMBER_ACTION" = "Compare with Clipboard"; +/* Multiline label explaining why compose-screen contact picker is empty. */ +"COMPOSE_SCREEN_MISSING_CONTACTS_PERMISSION" = "To see which of your contacts are Signal users, enable contacts access in your system settings."; + /* No comment provided by engineer. */ "CONFIRM_ACCOUNT_DESTRUCTION_TEXT" = "This will reset the application by deleting your messages and unregister you with the server. The app will close after deletion of data."; @@ -562,7 +565,7 @@ /* Call setup status label */ "IN_CALL_TERMINATED" = "Call Ended."; -/* Multi line label explainging how to show names instead of phone numbers in your inbox */ +/* Multiline label explaining how to show names instead of phone numbers in your inbox */ "INBOX_VIEW_MISSING_CONTACTS_PERMISSION" = "To see the names of your contacts, update your sytem settings to allow contact access."; /* notification body */ From 03727a27f65f65ec2be6d601132a6c578a487913 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Fri, 5 May 2017 18:17:13 -0400 Subject: [PATCH 4/9] compose w/o contact access -> "..by phone number" The search field and invite buttons are not very useful without contacts access. // FREEBIE --- .../MessageComposeTableViewController.m | 44 ++++++++++++------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/Signal/src/ViewControllers/MessageComposeTableViewController.m b/Signal/src/ViewControllers/MessageComposeTableViewController.m index 181363447..360d84a2d 100644 --- a/Signal/src/ViewControllers/MessageComposeTableViewController.m +++ b/Signal/src/ViewControllers/MessageComposeTableViewController.m @@ -115,9 +115,18 @@ NS_ASSUME_NONNULL_BEGIN [self updateTableContents]; } -- (void)showContactsPermissionReminder:(BOOL)flag +- (void)showContactsPermissionReminder:(BOOL)isVisible { - _hideContactsPermissionReminderViewConstraint.active = !flag; + _hideContactsPermissionReminderViewConstraint.active = !isVisible; +} + +- (void)showSearchBar:(BOOL)isVisible +{ + if (isVisible) { + self.tableViewController.tableView.tableHeaderView = self.searchBar; + } else { + self.tableViewController.tableView.tableHeaderView = nil; + } } - (UIView *)createNoSignalContactsView @@ -259,20 +268,22 @@ NS_ASSUME_NONNULL_BEGIN [weakSelf.navigationController pushViewController:viewController animated:YES]; }]]; - // Invite Contacts - [section addItem:[OWSTableItem itemWithCustomCellBlock:^{ - UITableViewCell *cell = [UITableViewCell new]; - cell.textLabel.text = NSLocalizedString(@"INVITE_FRIENDS_CONTACT_TABLE_BUTTON", - @"Label for the cell that presents the 'invite contacts' workflow."); - cell.textLabel.font = [UIFont ows_regularFontWithSize:18.f]; - cell.textLabel.textColor = [UIColor blackColor]; - cell.accessoryType = UITableViewCellAccessoryDisclosureIndicator; - return cell; + if (self.contactsViewHelper.contactsManager.isSystemContactsAuthorized) { + // Invite Contacts + [section addItem:[OWSTableItem itemWithCustomCellBlock:^{ + UITableViewCell *cell = [UITableViewCell new]; + cell.textLabel.text = NSLocalizedString(@"INVITE_FRIENDS_CONTACT_TABLE_BUTTON", + @"Label for the cell that presents the 'invite contacts' workflow."); + cell.textLabel.font = [UIFont ows_regularFontWithSize:18.f]; + cell.textLabel.textColor = [UIColor blackColor]; + cell.accessoryType = UITableViewCellAccessoryDisclosureIndicator; + return cell; + } + customRowHeight:kActionCellHeight + actionBlock:^{ + [weakSelf presentInviteFlow]; + }]]; } - customRowHeight:kActionCellHeight - actionBlock:^{ - [weakSelf presentInviteFlow]; - }]]; // If the search string looks like a phone number, show either "new conversation..." cells and/or // "invite via SMS..." cells. @@ -445,10 +456,13 @@ NS_ASSUME_NONNULL_BEGIN BOOL hasNoContacts = self.contactsViewHelper.signalAccounts.count < 1; self.isNoContactsModeActive = (hasNoContacts && ![[Environment preferences] hasDeclinedNoContactsView]); [self showContactsPermissionReminder:NO]; + + [self showSearchBar:YES]; } else { // don't show "no signal contacts", show "no contact access" self.isNoContactsModeActive = NO; [self showContactsPermissionReminder:YES]; + [self showSearchBar:NO]; } } From 04878bf222ee4bd2d5342a307f88b1093922062b Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 8 May 2017 11:35:34 -0400 Subject: [PATCH 5/9] rename method to better reflect new role // FREEBIE --- .../ViewControllers/MessageComposeTableViewController.m | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Signal/src/ViewControllers/MessageComposeTableViewController.m b/Signal/src/ViewControllers/MessageComposeTableViewController.m index 360d84a2d..8381df6fd 100644 --- a/Signal/src/ViewControllers/MessageComposeTableViewController.m +++ b/Signal/src/ViewControllers/MessageComposeTableViewController.m @@ -227,7 +227,7 @@ NS_ASSUME_NONNULL_BEGIN [self.navigationController.navigationBar setTranslucent:NO]; - [self showNoContactsModeIfNecessary]; + [self showContactAppropriateViews]; } #pragma mark - Table Contents @@ -439,7 +439,7 @@ NS_ASSUME_NONNULL_BEGIN { [[Environment preferences] setHasDeclinedNoContactsView:YES]; - [self showNoContactsModeIfNecessary]; + [self showContactAppropriateViews]; } - (void)presentInviteFlow @@ -450,7 +450,7 @@ NS_ASSUME_NONNULL_BEGIN [self presentViewController:inviteFlow.actionSheetController animated:YES completion:nil]; } -- (void)showNoContactsModeIfNecessary +- (void)showContactAppropriateViews { if (self.contactsViewHelper.contactsManager.isSystemContactsAuthorized) { BOOL hasNoContacts = self.contactsViewHelper.signalAccounts.count < 1; @@ -621,7 +621,7 @@ NS_ASSUME_NONNULL_BEGIN { [self updateTableContents]; - [self showNoContactsModeIfNecessary]; + [self showContactAppropriateViews]; } - (BOOL)shouldHideLocalNumber From 64bcc9458921852bb37581f7182a1c33ab9b4de6 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 8 May 2017 14:46:35 -0400 Subject: [PATCH 6/9] Instead of alert we're providing in context reminders - no need for these TODOs // FREEBIE --- Signal/src/contact/SystemContactsFetcher.swift | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/Signal/src/contact/SystemContactsFetcher.swift b/Signal/src/contact/SystemContactsFetcher.swift index 79cc2ee8f..efd70531d 100644 --- a/Signal/src/contact/SystemContactsFetcher.swift +++ b/Signal/src/contact/SystemContactsFetcher.swift @@ -58,15 +58,12 @@ class SystemContactsFetcher: NSObject { } if !granted { - // TODO, make this a one time dismissable admonishment - // e.g. remember across launches that the user has dismissed. - self.displayMissingContactsPermissionAlert() + Logger.info("\(self.TAG) declined contact access.") } else { self.updateContacts() } }) case .authorized: - // TODO reset onetime admonishment reminder, so that we remind user again (once) if they've since toggled permissions. self.updateContacts() case .denied, .restricted: Logger.debug("\(TAG) contacts were \(self.authorizationStatus)") @@ -82,11 +79,6 @@ class SystemContactsFetcher: NSObject { updateContacts() } - private func displayMissingContactsPermissionAlert() { - let foo = UIApplication.shared.frontmostViewController - Logger.error("TODO") - } - private func updateContacts() { AssertIsOnMainThread() From dc75e592c1a3fb2d09739c95724150290578f8ed Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 8 May 2017 15:50:12 -0400 Subject: [PATCH 7/9] ensure contact callback on proper thread // FREEBIE --- Signal/src/contact/SystemContactsFetcher.swift | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Signal/src/contact/SystemContactsFetcher.swift b/Signal/src/contact/SystemContactsFetcher.swift index efd70531d..2b4794767 100644 --- a/Signal/src/contact/SystemContactsFetcher.swift +++ b/Signal/src/contact/SystemContactsFetcher.swift @@ -51,18 +51,21 @@ class SystemContactsFetcher: NSObject { switch authorizationStatus { case .notDetermined: - contactStore.requestAccess(for: .contacts, completionHandler: { (granted, error) in + contactStore.requestAccess(for: .contacts) { (granted, error) in if let error = error { Logger.error("\(self.TAG) error fetching contacts: \(error)") assertionFailure() } - if !granted { + guard granted else { Logger.info("\(self.TAG) declined contact access.") - } else { + return + } + + DispatchQueue.main.async { self.updateContacts() } - }) + } case .authorized: self.updateContacts() case .denied, .restricted: From fee47efbea0fba4b33a7ab55e6a28c0af1cfda0e Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 8 May 2017 16:28:01 -0400 Subject: [PATCH 8/9] Avoid repaint by requestng contacts before Compose This entailed passing callback params through the contact request. // FREEBIE --- .../ViewControllers/SignalsViewController.m | 15 ++++++-- Signal/src/contact/OWSContactsManager.h | 1 + Signal/src/contact/OWSContactsManager.m | 8 +++- .../src/contact/SystemContactsFetcher.swift | 37 +++++++++++++++---- 4 files changed, 50 insertions(+), 11 deletions(-) diff --git a/Signal/src/ViewControllers/SignalsViewController.m b/Signal/src/ViewControllers/SignalsViewController.m index dc2b3d012..abcf37aa5 100644 --- a/Signal/src/ViewControllers/SignalsViewController.m +++ b/Signal/src/ViewControllers/SignalsViewController.m @@ -279,9 +279,18 @@ NSString *const SignalsViewControllerSegueShowIncomingCall = @"ShowIncomingCallS - (IBAction)composeNew { MessageComposeTableViewController *viewController = [MessageComposeTableViewController new]; - UINavigationController *navigationController = - [[UINavigationController alloc] initWithRootViewController:viewController]; - [self presentTopLevelModalViewController:navigationController animateDismissal:YES animatePresentation:YES]; + + [self.contactsManager requestSystemContactsOnceWithCompletion:^(NSError *_Nullable error) { + DDLogError(@"%@ Error when requesting contacts: %@", self.tag, error); + // Even if there was an error fetching contacts we proceed to the next screen. + // As the compose view will present the proper thing depending on contact access. + // + // We just want to make sure contact access is *complete* before showing the compose + // screen to avoid flicker. + UINavigationController *navigationController = + [[UINavigationController alloc] initWithRootViewController:viewController]; + [self presentTopLevelModalViewController:navigationController animateDismissal:YES animatePresentation:YES]; + }]; } - (void)swappedSegmentedControl { diff --git a/Signal/src/contact/OWSContactsManager.h b/Signal/src/contact/OWSContactsManager.h index 1de096163..5b6ba75f2 100644 --- a/Signal/src/contact/OWSContactsManager.h +++ b/Signal/src/contact/OWSContactsManager.h @@ -39,6 +39,7 @@ extern NSString *const OWSContactsManagerSignalAccountsDidChangeNotification; // Request systems contacts and start syncing changes. The user will see an alert // if they haven't previously. - (void)requestSystemContactsOnce; +- (void)requestSystemContactsOnceWithCompletion:(void (^_Nullable)(NSError *_Nullable error))completion; // Ensure's the app has the latest contacts, but won't prompt the user for contact // access if they haven't granted it. diff --git a/Signal/src/contact/OWSContactsManager.m b/Signal/src/contact/OWSContactsManager.m index 8e5808bd7..fe3e68e20 100644 --- a/Signal/src/contact/OWSContactsManager.m +++ b/Signal/src/contact/OWSContactsManager.m @@ -54,9 +54,15 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification = // Request contacts access if you haven't asked recently. - (void)requestSystemContactsOnce { - [self.systemContactsFetcher requestOnce]; + [self requestSystemContactsOnceWithCompletion:nil]; } +- (void)requestSystemContactsOnceWithCompletion:(void (^_Nullable)(NSError *_Nullable error))completion +{ + [self.systemContactsFetcher requestOnceWithCompletion:completion]; +} + + - (void)fetchSystemContactsIfAlreadyAuthorized { [self.systemContactsFetcher fetchIfAlreadyAuthorized]; diff --git a/Signal/src/contact/SystemContactsFetcher.swift b/Signal/src/contact/SystemContactsFetcher.swift index 2b4794767..5775c2c23 100644 --- a/Signal/src/contact/SystemContactsFetcher.swift +++ b/Signal/src/contact/SystemContactsFetcher.swift @@ -39,11 +39,19 @@ class SystemContactsFetcher: NSObject { CNContactEmailAddressesKey as CNKeyDescriptor ] - public func requestOnce() { + /** + * Ensures we've requested access for system contacts. This can be used in multiple places, + * where we might need contact access, but will ensure we don't wastefully reload contacts + * if we have already fetched contacts. + * + * @param completion completion handler is called on main thread. + */ + public func requestOnce(completion: ((Error?) -> Void)?) { AssertIsOnMainThread() guard !systemContactsHaveBeenRequestedAtLeastOnce else { Logger.debug("\(TAG) already requested system contacts") + completion?(nil) return } systemContactsHaveBeenRequestedAtLeastOnce = true @@ -54,22 +62,32 @@ class SystemContactsFetcher: NSObject { contactStore.requestAccess(for: .contacts) { (granted, error) in if let error = error { Logger.error("\(self.TAG) error fetching contacts: \(error)") - assertionFailure() + DispatchQueue.main.async { + completion?(error) + } } guard granted else { Logger.info("\(self.TAG) declined contact access.") + // This case should have been caught be the error guard a few lines up. + assertionFailure() + DispatchQueue.main.async { + completion?(nil) + } return } DispatchQueue.main.async { - self.updateContacts() + self.updateContacts(completion: completion) } } case .authorized: - self.updateContacts() + self.updateContacts(completion: completion) case .denied, .restricted: Logger.debug("\(TAG) contacts were \(self.authorizationStatus)") + DispatchQueue.main.async { + completion?(nil) + } } } @@ -79,10 +97,10 @@ class SystemContactsFetcher: NSObject { return } - updateContacts() + updateContacts(completion: nil) } - private func updateContacts() { + private func updateContacts(completion: ((Error?) -> Void)?) { AssertIsOnMainThread() systemContactsHaveBeenRequestedAtLeastOnce = true @@ -100,11 +118,16 @@ class SystemContactsFetcher: NSObject { } catch let error as NSError { Logger.error("\(self.TAG) Failed to fetch contacts with error:\(error)") assertionFailure() + DispatchQueue.main.async { + completion?(error) + } + return } let contacts = systemContacts.map { Contact(systemContact: $0) } DispatchQueue.main.async { self.delegate?.systemContactsFetcher(self, updatedContacts: contacts) + completion?(nil) } } } @@ -118,7 +141,7 @@ class SystemContactsFetcher: NSObject { @objc private func contactStoreDidChange() { - updateContacts() + updateContacts(completion: nil) } } From 3040c4a341402dd73aa5fb37f84ba74a59ddc2e5 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 8 May 2017 17:35:18 -0400 Subject: [PATCH 9/9] include missing return // FREEBIE --- Signal/src/contact/SystemContactsFetcher.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Signal/src/contact/SystemContactsFetcher.swift b/Signal/src/contact/SystemContactsFetcher.swift index 5775c2c23..c3a67c90b 100644 --- a/Signal/src/contact/SystemContactsFetcher.swift +++ b/Signal/src/contact/SystemContactsFetcher.swift @@ -65,6 +65,7 @@ class SystemContactsFetcher: NSObject { DispatchQueue.main.async { completion?(error) } + return } guard granted else {