diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController+Banners.swift b/Signal/src/ViewControllers/ConversationView/ConversationViewController+Banners.swift index 1281e7cc46..98a521e0ac 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController+Banners.swift +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController+Banners.swift @@ -156,7 +156,7 @@ public extension ConversationViewController { banner.reviewAction = { [weak self] in guard let self = self else { return } let vc = NameCollisionResolutionViewController(collisionFinder: collisionFinder, collisionDelegate: self) - self.present(vc, animated: true) + vc.present(fromViewController: self) } return banner @@ -247,7 +247,7 @@ public extension ConversationViewController { banner.reviewAction = { [weak self] in guard let self = self else { return } let vc = NameCollisionResolutionViewController(collisionFinder: collisionFinder, collisionDelegate: self) - self.present(vc, animated: true) + vc.present(fromViewController: self) } return banner diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index bb9abddc5a..66567ee45d 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -3809,10 +3809,15 @@ typedef enum : NSUInteger { - (void)traitCollectionDidChange:(nullable UITraitCollection *)previousTraitCollection { [super traitCollectionDidChange:previousTraitCollection]; - - [self ensureBannerState]; [self updateBarButtonItems]; [self updateNavigationBarSubtitleLabel]; + + // Invoking -ensureBannerState synchronously can lead to reenterant updates to the + // trait collection while building the banners. This can lead us to blow out the stack + // on unrelated trait collection changes (e.g. rotating to landscape). + // We workaround this by just asyncing any banner updates to break the synchronous + // dependency chain. + dispatch_async(dispatch_get_main_queue(), ^{ [self ensureBannerState]; }); } - (void)resetForSizeOrOrientationChange diff --git a/Signal/src/ViewControllers/ThreadSettings/NameCollisionResolutionViewController.swift b/Signal/src/ViewControllers/ThreadSettings/NameCollisionResolutionViewController.swift index 8ee78092d4..2fe27f8113 100644 --- a/Signal/src/ViewControllers/ThreadSettings/NameCollisionResolutionViewController.swift +++ b/Signal/src/ViewControllers/ThreadSettings/NameCollisionResolutionViewController.swift @@ -192,7 +192,7 @@ class NameCollisionResolutionViewController: OWSTableViewController2 { } }() - let actions: [NameCollisionActionCell.Action] = { + let actions: [NameCollisionCell.Action] = { switch (thread: thread, address: model.address, isBlocked: model.isBlocked) { case (thread: is TSContactThread, address: flattenedCellModels.first?.address, isBlocked: false): return [ @@ -217,19 +217,20 @@ class NameCollisionResolutionViewController: OWSTableViewController2 { } }() - let contactInfoCell = NameCollisionReviewContactCell.createWithModel(model) - let section = OWSTableSection(title: header, items: [ - OWSTableItem(customCell: contactInfoCell) + return OWSTableSection(title: header, items: [ + OWSTableItem( + customCellBlock: { + NameCollisionCell.createWithModel(model, actions: actions) + }, + actionBlock: { [weak self] in + guard let self = self else { return } + MemberActionSheet( + address: model.address, + groupViewHelper: self.groupViewHelper + ).present(fromViewController: self) + } + ) ]) - - if actions.isEmpty { - contactInfoCell.isPairedWithActions = false - } else { - contactInfoCell.isPairedWithActions = true - section.add(OWSTableItem(customCell: NameCollisionActionCell(actions: actions))) - } - - return section } // MARK: - Resolution Actions diff --git a/Signal/src/views/NameCollisionReviewCell.swift b/Signal/src/views/NameCollisionReviewCell.swift index a9759cf2e3..87f4be62c8 100644 --- a/Signal/src/views/NameCollisionReviewCell.swift +++ b/Signal/src/views/NameCollisionReviewCell.swift @@ -26,7 +26,7 @@ struct NameCollisionCellModel { extension NameCollision { private func avatar(for address: SignalServiceAddress, transaction: SDSAnyReadTransaction) -> UIImage? { if address.isLocalAddress, let localProfileAvatar = OWSProfileManager.shared().localProfileAvatarImage() { - return localProfileAvatar.resizedImage(to: CGSize(square: 64)) + return localProfileAvatar } else { return OWSContactAvatarBuilder.buildImage( address: address, @@ -86,7 +86,9 @@ extension NameCollision { } } -class NameCollisionReviewContactCell: UITableViewCell { +class NameCollisionCell: UITableViewCell { + typealias Action = (title: String, action: () -> Void) + let avatarView = AvatarImageView() let nameLabel: UILabel = { @@ -149,21 +151,38 @@ class NameCollisionReviewContactCell: UITableViewCell { return label }() - // Rolling our own cell separator. It should be aligned with the name/actions (which is pinned to the safe area) - // The separator UITableView provides does not respect safe area. By handling this ourselves it can now respect - // safe area. Additionally it makes the alignment a bit more explicit. - let separatorHairline: UIView = { + let separatorView: UIView = { + let hairline = UIView() + hairline.backgroundColor = Theme.cellSeparatorColor + hairline.autoSetDimension(.height, toSize: CGHairlineWidth()) let separator = UIView() - separator.backgroundColor = Theme.cellSeparatorColor - separator.autoSetDimension(.height, toSize: CGHairlineWidth()) + separator.addSubview(hairline) + hairline.autoPinEdgesToSuperviewEdges(with: UIEdgeInsets(hMargin: 0, vMargin: 12)) return separator }() + let actionStack: UIStackView = { + let stack = UIStackView() + stack.axis = .horizontal + stack.distribution = .equalSpacing + stack.alignment = .center + stack.spacing = 8 + return stack + }() + required override init(style: UITableViewCell.CellStyle, reuseIdentifier: String?) { super.init(style: style, reuseIdentifier: reuseIdentifier) let verticalStack = UIStackView(arrangedSubviews: [ - nameLabel, phoneNumberLabel, blockedLabel, commonGroupsLabel, nameChangeSpacer, recentNameChangeLabel + nameLabel, + phoneNumberLabel, + blockedLabel, + commonGroupsLabel, + nameChangeSpacer, + recentNameChangeLabel, + UIView.vStretchingSpacer(), + separatorView, + actionStack ]) let horizontalStack = UIStackView(arrangedSubviews: [ avatarView, verticalStack @@ -176,25 +195,23 @@ class NameCollisionReviewContactCell: UITableViewCell { horizontalStack.alignment = .top contentView.addSubview(horizontalStack) - horizontalStack.autoPinEdgesToSuperviewEdges(with: UIEdgeInsets(top: 16, leading: 16, bottom: 0, trailing: 16)) - verticalStack.autoPinEdge(.bottom, to: .bottom, of: contentView, withOffset: -12, relation: .lessThanOrEqual) + horizontalStack.autoPinEdgesToSuperviewMargins() avatarView.autoSetDimensions(to: CGSize(square: 64)) - contentView.addSubview(separatorHairline) - separatorHairline.autoPinLeading(toEdgeOf: verticalStack) - separatorHairline.autoPinTrailing(toEdgeOf: contentView) - separatorHairline.autoPinEdge(.bottom, to: .bottom, of: contentView) - - isPairedWithActions = false + avatarView.autoPinEdge(.bottom, to: .bottom, of: contentView, withOffset: -16, relation: .lessThanOrEqual) + separatorView.autoConstrainAttribute(.horizontal, to: .bottom, of: avatarView, withMultiplier: 1, relation: .greaterThanOrEqual) } required init?(coder: NSCoder) { fatalError("init(coder:) has not been implemented") } - static func createWithModel(_ model: NameCollisionCellModel) -> Self { + static func createWithModel( + _ model: NameCollisionCellModel, + actions: [NameCollisionCell.Action]) -> Self { + let cell = self.init(style: .default, reuseIdentifier: nil) - cell.configure(model: model) + cell.configure(model: model, actions: actions) return cell } @@ -208,10 +225,14 @@ class NameCollisionReviewContactCell: UITableViewCell { recentNameChangeLabel.text = "" } - func configure(model: NameCollisionCellModel) { + func configure(model: NameCollisionCellModel, actions: [NameCollisionCell.Action]) { + owsAssertDebug(actions.count < 3, "Only supports two actions. Feel free to update this for more.") + avatarView.image = model.avatar if model.address.isLocalAddress { - nameLabel.text = NSLocalizedString("GROUP_MEMBER_LOCAL_USER", comment: "Label indicating the local user.") + nameLabel.text = NSLocalizedString( + "GROUP_MEMBER_LOCAL_USER", + comment: "Label indicating the local user.") } else { nameLabel.text = model.name } @@ -236,55 +257,22 @@ class NameCollisionReviewContactCell: UITableViewCell { nameChangeSpacer.isHidden = true recentNameChangeLabel.isHidden = true } - } - lazy var avatarBottomEdgeConstraint: NSLayoutConstraint = { - avatarView.autoPinEdge(.bottom, to: .bottom, of: contentView, withOffset: -16, relation: .lessThanOrEqual) - }() - - // If the cell is paired with actions, we don't need to pad the avatar view - // If the cell is not paired with actions, we can hide our separator - var isPairedWithActions: Bool = false { - didSet { - avatarBottomEdgeConstraint.isActive = !isPairedWithActions - separatorHairline.isHidden = !isPairedWithActions - } - } -} - -class NameCollisionActionCell: UITableViewCell { - typealias Action = (title: String, action: () -> Void) - - init(actions: [Action]) { - owsAssertDebug(actions.count < 3, "Only supports two actions. Feel free to update this for more.") - - super.init(style: .default, reuseIdentifier: nil) - selectionStyle = .none + actionStack.removeAllSubviews() let buttons = actions.map { createButton(for: $0) } - - let horizontalStack = UIStackView(arrangedSubviews: buttons + [UIView()]) - horizontalStack.axis = .horizontal - horizontalStack.distribution = .equalSpacing - horizontalStack.alignment = .center - horizontalStack.spacing = 8 + buttons.forEach { actionStack.addArrangedSubview($0) } + actionStack.addArrangedSubview(UIView.vStretchingSpacer()) // If one button grows super tall, its larger intrinsic content size could result in the other button // being compressed very thin and tall in response. It's unlikely, since this would only be hit by a very - // edge case localization. But, if it does happen, things will look reasonably okay. + // edge case localization. But, if it does happen, these constraints ensure things will look reasonably okay. if let button1 = buttons[safe: 0], let button2 = buttons[safe: 1] { button1.autoSetDimension(.width, toSize: min(100, button1.intrinsicContentSize.width), relation: .greaterThanOrEqual) button2.autoSetDimension(.width, toSize: min(100, button2.intrinsicContentSize.width), relation: .greaterThanOrEqual) } - contentView.addSubview(horizontalStack) - horizontalStack.autoPinEdge(toSuperviewEdge: .top, withInset: 8) - horizontalStack.autoPinEdge(toSuperviewEdge: .bottom, withInset: 8) - horizontalStack.autoPinEdge(toSuperviewEdge: .leading, withInset: 96) - horizontalStack.autoPinEdge(toSuperviewEdge: .trailing) - } - - required init?(coder: NSCoder) { - fatalError("init(coder:) has not been implemented") + separatorView.isHidden = actions.isEmpty + actionStack.isHidden = actions.isEmpty } private func createButton(for action: Action) -> UIButton { diff --git a/SignalMessaging/profiles/NameCollisionFinder.swift b/SignalMessaging/profiles/NameCollisionFinder.swift index c886196d65..855d08f031 100644 --- a/SignalMessaging/profiles/NameCollisionFinder.swift +++ b/SignalMessaging/profiles/NameCollisionFinder.swift @@ -1,5 +1,5 @@ // -// Copyright (c) 2020 Open Whisper Systems. All rights reserved. +// Copyright (c) 2021 Open Whisper Systems. All rights reserved. // /// A collection of addresses (and adjacent info) that collide (i.e. the user many confuse one element's `currentName` for another) @@ -87,7 +87,7 @@ public class ContactThreadNameCollisionFinder: NameCollisionFinder { guard address1 != address2 else { return false } let name1 = address1.displayName(transaction: transaction) - let name2 = address1.displayName(transaction: transaction) + let name2 = address2.displayName(transaction: transaction) return name1 == name2 } } @@ -100,7 +100,7 @@ public class GroupMembershipNameCollisionFinder: NameCollisionFinder { /// "Recent" is defined as all profile update messages since a call to `markCollisionsAsResolved` /// This is only fetched once for the lifetime of the collision finder. Thread-safe. let lock = UnfairLock() - private var recentProfileUpdateMessages: [SignalServiceAddress: [TSInfoMessage]]? = nil + private var recentProfileUpdateMessages: [SignalServiceAddress: [TSInfoMessage]]? public var hasFetchedProfileUpdateMessages: Bool { lock.withLock { recentProfileUpdateMessages != nil } } @@ -114,7 +114,7 @@ public class GroupMembershipNameCollisionFinder: NameCollisionFinder { return [] } groupThread = updatedThread - + // Build a dictionary mapping displayName -> (All addresses with that name) let groupMembers = groupThread.groupModel.groupMembers let collisionMap: [String: [SignalServiceAddress]] = groupMembers.reduce(into: [:]) { (dictBuilder, address) in