Fixes some name collision bugs
- Fixes up content alignment issues from parallel changes to table view styling. - Re-adds view controller presentation within a nav controller (stemming from parallel changes to table view styling) - Fixes a bug around overzealous collision detection Also fixes an unrelated issue. Setting up cv banners performs layout, which can cause the conversation view controller's trait collection to change. This can lead to us reenterantly setting up banners and updating trait collection. The workaround is to just call -ensureBannerState asynchronously from -traitCollectionDidChange to break the synchronous chain.
This commit is contained in:
Michelle Linington
parent
86a60c95b0
commit
f483ee2cac
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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 {
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
Reference in New Issue
Block a user