From 609746abec64643e118fded601550e5ea6bb1b53 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 9 May 2018 11:13:29 -0400 Subject: [PATCH 1/5] clarify naming // FREEBIE --- Signal/src/ViewControllers/ContactShareViewHelper.swift | 4 ++-- Signal/src/ViewControllers/ContactViewController.swift | 4 ++-- .../ConversationView/ConversationViewController.m | 4 ++-- Signal/src/ViewControllers/MessageDetailViewController.swift | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Signal/src/ViewControllers/ContactShareViewHelper.swift b/Signal/src/ViewControllers/ContactShareViewHelper.swift index a87c56189..64e53b1aa 100644 --- a/Signal/src/ViewControllers/ContactShareViewHelper.swift +++ b/Signal/src/ViewControllers/ContactShareViewHelper.swift @@ -70,7 +70,7 @@ public class ContactShareViewHelper: NSObject, CNContactViewControllerDelegate { } @objc - public func inviteContact(contactShare: ContactShareViewModel, fromViewController: UIViewController) { + public func showInviteContact(contactShare: ContactShareViewModel, fromViewController: UIViewController) { Logger.info("\(logTag) \(#function)") guard MFMessageComposeViewController.canSendText() else { @@ -89,7 +89,7 @@ public class ContactShareViewHelper: NSObject, CNContactViewControllerDelegate { inviteFlow.sendSMSTo(phoneNumbers: phoneNumbers) } - func addToContacts(contactShare: ContactShareViewModel, fromViewController: UIViewController) { + func showAddToContacts(contactShare: ContactShareViewModel, fromViewController: UIViewController) { Logger.info("\(logTag) \(#function)") let actionSheet = UIAlertController(title: nil, message: nil, preferredStyle: .actionSheet) diff --git a/Signal/src/ViewControllers/ContactViewController.swift b/Signal/src/ViewControllers/ContactViewController.swift index 9f8dbe9d3..df50db5ab 100644 --- a/Signal/src/ViewControllers/ContactViewController.swift +++ b/Signal/src/ViewControllers/ContactViewController.swift @@ -501,13 +501,13 @@ class ContactViewController: OWSViewController, ContactShareViewHelperDelegate { func didPressInvite() { Logger.info("\(logTag) \(#function)") - self.contactShareViewHelper.inviteContact(contactShare: self.contactShare, fromViewController: self) + self.contactShareViewHelper.showInviteContact(contactShare: self.contactShare, fromViewController: self) } func didPressAddToContacts() { Logger.info("\(logTag) \(#function)") - self.contactShareViewHelper.addToContacts(contactShare: self.contactShare, fromViewController: self) + self.contactShareViewHelper.showAddToContacts(contactShare: self.contactShare, fromViewController: self) } func didPressDismiss() { diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 2c6a3ed96..c3fad83c1 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -2124,7 +2124,7 @@ typedef enum : NSUInteger { OWSAssertIsOnMainThread(); OWSAssert(contactShare); - [self.contactShareViewHelper inviteContactWithContactShare:contactShare fromViewController:self]; + [self.contactShareViewHelper showInviteContactWithContactShare:contactShare fromViewController:self]; } - (void)didTapShowAddToContactUIForContactShare:(ContactShareViewModel *)contactShare @@ -2132,7 +2132,7 @@ typedef enum : NSUInteger { OWSAssertIsOnMainThread(); OWSAssert(contactShare); - [self.contactShareViewHelper addToContactsWithContactShare:contactShare fromViewController:self]; + [self.contactShareViewHelper showAddToContactsWithContactShare:contactShare fromViewController:self]; } - (void)didTapFailedIncomingAttachment:(ConversationViewItem *)viewItem diff --git a/Signal/src/ViewControllers/MessageDetailViewController.swift b/Signal/src/ViewControllers/MessageDetailViewController.swift index fddaf8fb6..710be7c70 100644 --- a/Signal/src/ViewControllers/MessageDetailViewController.swift +++ b/Signal/src/ViewControllers/MessageDetailViewController.swift @@ -630,11 +630,11 @@ class MessageDetailViewController: OWSViewController, MediaGalleryDataSourceDele } func didTapSendInvite(toContactShare contactShare: ContactShareViewModel) { - contactShareViewHelper.inviteContact(contactShare: contactShare, fromViewController: self) + contactShareViewHelper.showInviteContact(contactShare: contactShare, fromViewController: self) } func didTapShowAddToContactUI(forContactShare contactShare: ContactShareViewModel) { - contactShareViewHelper.addToContacts(contactShare: contactShare, fromViewController: self) + contactShareViewHelper.showAddToContacts(contactShare: contactShare, fromViewController: self) } var audioAttachmentPlayer: OWSAudioPlayer? From 0c469764f1c07ef3bce0f474d5426270003c2805 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 9 May 2018 11:54:10 -0400 Subject: [PATCH 2/5] re-use contact picker for "add to existing" Required refactor of contact picker to be presented non-modally. TODO: merge emails, address, display names // FREEBIE --- Signal.xcodeproj/project.pbxproj | 4 + ...ShareToExistingContactViewController.swift | 121 ++++++++++++++++++ .../ContactShareViewHelper.swift | 26 ++-- .../src/ViewControllers/ContactsPicker.swift | 65 ++++------ .../ConversationViewController.m | 8 +- Signal/src/ViewControllers/InviteFlow.swift | 11 +- Signal/src/views/ContactCell.swift | 8 +- .../ViewModels/ContactShareViewModel.swift | 59 +++++++++ SignalMessaging/Views/ContactsViewHelper.m | 6 +- .../contacts/SystemContactsFetcher.swift | 5 +- 10 files changed, 253 insertions(+), 60 deletions(-) create mode 100644 Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift diff --git a/Signal.xcodeproj/project.pbxproj b/Signal.xcodeproj/project.pbxproj index ff09cf586..12c4fdc79 100644 --- a/Signal.xcodeproj/project.pbxproj +++ b/Signal.xcodeproj/project.pbxproj @@ -294,6 +294,7 @@ 4523149E1F7E916B003A428C /* SlideOffAnimatedTransition.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4523149D1F7E916B003A428C /* SlideOffAnimatedTransition.swift */; }; 452314A01F7E9E18003A428C /* DirectionalPanGestureRecognizer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4523149F1F7E9E18003A428C /* DirectionalPanGestureRecognizer.swift */; }; 4523D016206EDC2B00A2AB51 /* LRUCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4523D015206EDC2B00A2AB51 /* LRUCache.swift */; }; + 452B999020A34B6B006F2F9E /* AddContactShareToExistingContactViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 452B998F20A34B6B006F2F9E /* AddContactShareToExistingContactViewController.swift */; }; 452C468F1E427E200087B011 /* OutboundCallInitiator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 452C468E1E427E200087B011 /* OutboundCallInitiator.swift */; }; 452C7CA72037628B003D51A5 /* Weak.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45F170D51E315310003FC1F2 /* Weak.swift */; }; 452D1AF12081059C00A67F7F /* StringAdditionsTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 452D1AF02081059C00A67F7F /* StringAdditionsTest.swift */; }; @@ -926,6 +927,7 @@ 4523149D1F7E916B003A428C /* SlideOffAnimatedTransition.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; name = SlideOffAnimatedTransition.swift; path = UserInterface/SlideOffAnimatedTransition.swift; sourceTree = ""; }; 4523149F1F7E9E18003A428C /* DirectionalPanGestureRecognizer.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DirectionalPanGestureRecognizer.swift; sourceTree = ""; }; 4523D015206EDC2B00A2AB51 /* LRUCache.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LRUCache.swift; sourceTree = ""; }; + 452B998F20A34B6B006F2F9E /* AddContactShareToExistingContactViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AddContactShareToExistingContactViewController.swift; sourceTree = ""; }; 452C468E1E427E200087B011 /* OutboundCallInitiator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OutboundCallInitiator.swift; sourceTree = ""; }; 452D1AF02081059C00A67F7F /* StringAdditionsTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StringAdditionsTest.swift; sourceTree = ""; }; 452D1AF220810B6F00A67F7F /* String+OWS.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "String+OWS.swift"; sourceTree = ""; }; @@ -1681,6 +1683,7 @@ 34B3F86E1E8DF1700035BE1A /* SignalsNavigationController.m */, 340FC897204DAC8D007AEB0F /* ThreadSettings */, 34D1F0BE1F8EC1760066283D /* Utils */, + 452B998F20A34B6B006F2F9E /* AddContactShareToExistingContactViewController.swift */, ); path = ViewControllers; sourceTree = ""; @@ -3238,6 +3241,7 @@ 340FC8BC204DAC8D007AEB0F /* FingerprintViewController.m in Sources */, 450DF2051E0D74AC003D14BE /* Platform.swift in Sources */, 340FC8B2204DAC8D007AEB0F /* AdvancedSettingsTableViewController.m in Sources */, + 452B999020A34B6B006F2F9E /* AddContactShareToExistingContactViewController.swift in Sources */, 346129991FD1E4DA00532771 /* SignalApp.m in Sources */, 34BECE301F7ABCF800D7438D /* GifPickerLayout.swift in Sources */, 343A65951FC47D5E000477A1 /* DebugUISyncMessages.m in Sources */, diff --git a/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift b/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift new file mode 100644 index 000000000..c53eba4e6 --- /dev/null +++ b/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift @@ -0,0 +1,121 @@ +// +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. +// + +import Foundation +import UIKit +import ContactsUI + +class AddContactShareToExistingContactViewController: ContactsPicker, ContactsPickerDelegate, CNContactViewControllerDelegate { + + // TODO actual protocol? + weak var addToExistingContactDelegate: UIViewController? + + let contactShare: ContactShareViewModel + + required init(contactShare: ContactShareViewModel) { + self.contactShare = contactShare + super.init(allowsMultipleSelection: false, subtitleCellType: .none) + + self.contactsPickerDelegate = self + } + + required public init?(coder aDecoder: NSCoder) { + fatalError("init(coder:) has not been implemented") + } + + @objc required public init(allowsMultipleSelection: Bool, subtitleCellType: SubtitleCellValue) { + fatalError("init(allowsMultipleSelection:subtitleCellType:) has not been implemented") + } + + // MARK: - ContactsPickerDelegate + + func contactsPicker(_: ContactsPicker, contactFetchDidFail error: NSError) { + owsFail("\(logTag) in \(#function) with error: \(error)") + + guard let navigationController = self.navigationController else { + owsFail("\(logTag) in \(#function) navigationController was unexpectedly nil") + return + } + + navigationController.popViewController(animated: true) + } + + func contactsPickerDidCancel(_: ContactsPicker) { + Logger.debug("\(self.logTag) in \(#function)") + guard let navigationController = self.navigationController else { + owsFail("\(logTag) in \(#function) navigationController was unexpectedly nil") + return + } + + navigationController.popViewController(animated: true) + } + + func contactsPicker(_: ContactsPicker, didSelectContact contact: Contact) { + Logger.debug("\(self.logTag) in \(#function)") + + guard let mergedContact: CNContact = self.contactShare.cnContact(mergedWithExistingContact: contact) else { + // TODO maybe this should not be optional and return a blank contact so we can still save the (not-actually merged) contact + owsFail("\(logTag) in \(#function) navigationController was unexpectedly nil") + return + } + + // Not actually a "new" contact, but this brings up the edit form rather than the "Read" form + // saving our users a tap in some cases when we already know they want to edit. + let contactViewController: CNContactViewController = CNContactViewController(forNewContact: mergedContact) + + // Default title is "New Contact". We could give a more descriptive title, but anything + // seems redundant - the context is sufficiently clear. + contactViewController.title = "" + contactViewController.allowsActions = false + contactViewController.allowsEditing = true + contactViewController.delegate = self + + guard let navigationController = self.navigationController else { + owsFail("\(logTag) in \(#function) navigationController was unexpectedly nil") + return + } + navigationController.pushViewController(contactViewController, animated: true) + } + + func contactsPicker(_: ContactsPicker, didSelectMultipleContacts contacts: [Contact]) { + Logger.debug("\(self.logTag) in \(#function)") + owsFail("\(logTag) only supports single contact select") + + guard let navigationController = self.navigationController else { + owsFail("\(logTag) in \(#function) navigationController was unexpectedly nil") + return + } + + navigationController.popViewController(animated: true) + } + + func contactsPicker(_: ContactsPicker, shouldSelectContact contact: Contact) -> Bool { + return true + } + + // MARK: - CNContactViewControllerDelegate + + public func contactViewController(_ viewController: CNContactViewController, didCompleteWith contact: CNContact?) { + Logger.debug("\(self.logTag) in \(#function)") + + guard let navigationController = self.navigationController else { + owsFail("\(logTag) in \(#function) navigationController was unexpectedly nil") + return + } + + // We want to pop *this* view and the still presented CNContactViewController in a single animation. + // Note this happens for *cancel* and for *done*. Unfortunately, I don't know of a way to detect the difference + // between the two, since both just call this method. + guard let myIndex = navigationController.viewControllers.index(of: self) else { + owsFail("\(logTag) in \(#function) myIndex was unexpectedly nil") + navigationController.popViewController(animated: true) + navigationController.popViewController(animated: true) + return + } + + let previousViewControllerIndex = navigationController.viewControllers.index(before: myIndex) + let previousViewController = navigationController.viewControllers[previousViewControllerIndex] + navigationController.popToViewController(previousViewController, animated: true) + } +} diff --git a/Signal/src/ViewControllers/ContactShareViewHelper.swift b/Signal/src/ViewControllers/ContactShareViewHelper.swift index 64e53b1aa..b995c7f75 100644 --- a/Signal/src/ViewControllers/ContactShareViewHelper.swift +++ b/Signal/src/ViewControllers/ContactShareViewHelper.swift @@ -177,6 +177,7 @@ public class ContactShareViewHelper: NSObject, CNContactViewControllerDelegate { UIUtil.applyDefaultSystemAppearence() } + // MJK TODO fromNavigationController? private func presentSelectAddToExistingContactView(contactShare: ContactShareViewModel, fromViewController: UIViewController) { guard contactsManager.supportsContactEditing else { owsFail("\(logTag) Contact editing not supported") @@ -187,23 +188,26 @@ public class ContactShareViewHelper: NSObject, CNContactViewControllerDelegate { ContactsViewHelper.presentMissingContactAccessAlertController(from: fromViewController) return } - - // TODO: Revisit this. - guard let firstPhoneNumber = contactShare.e164PhoneNumbers().first else { - owsFail("\(logTag) Missing phone number.") - return - } - - // TODO: We need to modify OWSAddToContactViewController to take a OWSContact - // and merge it with an existing CNContact. - let viewController = OWSAddToContactViewController() - viewController.configure(withRecipientId: firstPhoneNumber) +// +// // TODO: Revisit this. +// guard let firstPhoneNumber = contactShare.e164PhoneNumbers().first else { +// owsFail("\(logTag) Missing phone number.") +// return +// } +// +// // TODO: We need to modify OWSAddToContactViewController to take a OWSContact +// // and merge it with an existing CNContact. +// let viewController = OWSAddToContactViewController() +// viewController.configure(withRecipientId: firstPhoneNumber) guard let navigationController = fromViewController.navigationController else { owsFail("\(logTag) missing navigationController") return } + let viewController = AddContactShareToExistingContactViewController(contactShare: contactShare) +// viewController.addToExistingContactDelegate = fromViewController + navigationController.pushViewController(viewController, animated: true) } diff --git a/Signal/src/ViewControllers/ContactsPicker.swift b/Signal/src/ViewControllers/ContactsPicker.swift index 1cdfb030b..db22a57e0 100644 --- a/Signal/src/ViewControllers/ContactsPicker.swift +++ b/Signal/src/ViewControllers/ContactsPicker.swift @@ -20,10 +20,6 @@ public protocol ContactsPickerDelegate: class { func contactsPicker(_: ContactsPicker, shouldSelectContact contact: Contact) -> Bool } -public extension ContactsPickerDelegate { - func contactsPicker(_: ContactsPicker, shouldSelectContact contact: Contact) -> Bool { return true } -} - @objc public enum SubtitleCellValue: Int { case phoneNumber, email, none @@ -72,15 +68,22 @@ public class ContactsPicker: OWSViewController, UITableViewDelegate, UITableView // Configuration public weak var contactsPickerDelegate: ContactsPickerDelegate? - private let subtitleCellValue: SubtitleCellValue - private let multiSelectEnabled: Bool - private let allowedContactKeys: [CNKeyDescriptor] = [ - CNContactFormatter.descriptorForRequiredKeys(for: .fullName), - CNContactThumbnailImageDataKey as CNKeyDescriptor, - CNContactPhoneNumbersKey as CNKeyDescriptor, - CNContactEmailAddressesKey as CNKeyDescriptor, - CNContactPostalAddressesKey as CNKeyDescriptor - ] + private let subtitleCellType: SubtitleCellValue + private let allowsMultipleSelection: Bool + private let allowedContactKeys: [CNKeyDescriptor] = ContactsFrameworkContactStoreAdaptee.allowedContactKeys + + // MARK: - Initializers + + @objc + required public init(allowsMultipleSelection: Bool, subtitleCellType: SubtitleCellValue) { + self.allowsMultipleSelection = allowsMultipleSelection + self.subtitleCellType = subtitleCellType + super.init(nibName: "ContactsPicker", bundle: nil) + } + + required public init?(coder aDecoder: NSCoder) { + fatalError("init(coder:) has not been implemented") + } // MARK: - Lifecycle Methods @@ -95,7 +98,7 @@ public class ContactsPicker: OWSViewController, UITableViewDelegate, UITableView tableView.estimatedRowHeight = 60.0 tableView.rowHeight = UITableViewAutomaticDimension - tableView.allowsMultipleSelection = multiSelectEnabled + tableView.allowsMultipleSelection = allowsMultipleSelection tableView.separatorInset = UIEdgeInsets(top: 0, left: ContactCell.kSeparatorHInset, bottom: 0, right: 16) @@ -116,7 +119,7 @@ public class ContactsPicker: OWSViewController, UITableViewDelegate, UITableView let cancelButton = UIBarButtonItem(barButtonSystemItem: .cancel, target: self, action: #selector(onTouchCancelButton)) self.navigationItem.leftBarButtonItem = cancelButton - if multiSelectEnabled { + if allowsMultipleSelection { let doneButton = UIBarButtonItem(barButtonSystemItem: .done, target: self, action: #selector(onTouchDoneButton)) self.navigationItem.rightBarButtonItem = doneButton } @@ -126,20 +129,6 @@ public class ContactsPicker: OWSViewController, UITableViewDelegate, UITableView tableView.register(ContactCell.self, forCellReuseIdentifier: contactCellReuseIdentifier) } - // MARK: - Initializers - - @objc - required public init(delegate: ContactsPickerDelegate?, multiSelection: Bool, subtitleCellType: SubtitleCellValue) { - multiSelectEnabled = multiSelection - subtitleCellValue = subtitleCellType - super.init(nibName: nil, bundle: nil) - contactsPickerDelegate = delegate - } - - required public init?(coder aDecoder: NSCoder) { - fatalError("init(coder:) has not been implemented") - } - // MARK: - Contact Operations private func reloadContacts() { @@ -162,7 +151,6 @@ public class ContactsPicker: OWSViewController, UITableViewDelegate, UITableView let error = NSError(domain: "contactsPickerErrorDomain", code: 1, userInfo: [NSLocalizedDescriptionKey: "No Contacts Access"]) self.contactsPickerDelegate?.contactsPicker(self, contactFetchDidFail: error) errorHandler(error) - self.dismiss(animated: true, completion: nil) }) alert.addAction(cancelAction) @@ -231,13 +219,16 @@ public class ContactsPicker: OWSViewController, UITableViewDelegate, UITableView // MARK: - Table View Delegates open func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell { - let cell = tableView.dequeueReusableCell(withIdentifier: contactCellReuseIdentifier, for: indexPath) as! ContactCell + guard let cell = tableView.dequeueReusableCell(withIdentifier: contactCellReuseIdentifier, for: indexPath) as? ContactCell else { + owsFail("\(logTag) in \(#function) cell had unexpected type") + return UITableViewCell() + } let dataSource = filteredSections let cnContact = dataSource[indexPath.section][indexPath.row] let contact = Contact(systemContact: cnContact) - cell.configure(contact: contact, subtitleType: subtitleCellValue, contactsManager: self.contactsManager) + cell.configure(contact: contact, subtitleType: subtitleCellType, showsWhenSelected: self.allowsMultipleSelection, contactsManager: self.contactsManager) let isSelected = selectedContacts.contains(where: { $0.uniqueId == contact.uniqueId }) cell.isSelected = isSelected @@ -274,11 +265,9 @@ public class ContactsPicker: OWSViewController, UITableViewDelegate, UITableView selectedContacts.append(selectedContact) - if !multiSelectEnabled { - //Single selection code - self.dismiss(animated: true) { - self.contactsPickerDelegate?.contactsPicker(self, didSelectContact: selectedContact) - } + if !allowsMultipleSelection { + // Single selection code + self.contactsPickerDelegate?.contactsPicker(self, didSelectContact: selectedContact) } } @@ -313,12 +302,10 @@ public class ContactsPicker: OWSViewController, UITableViewDelegate, UITableView func onTouchCancelButton() { contactsPickerDelegate?.contactsPickerDidCancel(self) - dismiss(animated: true, completion: nil) } func onTouchDoneButton() { contactsPickerDelegate?.contactsPicker(self, didSelectMultipleContacts: selectedContacts) - dismiss(animated: true, completion: nil) } // MARK: - Search Actions diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index c3fad83c1..c976699fe 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -2571,7 +2571,8 @@ typedef enum : NSUInteger { - (void)chooseContactForSending { ContactsPicker *contactsPicker = - [[ContactsPicker alloc] initWithDelegate:self multiSelection:NO subtitleCellType:SubtitleCellValueNone]; + [[ContactsPicker alloc] initWithAllowsMultipleSelection:NO subtitleCellType:SubtitleCellValueNone]; + contactsPicker.contactsPickerDelegate = self; contactsPicker.title = NSLocalizedString(@"CONTACT_PICKER_TITLE", @"navbar title for contact picker when sharing a contact"); @@ -4978,11 +4979,13 @@ interactionControllerForAnimationController:(id *)contacts { OWSFail(@"%@ in %s with contacts: %@", self.logTag, __PRETTY_FUNCTION__, contacts); + [self dismissViewControllerAnimated:YES completion:nil]; } - (BOOL)contactsPicker:(ContactsPicker *)contactsPicker shouldSelectContact:(Contact *)contact diff --git a/Signal/src/ViewControllers/InviteFlow.swift b/Signal/src/ViewControllers/InviteFlow.swift index 075fa0d66..323167a7c 100644 --- a/Signal/src/ViewControllers/InviteFlow.swift +++ b/Signal/src/ViewControllers/InviteFlow.swift @@ -88,6 +88,8 @@ class InviteFlow: NSObject, MFMessageComposeViewControllerDelegate, MFMailCompos func contactsPicker(_: ContactsPicker, didSelectMultipleContacts contacts: [Contact]) { Logger.debug("\(TAG) didSelectContacts:\(contacts)") + self.presentingViewController.dismiss(animated: true) + guard let inviteChannel = channel else { Logger.error("\(TAG) unexpected nil channel after returning from contact picker.") return @@ -124,14 +126,17 @@ class InviteFlow: NSObject, MFMessageComposeViewControllerDelegate, MFMailCompos func contactsPicker(_: ContactsPicker, contactFetchDidFail error: NSError) { Logger.error("\(self.logTag) in \(#function) with error: \(error)") + self.presentingViewController.dismiss(animated: true) } func contactsPickerDidCancel(_: ContactsPicker) { Logger.debug("\(self.logTag) in \(#function)") + self.presentingViewController.dismiss(animated: true) } func contactsPicker(_: ContactsPicker, didSelectContact contact: Contact) { owsFail("\(logTag) in \(#function) InviteFlow only supports multi-select") + self.presentingViewController.dismiss(animated: true) } // MARK: SMS @@ -146,7 +151,8 @@ class InviteFlow: NSObject, MFMessageComposeViewControllerDelegate, MFMailCompos return UIAlertAction(title: messageTitle, style: .default) { _ in Logger.debug("\(self.TAG) Chose message.") self.channel = .message - let picker = ContactsPicker(delegate: self, multiSelection: true, subtitleCellType: .phoneNumber) + let picker = ContactsPicker(allowsMultipleSelection: true, subtitleCellType: .phoneNumber) + picker.contactsPickerDelegate = self picker.title = NSLocalizedString("INVITE_FRIENDS_PICKER_TITLE", comment: "Navbar title") let navigationController = UINavigationController(rootViewController: picker) self.presentingViewController.present(navigationController, animated: true) @@ -209,7 +215,8 @@ class InviteFlow: NSObject, MFMessageComposeViewControllerDelegate, MFMailCompos Logger.debug("\(self.TAG) Chose mail.") self.channel = .mail - let picker = ContactsPicker(delegate: self, multiSelection: true, subtitleCellType: .email) + let picker = ContactsPicker(allowsMultipleSelection: true, subtitleCellType: .email) + picker.contactsPickerDelegate = self picker.title = NSLocalizedString("INVITE_FRIENDS_PICKER_TITLE", comment: "Navbar title") let navigationController = UINavigationController(rootViewController: picker) self.presentingViewController.present(navigationController, animated: true) diff --git a/Signal/src/views/ContactCell.swift b/Signal/src/views/ContactCell.swift index aa520ef95..df015109e 100644 --- a/Signal/src/views/ContactCell.swift +++ b/Signal/src/views/ContactCell.swift @@ -19,6 +19,7 @@ class ContactCell: UITableViewCell { var subtitleLabel: UILabel var contact: Contact? + var showsWhenSelected: Bool = false override init(style: UITableViewCellStyle, reuseIdentifier: String?) { self.contactImageView = AvatarImageView() @@ -59,7 +60,9 @@ class ContactCell: UITableViewCell { override func setSelected(_ selected: Bool, animated: Bool) { super.setSelected(selected, animated: animated) - accessoryType = selected ? .checkmark : .none + if showsWhenSelected { + accessoryType = selected ? .checkmark : .none + } } func didChangePreferredContentSize() { @@ -67,8 +70,9 @@ class ContactCell: UITableViewCell { self.subtitleLabel.font = UIFont.ows_dynamicTypeSubheadline } - func configure(contact: Contact, subtitleType: SubtitleCellValue, contactsManager: OWSContactsManager) { + func configure(contact: Contact, subtitleType: SubtitleCellValue, showsWhenSelected: Bool, contactsManager: OWSContactsManager) { self.contact = contact + self.showsWhenSelected = showsWhenSelected titleLabel.attributedText = contact.cnContact?.formattedFullName(font: titleLabel.font) updateSubtitle(subtitleType: subtitleType, contact: contact) diff --git a/SignalMessaging/ViewModels/ContactShareViewModel.swift b/SignalMessaging/ViewModels/ContactShareViewModel.swift index da6b3f087..210c84117 100644 --- a/SignalMessaging/ViewModels/ContactShareViewModel.swift +++ b/SignalMessaging/ViewModels/ContactShareViewModel.swift @@ -109,6 +109,65 @@ public class ContactShareViewModel: NSObject { return dbRecord.isProfileAvatar } + public func cnContact(mergedWithExistingContact existingContact: Contact) -> CNContact? { + + guard let mergedCNContact: CNMutableContact = existingContact.cnContact?.mutableCopy() as? CNMutableContact else { + owsFail("\(logTag) in \(#function) mergedCNContact was unexpectedly nil") + return nil + } + + guard let newCNContact = OWSContacts.systemContact(for: self.dbRecord) else { + owsFail("\(logTag) in \(#function) newCNContact was unexpectedly nil") + return nil + } + + let normalize = { (input: String) -> String in + return (input as NSString).ows_stripped() + } + + // TODO display name - but only if existing is blank + + var existingPhoneNumberSet: Set = Set() + mergedCNContact.phoneNumbers.map { $0.value }.forEach { (existingPhoneNumber: CNPhoneNumber) in + // compare e164? + let normalizedValue = normalize(existingPhoneNumber.stringValue as String) + existingPhoneNumberSet.insert(normalizedValue) + } + + newCNContact.phoneNumbers.forEach { (newLabeledPhoneNumber: CNLabeledValue) in + let normalizedValue = normalize(newLabeledPhoneNumber.value.stringValue as String) + guard !existingPhoneNumberSet.contains(normalizedValue) else { + Logger.debug("\(self.logTag) in \(#function) ignoring matching phone number: \(normalizedValue)") + return + } + + Logger.debug("\(self.logTag) in \(#function) adding new phone number: \(normalizedValue)") + mergedCNContact.phoneNumbers.append(newLabeledPhoneNumber) + } + +// // TODO emails +// var existingEmailSet: Set = Set() +// mergedCNContact.emailAddresses.forEach { (existingEmail: CNLabeledValue) in +// existingEmailSet.insert(normalize(existingEmail.value as String)) +// } + + // TODO address + +// newCNContact.emailAddresses.forEach { newEmail in +// let match = mergedCNContact.emailAddresses.first { existingEmail in +// return normalize(existingEmail) == normalize(newEmail) +// } +// if match == nil { +// mergedCNContact.emailAddresses.add +// } +// } +// +// +// + + return mergedCNContact.copy() as? CNContact + } + public func copy(withNamePrefix namePrefix: String?, givenName: String?, middleName: String?, diff --git a/SignalMessaging/Views/ContactsViewHelper.m b/SignalMessaging/Views/ContactsViewHelper.m index 17c38ef4d..d1afa28db 100644 --- a/SignalMessaging/Views/ContactsViewHelper.m +++ b/SignalMessaging/Views/ContactsViewHelper.m @@ -308,7 +308,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)presentContactViewControllerForRecipientId:(NSString *)recipientId fromViewController:(UIViewController *)fromViewController editImmediately:(BOOL)shouldEditImmediately - addToExistingCnContact:(CNContact *_Nullable)addToExistingCnContact + addToExistingCnContact:(CNContact *_Nullable)existingContact { SignalAccount *signalAccount = [self signalAccountForRecipientId:recipientId]; @@ -325,8 +325,8 @@ NS_ASSUME_NONNULL_BEGIN CNContactViewController *_Nullable contactViewController; CNContact *_Nullable cnContact = nil; - if (addToExistingCnContact) { - CNMutableContact *updatedContact = [addToExistingCnContact mutableCopy]; + if (existingContact) { + CNMutableContact *updatedContact = [existingContact mutableCopy]; NSMutableArray *phoneNumbers = (updatedContact.phoneNumbers ? [updatedContact.phoneNumbers mutableCopy] : [NSMutableArray new]); // Only add recipientId as a phone number for the existing contact diff --git a/SignalMessaging/contacts/SystemContactsFetcher.swift b/SignalMessaging/contacts/SystemContactsFetcher.swift index 513380e33..33a3900f6 100644 --- a/SignalMessaging/contacts/SystemContactsFetcher.swift +++ b/SignalMessaging/contacts/SystemContactsFetcher.swift @@ -20,6 +20,7 @@ protocol ContactStoreAdaptee { func startObservingChanges(changeHandler: @escaping () -> Void) } +public class ContactsFrameworkContactStoreAdaptee: ContactStoreAdaptee { let TAG = "[ContactsFrameworkContactStoreAdaptee]" private let contactStore = CNContactStore() @@ -29,7 +30,7 @@ class ContactsFrameworkContactStoreAdaptee: ContactStoreAdaptee { let supportsContactEditing = true - private let allowedContactKeys: [CNKeyDescriptor] = [ + public static let allowedContactKeys: [CNKeyDescriptor] = [ CNContactFormatter.descriptorForRequiredKeys(for: .fullName), CNContactThumbnailImageDataKey as CNKeyDescriptor, // TODO full image instead of thumbnail? CNContactPhoneNumbersKey as CNKeyDescriptor, @@ -92,7 +93,7 @@ class ContactsFrameworkContactStoreAdaptee: ContactStoreAdaptee { func fetchContacts() -> Result<[Contact], Error> { var systemContacts = [CNContact]() do { - let contactFetchRequest = CNContactFetchRequest(keysToFetch: self.allowedContactKeys) + let contactFetchRequest = CNContactFetchRequest(keysToFetch: ContactsFrameworkContactStoreAdaptee.allowedContactKeys) contactFetchRequest.sortOrder = .userDefault try self.contactStore.enumerateContacts(with: contactFetchRequest) { (contact, _) -> Void in systemContacts.append(contact) From bf37f4116459e3e61d0ef4c0eeb99132c7588870 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 9 May 2018 20:24:42 -0400 Subject: [PATCH 3/5] Move CNContact logic into our system contact adapter // FREEBIE --- ...ShareToExistingContactViewController.swift | 22 ++++-- .../ViewModels/ContactShareViewModel.swift | 51 +------------ SignalServiceKit/src/Contacts/Contact.h | 8 +- SignalServiceKit/src/Contacts/Contact.m | 73 +++++++++++++++++-- .../src/Messages/Interactions/OWSContact.m | 6 +- 5 files changed, 91 insertions(+), 69 deletions(-) diff --git a/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift b/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift index c53eba4e6..327de07ab 100644 --- a/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift +++ b/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift @@ -8,8 +8,12 @@ import ContactsUI class AddContactShareToExistingContactViewController: ContactsPicker, ContactsPickerDelegate, CNContactViewControllerDelegate { - // TODO actual protocol? - weak var addToExistingContactDelegate: UIViewController? + // TODO - there are some hard coded assumptions in this VC that assume we are *pushed* onto a + // navigation controller. That seems fine for now, but if we need to be presented as a modal, + // or need to notify our presenter about our dismisall or other contact actions, a delegate + // would be helpful. It seems like this would require some broad changes to the ContactShareViewHelper, + // so I've left it as is for now, since it happens to work. + // weak var addToExistingContactDelegate: AddContactShareToExistingContactViewControllerDelegate? let contactShare: ContactShareViewModel @@ -55,8 +59,7 @@ class AddContactShareToExistingContactViewController: ContactsPicker, ContactsPi Logger.debug("\(self.logTag) in \(#function)") guard let mergedContact: CNContact = self.contactShare.cnContact(mergedWithExistingContact: contact) else { - // TODO maybe this should not be optional and return a blank contact so we can still save the (not-actually merged) contact - owsFail("\(logTag) in \(#function) navigationController was unexpectedly nil") + owsFail("\(logTag) in \(#function) mergedContact was unexpectedly nil") return } @@ -104,7 +107,16 @@ class AddContactShareToExistingContactViewController: ContactsPicker, ContactsPi return } - // We want to pop *this* view and the still presented CNContactViewController in a single animation. + // TODO this is weird - ideally we'd do something like + // self.delegate?.didFinishAddingContact + // and the delegate, which knows about our presentation context could do the right thing. + // + // As it is, we happen to always be *pushing* this view controller onto a navcontroller, so the + // following works in all current cases. + // + // If we ever wanted to do something different, like present this in a modal, we'd have to rethink. + + // We want to pop *this* view *and* the still presented CNContactViewController in a single animation. // Note this happens for *cancel* and for *done*. Unfortunately, I don't know of a way to detect the difference // between the two, since both just call this method. guard let myIndex = navigationController.viewControllers.index(of: self) else { diff --git a/SignalMessaging/ViewModels/ContactShareViewModel.swift b/SignalMessaging/ViewModels/ContactShareViewModel.swift index 210c84117..8e2a744cd 100644 --- a/SignalMessaging/ViewModels/ContactShareViewModel.swift +++ b/SignalMessaging/ViewModels/ContactShareViewModel.swift @@ -111,61 +111,12 @@ public class ContactShareViewModel: NSObject { public func cnContact(mergedWithExistingContact existingContact: Contact) -> CNContact? { - guard let mergedCNContact: CNMutableContact = existingContact.cnContact?.mutableCopy() as? CNMutableContact else { - owsFail("\(logTag) in \(#function) mergedCNContact was unexpectedly nil") - return nil - } - guard let newCNContact = OWSContacts.systemContact(for: self.dbRecord) else { owsFail("\(logTag) in \(#function) newCNContact was unexpectedly nil") return nil } - let normalize = { (input: String) -> String in - return (input as NSString).ows_stripped() - } - - // TODO display name - but only if existing is blank - - var existingPhoneNumberSet: Set = Set() - mergedCNContact.phoneNumbers.map { $0.value }.forEach { (existingPhoneNumber: CNPhoneNumber) in - // compare e164? - let normalizedValue = normalize(existingPhoneNumber.stringValue as String) - existingPhoneNumberSet.insert(normalizedValue) - } - - newCNContact.phoneNumbers.forEach { (newLabeledPhoneNumber: CNLabeledValue) in - let normalizedValue = normalize(newLabeledPhoneNumber.value.stringValue as String) - guard !existingPhoneNumberSet.contains(normalizedValue) else { - Logger.debug("\(self.logTag) in \(#function) ignoring matching phone number: \(normalizedValue)") - return - } - - Logger.debug("\(self.logTag) in \(#function) adding new phone number: \(normalizedValue)") - mergedCNContact.phoneNumbers.append(newLabeledPhoneNumber) - } - -// // TODO emails -// var existingEmailSet: Set = Set() -// mergedCNContact.emailAddresses.forEach { (existingEmail: CNLabeledValue) in -// existingEmailSet.insert(normalize(existingEmail.value as String)) -// } - - // TODO address - -// newCNContact.emailAddresses.forEach { newEmail in -// let match = mergedCNContact.emailAddresses.first { existingEmail in -// return normalize(existingEmail) == normalize(newEmail) -// } -// if match == nil { -// mergedCNContact.emailAddresses.add -// } -// } -// -// -// - - return mergedCNContact.copy() as? CNContact + return existingContact.buildCNContact(mergedWithNewContact: newCNContact) } public func copy(withNamePrefix namePrefix: String?, diff --git a/SignalServiceKit/src/Contacts/Contact.h b/SignalServiceKit/src/Contacts/Contact.h index f0205c48d..20ce68c66 100644 --- a/SignalServiceKit/src/Contacts/Contact.h +++ b/SignalServiceKit/src/Contacts/Contact.h @@ -7,10 +7,7 @@ NS_ASSUME_NONNULL_BEGIN /** - * - * Contact represents relevant information related to a contact from the user's - * contact list. - * + * An adapter for the system contacts */ @class CNContact; @@ -49,6 +46,9 @@ NS_ASSUME_NONNULL_BEGIN #endif // TARGET_OS_IOS + (NSComparator)comparatorSortingNamesByFirstThenLast:(BOOL)firstNameOrdering; ++ (NSString *)formattedFullNameWithCNContact:(CNContact *)cnContact NS_SWIFT_NAME(formattedFullName(cnContact:)); + +- (CNContact *)buildCNContactMergedWithNewContact:(CNContact *)newCNContact NS_SWIFT_NAME(buildCNContact(mergedWithNewContact:)); @end diff --git a/SignalServiceKit/src/Contacts/Contact.m b/SignalServiceKit/src/Contacts/Contact.m index 74fd195dc..01055d81b 100644 --- a/SignalServiceKit/src/Contacts/Contact.m +++ b/SignalServiceKit/src/Contacts/Contact.m @@ -4,6 +4,7 @@ #import "Contact.h" #import "Cryptography.h" +#import "NSString+SSK.h" #import "OWSPrimaryStorage.h" #import "PhoneNumber.h" #import "SignalRecipient.h" @@ -38,9 +39,9 @@ NS_ASSUME_NONNULL_BEGIN } _cnContact = contact; - _firstName = [self trimName:contact.givenName]; - _lastName = [self trimName:contact.familyName]; - _fullName = [CNContactFormatter stringFromContact:contact style:CNContactFormatterStyleFullName]; + _firstName = contact.givenName.ows_stripped; + _lastName = contact.familyName.ows_stripped; + _fullName = [Contact formattedFullNameWithCNContact:contact]; _uniqueId = contact.identifier; NSMutableArray *phoneNumbers = [NSMutableArray new]; @@ -125,11 +126,6 @@ NS_ASSUME_NONNULL_BEGIN return _image; } -- (NSString *)trimName:(NSString *)name -{ - return [name stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceAndNewlineCharacterSet]]; -} - + (MTLPropertyStorage)storageBehaviorForPropertyWithKey:(NSString *)propertyKey { if ([propertyKey isEqualToString:@"cnContact"] || [propertyKey isEqualToString:@"image"]) { @@ -250,6 +246,11 @@ NS_ASSUME_NONNULL_BEGIN }; } ++ (NSString *)formattedFullNameWithCNContact:(CNContact *)cnContact +{ + return [CNContactFormatter stringFromContact:cnContact style:CNContactFormatterStyleFullName].ows_stripped; +} + - (NSString *)nameForPhoneNumber:(NSString *)recipientId { OWSAssert(recipientId.length > 0); @@ -290,6 +291,62 @@ NS_ASSUME_NONNULL_BEGIN return hash; } +- (CNContact *)buildCNContactMergedWithNewContact:(CNContact *)newCNContact +{ + CNMutableContact *_Nullable mergedCNContact = [self.cnContact mutableCopy]; + if (!mergedCNContact) { + OWSFail(@"%@ in %s mergedCNContact was unexpectedly nil", self.logTag, __PRETTY_FUNCTION__); + return [CNContact new]; + } + + // Name + NSString *formattedFullName = [self.class formattedFullNameWithCNContact:mergedCNContact]; + + // merged all or nothing - do not try to piece-meal merge. + if (formattedFullName.length == 0) { + mergedCNContact.namePrefix = newCNContact.namePrefix.ows_stripped; + mergedCNContact.givenName = newCNContact.givenName.ows_stripped; + mergedCNContact.middleName = newCNContact.middleName.ows_stripped; + mergedCNContact.familyName = newCNContact.familyName.ows_stripped; + mergedCNContact.nameSuffix = newCNContact.nameSuffix.ows_stripped; + } + + // Phone Numbers + NSSet *existingPhoneNumberSet = [NSSet setWithArray:self.parsedPhoneNumbers]; + + NSMutableArray *> *mergedPhoneNumbers = [mergedCNContact.phoneNumbers mutableCopy]; + for (CNLabeledValue *labeledPhoneNumber in newCNContact.phoneNumbers) { + PhoneNumber *_Nullable parsedPhoneNumber = [PhoneNumber tryParsePhoneNumberFromUserSpecifiedText:labeledPhoneNumber.value.stringValue]; + if (parsedPhoneNumber && ![existingPhoneNumberSet containsObject:parsedPhoneNumber]) { + [mergedPhoneNumbers addObject:labeledPhoneNumber]; + } + } + mergedCNContact.phoneNumbers = mergedPhoneNumbers; + + // Emails + NSSet *existingEmailSet = [NSSet setWithArray:self.emails]; + NSMutableArray *> *mergedEmailAddresses = [mergedCNContact.emailAddresses mutableCopy]; + for (CNLabeledValue *labeledEmail in newCNContact.emailAddresses) { + NSString *normalizedValue = labeledEmail.value.ows_stripped; + if (![existingEmailSet containsObject:normalizedValue]) { + [mergedEmailAddresses addObject:labeledEmail]; + } + } + mergedCNContact.emailAddresses = mergedEmailAddresses; + + // Address + // merged all or nothing - do not try to piece-meal merge. + BOOL hasExistingAddress = NO; + for (CNLabeledValue *labeledPostalAddress in mergedCNContact.postalAddresses) { + hasExistingAddress = YES; + } + if (!hasExistingAddress) { + mergedCNContact.postalAddresses = newCNContact.postalAddresses; + } + + return [mergedCNContact copy]; +} + @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Messages/Interactions/OWSContact.m b/SignalServiceKit/src/Messages/Interactions/OWSContact.m index 6131fda32..bebb3aef2 100644 --- a/SignalServiceKit/src/Messages/Interactions/OWSContact.m +++ b/SignalServiceKit/src/Messages/Interactions/OWSContact.m @@ -3,6 +3,7 @@ // #import "OWSContact.h" +#import "Contact.h" #import "MimeTypeUtil.h" #import "NSString+SSK.h" #import "OWSContact+Private.h" @@ -371,8 +372,9 @@ NSString *NSStringForContactAddressType(OWSContactAddressType value) - (void)ensureDisplayName { if (_displayName.length < 1) { - CNContact *_Nullable systemContact = [OWSContacts systemContactForContact:self]; - _displayName = [CNContactFormatter stringFromContact:systemContact style:CNContactFormatterStyleFullName]; + + CNContact *_Nullable cnContact = [OWSContacts systemContactForContact:self]; + _displayName = [Contact formattedFullNameWithCNContact:cnContact]; } if (_displayName.length < 1) { // Fall back to using the organization name. From 95b93115f9c51f326afde66ff0c378fa90103141 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 9 May 2018 20:30:20 -0400 Subject: [PATCH 4/5] Code formatting // FREEBIE --- .../ViewControllers/ContactShareViewHelper.swift | 13 ------------- .../src/Messages/Interactions/OWSContact.m | 1 - 2 files changed, 14 deletions(-) diff --git a/Signal/src/ViewControllers/ContactShareViewHelper.swift b/Signal/src/ViewControllers/ContactShareViewHelper.swift index b995c7f75..436e9b63c 100644 --- a/Signal/src/ViewControllers/ContactShareViewHelper.swift +++ b/Signal/src/ViewControllers/ContactShareViewHelper.swift @@ -177,7 +177,6 @@ public class ContactShareViewHelper: NSObject, CNContactViewControllerDelegate { UIUtil.applyDefaultSystemAppearence() } - // MJK TODO fromNavigationController? private func presentSelectAddToExistingContactView(contactShare: ContactShareViewModel, fromViewController: UIViewController) { guard contactsManager.supportsContactEditing else { owsFail("\(logTag) Contact editing not supported") @@ -188,17 +187,6 @@ public class ContactShareViewHelper: NSObject, CNContactViewControllerDelegate { ContactsViewHelper.presentMissingContactAccessAlertController(from: fromViewController) return } -// -// // TODO: Revisit this. -// guard let firstPhoneNumber = contactShare.e164PhoneNumbers().first else { -// owsFail("\(logTag) Missing phone number.") -// return -// } -// -// // TODO: We need to modify OWSAddToContactViewController to take a OWSContact -// // and merge it with an existing CNContact. -// let viewController = OWSAddToContactViewController() -// viewController.configure(withRecipientId: firstPhoneNumber) guard let navigationController = fromViewController.navigationController else { owsFail("\(logTag) missing navigationController") @@ -206,7 +194,6 @@ public class ContactShareViewHelper: NSObject, CNContactViewControllerDelegate { } let viewController = AddContactShareToExistingContactViewController(contactShare: contactShare) -// viewController.addToExistingContactDelegate = fromViewController navigationController.pushViewController(viewController, animated: true) } diff --git a/SignalServiceKit/src/Messages/Interactions/OWSContact.m b/SignalServiceKit/src/Messages/Interactions/OWSContact.m index bebb3aef2..935868841 100644 --- a/SignalServiceKit/src/Messages/Interactions/OWSContact.m +++ b/SignalServiceKit/src/Messages/Interactions/OWSContact.m @@ -372,7 +372,6 @@ NSString *NSStringForContactAddressType(OWSContactAddressType value) - (void)ensureDisplayName { if (_displayName.length < 1) { - CNContact *_Nullable cnContact = [OWSContacts systemContactForContact:self]; _displayName = [Contact formattedFullNameWithCNContact:cnContact]; } From c15fea4ecab3a39ceea9a7c74e0075e6017ee3b0 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 9 May 2018 21:10:23 -0400 Subject: [PATCH 5/5] merge avatar with existing // FREEBIE --- .../ContactShareViewHelper.swift | 2 +- .../ConversationViewController.m | 10 ++++---- .../ViewModels/ContactShareViewModel.swift | 24 ++++++++++++------- .../SharingThreadPickerViewController.m | 4 ++-- SignalMessaging/contacts/OWSContactsManager.h | 1 + SignalMessaging/contacts/OWSContactsManager.m | 5 ++++ SignalMessaging/profiles/OWSProfileManager.h | 1 + SignalMessaging/profiles/OWSProfileManager.m | 14 +++++++++++ SignalServiceKit/src/Contacts/Contact.h | 1 + SignalServiceKit/src/Contacts/Contact.m | 12 +++++----- .../Messages/Attachments/TSAttachmentStream.h | 1 + .../Messages/Attachments/TSAttachmentStream.m | 24 +++++++++++++++++++ .../src/Messages/Interactions/OWSContact.h | 2 +- .../src/Messages/Interactions/OWSContact.m | 13 ++++------ 14 files changed, 83 insertions(+), 31 deletions(-) diff --git a/Signal/src/ViewControllers/ContactShareViewHelper.swift b/Signal/src/ViewControllers/ContactShareViewHelper.swift index 436e9b63c..89e6c4d74 100644 --- a/Signal/src/ViewControllers/ContactShareViewHelper.swift +++ b/Signal/src/ViewControllers/ContactShareViewHelper.swift @@ -144,7 +144,7 @@ public class ContactShareViewHelper: NSObject, CNContactViewControllerDelegate { return } - guard let systemContact = OWSContacts.systemContact(for: contactShare.dbRecord) else { + guard let systemContact = OWSContacts.systemContact(for: contactShare.dbRecord, imageData: contactShare.avatarImageData) else { owsFail("\(logTag) Could not derive system contact.") return } diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index c976699fe..8f05412e5 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -5004,20 +5004,20 @@ interactionControllerForAnimationController:(id CNContact? { - guard let newCNContact = OWSContacts.systemContact(for: self.dbRecord) else { + guard let newCNContact = OWSContacts.systemContact(for: self.dbRecord, imageData: self.avatarImageData) else { owsFail("\(logTag) in \(#function) newCNContact was unexpectedly nil") return nil } @@ -128,7 +136,7 @@ public class ContactShareViewModel: NSObject { // TODO move the `copy` logic into the view model? let newDbRecord = dbRecord.copy(withNamePrefix: namePrefix, givenName: givenName, middleName: middleName, familyName: familyName, nameSuffix: nameSuffix) - return ContactShareViewModel(contactShareRecord: newDbRecord, avatarImage: self.avatarImage) + return ContactShareViewModel(contactShareRecord: newDbRecord, avatarImageData: self.avatarImageData) } public func newContact(withNamePrefix namePrefix: String?, @@ -144,7 +152,7 @@ public class ContactShareViewModel: NSObject { familyName: familyName, nameSuffix: nameSuffix) - return ContactShareViewModel(contactShareRecord: newDbRecord, avatarImage: self.avatarImage) + return ContactShareViewModel(contactShareRecord: newDbRecord, avatarImageData: self.avatarImageData) } } diff --git a/SignalMessaging/attachments/SharingThreadPickerViewController.m b/SignalMessaging/attachments/SharingThreadPickerViewController.m index 2ae811df9..15e47f0a6 100644 --- a/SignalMessaging/attachments/SharingThreadPickerViewController.m +++ b/SignalMessaging/attachments/SharingThreadPickerViewController.m @@ -159,11 +159,11 @@ typedef void (^SendMessageBlock)(SendCompletionBlock completion); // TODO: Populate avatar image. BOOL isProfileAvatar = NO; - UIImage *_Nullable avatarImage = nil; + NSData *_Nullable avatarImageData = nil; contact.isProfileAvatar = isProfileAvatar; ContactShareViewModel *contactShare = - [[ContactShareViewModel alloc] initWithContactShareRecord:contact avatarImage:avatarImage]; + [[ContactShareViewModel alloc] initWithContactShareRecord:contact avatarImageData:avatarImageData]; ApproveContactShareViewController *approvalVC = [[ApproveContactShareViewController alloc] initWithContactShare:contactShare diff --git a/SignalMessaging/contacts/OWSContactsManager.h b/SignalMessaging/contacts/OWSContactsManager.h index 92ce39844..f49dc29cb 100644 --- a/SignalMessaging/contacts/OWSContactsManager.h +++ b/SignalMessaging/contacts/OWSContactsManager.h @@ -81,6 +81,7 @@ extern NSString *const OWSContactsManagerSignalAccountsDidChangeNotification; - (nullable UIImage *)systemContactImageForPhoneIdentifier:(nullable NSString *)identifier; - (nullable UIImage *)profileImageForPhoneIdentifier:(nullable NSString *)identifier; +- (nullable NSData *)profileImageDataForPhoneIdentifier:(nullable NSString *)identifier; - (nullable UIImage *)imageForPhoneIdentifier:(nullable NSString *)identifier; - (NSAttributedString *)formattedDisplayNameForSignalAccount:(SignalAccount *)signalAccount font:(UIFont *_Nonnull)font; diff --git a/SignalMessaging/contacts/OWSContactsManager.m b/SignalMessaging/contacts/OWSContactsManager.m index bde6d537f..9e0f07a58 100644 --- a/SignalMessaging/contacts/OWSContactsManager.m +++ b/SignalMessaging/contacts/OWSContactsManager.m @@ -739,6 +739,11 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification return [self.profileManager profileAvatarForRecipientId:identifier]; } +- (nullable NSData *)profileImageDataForPhoneIdentifier:(nullable NSString *)identifier +{ + return [self.profileManager profileAvatarDataForRecipientId:identifier]; +} + - (UIImage *_Nullable)imageForPhoneIdentifier:(NSString *_Nullable)identifier { // Prefer the contact image from the local address book if available diff --git a/SignalMessaging/profiles/OWSProfileManager.h b/SignalMessaging/profiles/OWSProfileManager.h index 983363b59..d1b228d98 100644 --- a/SignalMessaging/profiles/OWSProfileManager.h +++ b/SignalMessaging/profiles/OWSProfileManager.h @@ -75,6 +75,7 @@ extern const NSUInteger kOWSProfileManager_MaxAvatarDiameter; - (nullable NSString *)profileNameForRecipientId:(NSString *)recipientId; - (nullable UIImage *)profileAvatarForRecipientId:(NSString *)recipientId; +- (nullable NSData *)profileAvatarDataForRecipientId:(NSString *)recipientId; - (void)updateProfileForRecipientId:(NSString *)recipientId profileNameEncrypted:(nullable NSData *)profileNameEncrypted diff --git a/SignalMessaging/profiles/OWSProfileManager.m b/SignalMessaging/profiles/OWSProfileManager.m index 796d9b3ed..3f3a2822e 100644 --- a/SignalMessaging/profiles/OWSProfileManager.m +++ b/SignalMessaging/profiles/OWSProfileManager.m @@ -783,6 +783,20 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; return nil; } +- (nullable NSData *)profileAvatarDataForRecipientId:(NSString *)recipientId +{ + OWSAssert(recipientId.length > 0); + + OWSUserProfile *userProfile = + [OWSUserProfile getOrBuildUserProfileForRecipientId:recipientId dbConnection:self.dbConnection]; + + if (userProfile.avatarFileName.length > 0) { + return [self loadProfileDataWithFilename:userProfile.avatarFileName]; + } + + return nil; +} + - (void)downloadAvatarForUserProfile:(OWSUserProfile *)userProfile { OWSAssert(userProfile); diff --git a/SignalServiceKit/src/Contacts/Contact.h b/SignalServiceKit/src/Contacts/Contact.h index 20ce68c66..aec8c014b 100644 --- a/SignalServiceKit/src/Contacts/Contact.h +++ b/SignalServiceKit/src/Contacts/Contact.h @@ -30,6 +30,7 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, readonly) BOOL isSignalContact; #if TARGET_OS_IOS @property (nullable, readonly, nonatomic) UIImage *image; +@property (nullable, readonly, nonatomic) NSData *imageData; @property (nullable, nonatomic, readonly) CNContact *cnContact; #endif // TARGET_OS_IOS diff --git a/SignalServiceKit/src/Contacts/Contact.m b/SignalServiceKit/src/Contacts/Contact.m index 01055d81b..84d82b9a8 100644 --- a/SignalServiceKit/src/Contacts/Contact.m +++ b/SignalServiceKit/src/Contacts/Contact.m @@ -17,7 +17,6 @@ NS_ASSUME_NONNULL_BEGIN @interface Contact () @property (readonly, nonatomic) NSMutableDictionary *phoneNumberNameMap; -@property (readonly, nonatomic) NSData *imageData; @end @@ -336,14 +335,15 @@ NS_ASSUME_NONNULL_BEGIN // Address // merged all or nothing - do not try to piece-meal merge. - BOOL hasExistingAddress = NO; - for (CNLabeledValue *labeledPostalAddress in mergedCNContact.postalAddresses) { - hasExistingAddress = YES; - } - if (!hasExistingAddress) { + if (mergedCNContact.postalAddresses.count == 0) { mergedCNContact.postalAddresses = newCNContact.postalAddresses; } + // Avatar + if (!mergedCNContact.imageData) { + mergedCNContact.imageData = newCNContact.imageData; + } + return [mergedCNContact copy]; } diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h index 8657739a8..a14f48801 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h @@ -40,6 +40,7 @@ NS_ASSUME_NONNULL_BEGIN - (nullable UIImage *)image; - (nullable UIImage *)thumbnailImage; - (nullable NSData *)thumbnailData; +- (nullable NSData *)validStillImageData; #endif - (BOOL)isAnimated; diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index c5b523a38..a7d19a17c 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -331,6 +331,30 @@ NS_ASSUME_NONNULL_BEGIN } } +- (nullable NSData *)validStillImageData +{ + if ([self isVideo]) { + OWSFail(@"%@ in %s isVideo was unexpectedly true", self.logTag, __PRETTY_FUNCTION__); + return nil; + } + if ([self isAnimated]) { + OWSFail(@"%@ in %s isAnimated was unexpectedly true", self.logTag, __PRETTY_FUNCTION__); + return nil; + } + + NSURL *_Nullable mediaUrl = [self mediaURL]; + if (!mediaUrl) { + return nil; + } + + NSData *data = [NSData dataWithContentsOfURL:mediaUrl]; + if (![data ows_isValidImage]) { + return nil; + } + + return data; +} + + (BOOL)hasThumbnailForMimeType:(NSString *)contentType { return ([MIMETypeUtil isVideo:contentType] || [MIMETypeUtil isImage:contentType] || diff --git a/SignalServiceKit/src/Messages/Interactions/OWSContact.h b/SignalServiceKit/src/Messages/Interactions/OWSContact.h index 91058668a..8afad46cd 100644 --- a/SignalServiceKit/src/Messages/Interactions/OWSContact.h +++ b/SignalServiceKit/src/Messages/Interactions/OWSContact.h @@ -166,7 +166,7 @@ NSString *NSStringForContactAddressType(OWSContactAddressType value); #pragma mark - System Contact Conversion + (nullable OWSContact *)contactForSystemContact:(CNContact *)systemContact; -+ (nullable CNContact *)systemContactForContact:(OWSContact *)contact; ++ (nullable CNContact *)systemContactForContact:(OWSContact *)contact imageData:(nullable NSData *)imageData; #pragma mark - diff --git a/SignalServiceKit/src/Messages/Interactions/OWSContact.m b/SignalServiceKit/src/Messages/Interactions/OWSContact.m index 935868841..975db2145 100644 --- a/SignalServiceKit/src/Messages/Interactions/OWSContact.m +++ b/SignalServiceKit/src/Messages/Interactions/OWSContact.m @@ -372,7 +372,7 @@ NSString *NSStringForContactAddressType(OWSContactAddressType value) - (void)ensureDisplayName { if (_displayName.length < 1) { - CNContact *_Nullable cnContact = [OWSContacts systemContactForContact:self]; + CNContact *_Nullable cnContact = [OWSContacts systemContactForContact:self imageData:nil]; _displayName = [Contact formattedFullNameWithCNContact:cnContact]; } if (_displayName.length < 1) { @@ -693,7 +693,7 @@ NSString *NSStringForContactAddressType(OWSContactAddressType value) return contact; } -+ (nullable CNContact *)systemContactForContact:(OWSContact *)contact ++ (nullable CNContact *)systemContactForContact:(OWSContact *)contact imageData:(nullable NSData *)imageData { if (!contact) { OWSProdLogAndFail(@"%@ Missing contact.", self.logTag); @@ -790,11 +790,7 @@ NSString *NSStringForContactAddressType(OWSContactAddressType value) } } systemContact.postalAddresses = systemAddresses; - - // TODO: Avatar - - // @property (readonly, copy, nullable, NS_NONATOMIC_IOSONLY) NSData *imageData; - // @property (readonly, copy, nullable, NS_NONATOMIC_IOSONLY) NSData *thumbnailImageData; + systemContact.imageData = imageData; return systemContact; } @@ -816,7 +812,8 @@ NSString *NSStringForContactAddressType(OWSContactAddressType value) { OWSAssert(contact); - CNContact *_Nullable systemContact = [self systemContactForContact:contact]; + // TODO pass in image for vcard + CNContact *_Nullable systemContact = [self systemContactForContact:contact imageData:nil]; if (!systemContact) { return nil; }