PR Feedback
- More robust handling of large button labels in the collision view controller - Fixes for VoiceOver labels and navigation - Fixes for layout issues when rotating - Update copy for collision banner - Remove banner shadow for all banners
This commit is contained in:
Michelle Linington
parent
1de7295f39
commit
b152d985b0
@ -357,6 +357,8 @@ private class MessageRequestNameCollisionBanner: UIView {
|
||||
let button = OWSButton(
|
||||
imageName: "x-circle-16",
|
||||
tintColor: Theme.secondaryTextAndIconColor)
|
||||
button.accessibilityLabel = NSLocalizedString("BANNER_CLOSE_ACCESSIBILITY_LABEL",
|
||||
comment: "Accessibility label for banner close button")
|
||||
button.translatesAutoresizingMaskIntoConstraints = false
|
||||
button.setCompressionResistanceHigh()
|
||||
button.setContentHuggingHigh()
|
||||
@ -364,8 +366,7 @@ private class MessageRequestNameCollisionBanner: UIView {
|
||||
}()
|
||||
|
||||
private let reviewButton: OWSButton = {
|
||||
let buttonText = NSLocalizedString(
|
||||
"MESSAGE_REQUEST_REVIEW_NAME_COLLISION",
|
||||
let buttonText = NSLocalizedString("MESSAGE_REQUEST_REVIEW_NAME_COLLISION",
|
||||
comment: "Button to allow user to review known name collisions with an incoming message request")
|
||||
|
||||
let button = OWSButton(title: buttonText)
|
||||
@ -401,6 +402,8 @@ private class MessageRequestNameCollisionBanner: UIView {
|
||||
closeButton.imageView?.autoPinLeading(toTrailingEdgeOf: label, offset: 16)
|
||||
closeButton.imageView?.autoPinTrailing(toEdgeOf: self, offset: -16)
|
||||
reviewButton.titleLabel?.autoPinLeading(toEdgeOf: label)
|
||||
|
||||
accessibilityElements = [label, reviewButton, closeButton]
|
||||
}
|
||||
|
||||
required init?(coder: NSCoder) {
|
||||
|
||||
@ -782,18 +782,6 @@ typedef enum : NSUInteger {
|
||||
[bannerView autoPinToTopLayoutGuideOfViewController:self withInset:0];
|
||||
[bannerView autoPinEdgeToSuperviewEdge:ALEdgeLeading];
|
||||
[bannerView autoPinEdgeToSuperviewEdge:ALEdgeTrailing];
|
||||
|
||||
UIView *bannerShadow = [UIView new];
|
||||
bannerShadow.backgroundColor = Theme.backgroundColor;
|
||||
// Use a shadow to "pop" the indicator above the other views.
|
||||
bannerShadow.layer.shadowColor = [UIColor blackColor].CGColor;
|
||||
bannerShadow.layer.shadowOffset = CGSizeMake(0, 4);
|
||||
bannerShadow.layer.shadowRadius = 4.f;
|
||||
bannerShadow.layer.shadowOpacity = 0.15f;
|
||||
[bannerView addSubview:bannerShadow];
|
||||
[bannerShadow autoPinEdgesToSuperviewEdges];
|
||||
[bannerView sendSubviewToBack:bannerShadow];
|
||||
|
||||
[self.view layoutSubviews];
|
||||
|
||||
self.bannerView = bannerView;
|
||||
|
||||
@ -33,6 +33,7 @@ class MessageRequestNameCollisionViewController: OWSTableViewController {
|
||||
override func viewDidLoad() {
|
||||
super.viewDidLoad()
|
||||
updateModel()
|
||||
tableView.separatorStyle = .none
|
||||
}
|
||||
|
||||
override func viewWillAppear(_ animated: Bool) {
|
||||
@ -45,6 +46,15 @@ class MessageRequestNameCollisionViewController: OWSTableViewController {
|
||||
action: #selector(donePressed))
|
||||
}
|
||||
|
||||
override func viewWillTransition(to size: CGSize, with coordinator: UIViewControllerTransitionCoordinator) {
|
||||
super.viewWillTransition(to: size, with: coordinator)
|
||||
coordinator.animate { _ in
|
||||
// Force tableview to recalculate self-sized cell height
|
||||
self.tableView.beginUpdates()
|
||||
self.tableView.endUpdates()
|
||||
}
|
||||
}
|
||||
|
||||
func updateModel() {
|
||||
databaseStorage.uiRead { readTx in
|
||||
self.requesterModel = NameCollisionModel.buildFromAddress(
|
||||
|
||||
@ -68,6 +68,16 @@ 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 separator = UIView()
|
||||
separator.backgroundColor = Theme.cellSeparatorColor
|
||||
separator.autoSetDimension(.height, toSize: CGHairlineWidth())
|
||||
return separator
|
||||
}()
|
||||
|
||||
required override init(style: UITableViewCell.CellStyle, reuseIdentifier: String?) {
|
||||
super.init(style: style, reuseIdentifier: reuseIdentifier)
|
||||
|
||||
@ -85,14 +95,15 @@ class NameCollisionReviewContactCell: UITableViewCell {
|
||||
horizontalStack.alignment = .top
|
||||
|
||||
contentView.addSubview(horizontalStack)
|
||||
horizontalStack.autoPinEdge(toSuperviewSafeArea: .leading, withInset: 16)
|
||||
horizontalStack.autoPinEdge(toSuperviewSafeArea: .trailing, withInset: 16)
|
||||
horizontalStack.autoPinEdge(toSuperviewEdge: .top, withInset: 16)
|
||||
horizontalStack.autoPinEdge(toSuperviewEdge: .bottom)
|
||||
|
||||
horizontalStack.autoPinEdgesToSuperviewEdges(with: UIEdgeInsets(top: 16, leading: 16, bottom: 0, trailing: 16))
|
||||
verticalStack.autoPinEdge(.bottom, to: .bottom, of: contentView, withOffset: -12, relation: .lessThanOrEqual)
|
||||
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
|
||||
}
|
||||
|
||||
@ -127,7 +138,7 @@ class NameCollisionReviewContactCell: UITableViewCell {
|
||||
|
||||
commonGroupsLabel.text = model.commonGroupsString
|
||||
|
||||
if let oldName = model.oldName {
|
||||
if let _ = model.oldName {
|
||||
nameChangeSpacer.isHidden = false
|
||||
recentNameChangeLabel.isHidden = false
|
||||
recentNameChangeLabel.text = "" // TODO
|
||||
@ -141,11 +152,12 @@ class NameCollisionReviewContactCell: UITableViewCell {
|
||||
avatarView.autoPinEdge(.bottom, to: .bottom, of: contentView, withOffset: -16, relation: .lessThanOrEqual)
|
||||
}()
|
||||
|
||||
// If this cell is paired with actions, we don't need to pad the avatar view
|
||||
// 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
|
||||
separatorInset = UIEdgeInsets(top: 0, leading: isPairedWithActions ? 96 : 0, bottom: 0, trailing: 0)
|
||||
separatorHairline.isHidden = !isPairedWithActions
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -154,29 +166,49 @@ class NameCollisionActionCell: UITableViewCell {
|
||||
typealias Action = (title: String, action: () -> Void)
|
||||
|
||||
init(actions: [Action]) {
|
||||
owsAssertDebug(actions.count < 3, "Untested above two actions. This is likely to truncate button text")
|
||||
owsAssertDebug(actions.count < 3, "Only supports two actions. Feel free to update this for more.")
|
||||
|
||||
super.init(style: .default, reuseIdentifier: nil)
|
||||
selectionStyle = .none
|
||||
|
||||
let buttons = actions.map { (action: Action) -> UIButton in
|
||||
let button = OWSButton(title: action.title, block: action.action)
|
||||
button.setTitleColor(Theme.accentBlueColor, for: .normal)
|
||||
button.setTitleColor(Theme.accentBlueColor.withAlphaComponent(0.7), for: .highlighted)
|
||||
button.titleLabel?.font = UIFont.ows_dynamicTypeSubheadlineClamped.ows_semibold
|
||||
button.titleLabel?.adjustsFontForContentSizeCategory = true
|
||||
return button
|
||||
}
|
||||
let buttons = actions.map { createButton(for: $0) }
|
||||
|
||||
let horizontalStack = UIStackView(arrangedSubviews: buttons + [UIView()])
|
||||
horizontalStack.axis = .horizontal
|
||||
horizontalStack.distribution = .equalCentering
|
||||
horizontalStack.distribution = .equalSpacing
|
||||
horizontalStack.alignment = .center
|
||||
horizontalStack.spacing = 8
|
||||
|
||||
// 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.
|
||||
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.autoPinEdgesToSuperviewSafeArea(with: UIEdgeInsets(top: 8, leading: 96, bottom: 8, trailing: 0))
|
||||
|
||||
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")
|
||||
}
|
||||
|
||||
private func createButton(for action: Action) -> UIButton {
|
||||
let button = OWSButton(title: action.title, block: action.action)
|
||||
button.setTitleColor(Theme.accentBlueColor, for: .normal)
|
||||
button.setTitleColor(Theme.accentBlueColor.withAlphaComponent(0.7), for: .highlighted)
|
||||
button.titleLabel?.font = UIFont.ows_dynamicTypeSubheadlineClamped.ows_semibold
|
||||
button.titleLabel?.adjustsFontForContentSizeCategory = true
|
||||
button.titleLabel?.numberOfLines = 0
|
||||
button.contentHorizontalAlignment = .leading
|
||||
|
||||
// By default, a button's label will grow outside of the buttons bounds
|
||||
button.titleLabel?.autoMatch(.height, to: .height, of: button, withMultiplier: 1, relation: .lessThanOrEqual)
|
||||
button.setContentHuggingHorizontalHigh()
|
||||
return button
|
||||
}
|
||||
}
|
||||
|
||||
@ -358,6 +358,9 @@
|
||||
/* Error shown when backup fails due to an unexpected error. */
|
||||
"BACKUP_UNEXPECTED_ERROR" = "Unexpected Backup Error";
|
||||
|
||||
/* Accessibility label for banner close button */
|
||||
"BANNER_CLOSE_ACCESSIBILITY_LABEL" = "Close banner";
|
||||
|
||||
/* An explanation of the consequences of blocking a group. */
|
||||
"BLOCK_GROUP_BEHAVIOR_EXPLANATION" = "You will no longer receive messages or updates from this group.";
|
||||
|
||||
@ -2780,7 +2783,7 @@
|
||||
"MESSAGE_REQUEST_LEAVE_AND_DELETE_GROUP_TITLE" = "Delete and Leave Group?";
|
||||
|
||||
/* Banner label notifying user that a new message is from a user with the same name as an existing contact */
|
||||
"MESSAGE_REQUEST_NAME_COLLISON_BANNER_LABEL" = "Review requests carefuly Signal found another contact with the same name.";
|
||||
"MESSAGE_REQUEST_NAME_COLLISON_BANNER_LABEL" = "Review requests carefully. Signal found another contact with the same name.";
|
||||
|
||||
/* A header string above a known contact's contact info */
|
||||
"MESSAGE_REQUEST_NAME_COLLISON_CONTACT_HEADER" = "Your contact";
|
||||
|
||||
Loading…
Reference in New Issue
Block a user