From 875a3adde8a8fa23fac122db608f9e53cce23da7 Mon Sep 17 00:00:00 2001 From: Nora Trapp Date: Tue, 13 Aug 2019 20:57:30 -0700 Subject: [PATCH] PR Feedback --- .../AppSettings/AppSettingsViewController.m | 2 +- .../Cells/ThreadDetailsCell.swift | 4 +- .../ViewControllers/ProfileViewController.m | 148 ++++++++---------- .../OWSConversationSettingsViewController.m | 3 +- .../UsernameViewController.swift | 18 +-- SignalMessaging/Views/CommonStrings.swift | 8 +- .../Network/API/Requests/OWSRequestFactory.m | 6 +- SignalServiceKit/src/TSConstants.h | 3 - 8 files changed, 87 insertions(+), 105 deletions(-) diff --git a/Signal/src/ViewControllers/AppSettings/AppSettingsViewController.m b/Signal/src/ViewControllers/AppSettings/AppSettingsViewController.m index 8a2949f8e6..f2a46e9ed3 100644 --- a/Signal/src/ViewControllers/AppSettings/AppSettingsViewController.m +++ b/Signal/src/ViewControllers/AppSettings/AppSettingsViewController.m @@ -360,7 +360,7 @@ NSString *_Nullable username = [OWSProfileManager.sharedManager localUsername]; if (username.length > 0) { - addSubtitle([CommonStrings.usernamePrefix stringByAppendingString:username]); + addSubtitle([CommonFormats formatUsername:username]); } [lastTitleView autoPinEdgeToSuperviewEdge:ALEdgeBottom]; diff --git a/Signal/src/ViewControllers/ConversationView/Cells/ThreadDetailsCell.swift b/Signal/src/ViewControllers/ConversationView/Cells/ThreadDetailsCell.swift index 5950127286..30b48eea65 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/ThreadDetailsCell.swift +++ b/Signal/src/ViewControllers/ConversationView/Cells/ThreadDetailsCell.swift @@ -172,9 +172,9 @@ public class ThreadDetailsCell: ConversationViewCell { } if let username = OWSProfileManager.shared().username(for: contactThread.contactAddress) { if let existingDetails = details { - details = existingDetails + "\n" + CommonStrings.usernamePrefix + username + details = existingDetails + "\n" + CommonFormats.formatUsername(username) } else { - details = CommonStrings.usernamePrefix + username + details = CommonFormats.formatUsername(username) } } } else { diff --git a/Signal/src/ViewControllers/ProfileViewController.m b/Signal/src/ViewControllers/ProfileViewController.m index 13b710832d..20e3bd6cc1 100644 --- a/Signal/src/ViewControllers/ProfileViewController.m +++ b/Signal/src/ViewControllers/ProfileViewController.m @@ -118,19 +118,34 @@ NSString *const kProfileView_LastPresentedDate = @"kProfileView_LastPresentedDat { self.view.backgroundColor = Theme.backgroundColor; - UIView *contentView = [UIView containerView]; - contentView.backgroundColor = Theme.backgroundColor; - [self.view addSubview:contentView]; - [contentView autoPinToTopLayoutGuideOfViewController:self withInset:0]; - [contentView autoPinWidthToSuperview]; + UIStackView *stackView = [UIStackView new]; + stackView.axis = UILayoutConstraintAxisVertical; + stackView.spacing = 0; - NSMutableArray *rows = [NSMutableArray new]; + [self.view addSubview:stackView]; + [stackView autoPinToTopLayoutGuideOfViewController:self withInset:15.f]; + [stackView autoPinWidthToSuperview]; + + void (^addSeparator)(BOOL) = ^(BOOL withLeadingInset) { + UIView *separatorWrapper = [UIView containerView]; + [stackView addArrangedSubview:separatorWrapper]; + UIView *separator = [UIView containerView]; + separator.backgroundColor = Theme.cellSeparatorColor; + [separatorWrapper addSubview:separator]; + [separator autoPinHeightToSuperview]; + [separator autoPinLeadingToSuperviewMarginWithInset:withLeadingInset ? 18 : 0]; + [separator autoPinTrailingToSuperviewMargin]; + [separator autoSetDimension:ALDimensionHeight toSize:CGHairlineWidth()]; + }; + + CGFloat rowSpacing = 10; + UIEdgeInsets rowMargins = UIEdgeInsetsMake(10, 18, 10, 18); // Avatar UIView *avatarRow = [UIView containerView]; - [rows addObject:avatarRow]; + [stackView addArrangedSubview:avatarRow]; self.avatarView = [AvatarImageView new]; self.avatarView.userInteractionEnabled = YES; @@ -164,24 +179,26 @@ NSString *const kProfileView_LastPresentedDate = @"kProfileView_LastPresentedDat [self updateAvatarView]; + addSeparator(NO); + // Name - UIView *profileNameRow = [UIView containerView]; - profileNameRow.userInteractionEnabled = YES; + UIStackView *profileNameRow = [UIStackView new]; + profileNameRow.spacing = rowSpacing; + profileNameRow.layoutMarginsRelativeArrangement = YES; + profileNameRow.layoutMargins = rowMargins; [profileNameRow addGestureRecognizer:[[UITapGestureRecognizer alloc] initWithTarget:self action:@selector(profileNameRowTapped:)]]; - profileNameRow.accessibilityIdentifier = ACCESSIBILITY_IDENTIFIER_WITH_NAME(self, @"nameRow"); - [rows addObject:profileNameRow]; + profileNameRow.accessibilityIdentifier = ACCESSIBILITY_IDENTIFIER_WITH_NAME(self, @"profileNameRow"); + [stackView addArrangedSubview:profileNameRow]; UILabel *profileNameLabel = [UILabel new]; profileNameLabel.text = NSLocalizedString( @"PROFILE_VIEW_PROFILE_NAME_FIELD", @"Label for the profile name field of the profile view."); profileNameLabel.textColor = Theme.primaryColor; profileNameLabel.font = [[UIFont ows_dynamicTypeBodyClampedFont] ows_mediumWeight]; - [profileNameRow addSubview:profileNameLabel]; - [profileNameLabel autoPinLeadingToSuperviewMargin]; - [profileNameLabel autoPinHeightToSuperviewWithMargin:5.f]; + [profileNameRow addArrangedSubview:profileNameLabel]; UITextField *profileNameTextField = [OWSTextField new]; _profileNameTextField = profileNameTextField; @@ -197,64 +214,67 @@ NSString *const kProfileView_LastPresentedDate = @"kProfileView_LastPresentedDat [profileNameTextField addTarget:self action:@selector(textFieldDidChange:) forControlEvents:UIControlEventEditingChanged]; - [profileNameRow addSubview:profileNameTextField]; - [profileNameTextField autoPinLeadingToTrailingEdgeOfView:profileNameLabel offset:10.f]; - [profileNameTextField autoPinTrailingToSuperviewMargin]; - [profileNameTextField autoVCenterInSuperview]; + [profileNameRow addArrangedSubview:profileNameTextField]; // Username if (SSKFeatureFlags.usernames) { - UIView *usernameRow = [UIView containerView]; - usernameRow.userInteractionEnabled = YES; + addSeparator(YES); + + UIStackView *usernameRow = [UIStackView new]; + usernameRow.spacing = rowSpacing; + usernameRow.layoutMarginsRelativeArrangement = YES; + usernameRow.layoutMargins = rowMargins; [usernameRow addGestureRecognizer:[[UITapGestureRecognizer alloc] initWithTarget:self action:@selector(usernameRowTapped:)]]; usernameRow.accessibilityIdentifier = ACCESSIBILITY_IDENTIFIER_WITH_NAME(self, @"nameRow"); - [rows addObject:usernameRow]; + [stackView addArrangedSubview:usernameRow]; UILabel *usernameTitleLabel = [UILabel new]; usernameTitleLabel.text = NSLocalizedString(@"PROFILE_VIEW_USERNAME_FIELD", @"Label for the username field of the profile view."); usernameTitleLabel.textColor = Theme.primaryColor; usernameTitleLabel.font = [[UIFont ows_dynamicTypeBodyClampedFont] ows_mediumWeight]; - [usernameRow addSubview:usernameTitleLabel]; - [usernameTitleLabel autoPinLeadingToSuperviewMargin]; - [usernameTitleLabel autoPinHeightToSuperviewWithMargin:5.f]; + [usernameRow addArrangedSubview:usernameTitleLabel]; + + UILabel *usernameLabel = [UILabel new]; + + usernameLabel.font = [UIFont ows_dynamicTypeBodyClampedFont]; + usernameLabel.textAlignment = NSTextAlignmentRight; + [usernameRow addArrangedSubview:usernameLabel]; + + _usernameLabel = usernameLabel; + + UIView *disclosureImageContainer = [UIView containerView]; + [usernameRow addArrangedSubview:disclosureImageContainer]; NSString *disclosureImageName = CurrentAppContext().isRTL ? @"system_disclosure_indicator_rtl" : @"system_disclosure_indicator"; UIImageView *disclosureImageView = [UIImageView new]; [disclosureImageView setTemplateImageName:disclosureImageName tintColor:Theme.cellSeparatorColor]; - [usernameRow addSubview:disclosureImageView]; - [disclosureImageView autoPinTrailingToSuperviewMargin]; + [disclosureImageContainer addSubview:disclosureImageView]; + [disclosureImageView autoPinWidthToSuperview]; [disclosureImageView autoVCenterInSuperview]; [disclosureImageView autoSetDimension:ALDimensionHeight toSize:13]; [disclosureImageView autoSetDimension:ALDimensionWidth toSize:11]; - UILabel *usernameLabel = [UILabel new]; - - usernameLabel.font = [UIFont ows_dynamicTypeBodyClampedFont]; - usernameLabel.textAlignment = NSTextAlignmentRight; - [usernameRow addSubview:usernameLabel]; - [usernameLabel autoPinTrailingToLeadingEdgeOfView:disclosureImageView offset:7.f]; - [usernameLabel autoPinLeadingToTrailingEdgeOfView:usernameTitleLabel offset:10.f]; - [usernameLabel autoPinHeightToSuperviewWithMargin:5.f]; - - _usernameLabel = usernameLabel; - [self updateUsername]; + + addSeparator(NO); + } else { + addSeparator(NO); } // Information - UIView *infoRow = [UIView containerView]; - infoRow.userInteractionEnabled = YES; + UIView *infoRow = [UIView new]; + infoRow.layoutMargins = rowMargins; [infoRow addGestureRecognizer:[[UITapGestureRecognizer alloc] initWithTarget:self action:@selector(infoRowTapped:)]]; infoRow.accessibilityIdentifier = ACCESSIBILITY_IDENTIFIER_WITH_NAME(self, @"infoRow"); - [rows addObject:infoRow]; + [stackView addArrangedSubview:infoRow]; UILabel *infoLabel = [UILabel new]; infoLabel.textColor = Theme.secondaryColor; @@ -276,16 +296,14 @@ NSString *const kProfileView_LastPresentedDate = @"kProfileView_LastPresentedDat infoLabel.numberOfLines = 0; infoLabel.lineBreakMode = NSLineBreakByWordWrapping; [infoRow addSubview:infoLabel]; - [infoLabel autoPinLeadingToSuperviewMargin]; - [infoLabel autoPinTrailingToSuperviewMargin]; - [infoLabel autoPinEdgeToSuperviewEdge:ALEdgeTop withInset:10.f]; - [infoLabel autoPinEdgeToSuperviewEdge:ALEdgeBottom withInset:10.f]; + [infoLabel autoPinEdgesToSuperviewMargins]; // Big Button if (self.profileViewMode == ProfileViewMode_Registration) { - UIView *buttonRow = [UIView containerView]; - [rows addObject:buttonRow]; + UIView *buttonRow = [UIView new]; + buttonRow.layoutMargins = rowMargins; + [stackView addArrangedSubview:buttonRow]; const CGFloat kButtonHeight = 47.f; // NOTE: We use ows_signalBrandBlueColor instead of ows_materialBlueColor @@ -301,43 +319,9 @@ NSString *const kProfileView_LastPresentedDate = @"kProfileView_LastPresentedDat SET_SUBVIEW_ACCESSIBILITY_IDENTIFIER(self, saveButton); self.saveButton = saveButton; [buttonRow addSubview:saveButton]; - [saveButton autoPinLeadingAndTrailingToSuperviewMargin]; - [saveButton autoPinHeightToSuperview]; + [saveButton autoPinEdgesToSuperviewMargins]; [saveButton autoSetDimension:ALDimensionHeight toSize:47.f]; } - - // Row Layout - - UIView *_Nullable lastRow = nil; - for (UIView *row in rows) { - [contentView addSubview:row]; - if (lastRow) { - [row autoPinEdge:ALEdgeTop toEdge:ALEdgeBottom ofView:lastRow withOffset:5.f]; - } else { - [row autoPinEdgeToSuperviewEdge:ALEdgeTop withInset:15.f]; - } - [row autoPinLeadingToSuperviewMarginWithInset:18.f]; - [row autoPinTrailingToSuperviewMarginWithInset:18.f]; - lastRow = row; - - if (lastRow != infoRow) { - UIView *separator = [UIView containerView]; - separator.backgroundColor = Theme.cellSeparatorColor; - [contentView addSubview:separator]; - [separator autoPinEdge:ALEdgeTop toEdge:ALEdgeBottom ofView:lastRow withOffset:5.f]; - - if (lastRow == profileNameRow && SSKFeatureFlags.usernames) { - [separator autoPinLeadingToSuperviewMarginWithInset:18.f]; - [separator autoPinTrailingToSuperviewMargin]; - } else { - [separator autoPinWidthToSuperview]; - } - - [separator autoSetDimension:ALDimensionHeight toSize:CGHairlineWidth()]; - lastRow = separator; - } - } - [lastRow autoPinEdgeToSuperviewEdge:ALEdgeBottom withInset:10.f]; } - (void)viewWillAppear:(BOOL)animated @@ -581,7 +565,7 @@ NSString *const kProfileView_LastPresentedDate = @"kProfileView_LastPresentedDat { NSString *_Nullable username = [OWSProfileManager.sharedManager localUsername]; if (username) { - self.usernameLabel.text = [CommonStrings.usernamePrefix stringByAppendingString:username]; + self.usernameLabel.text = [CommonFormats formatUsername:username]; self.usernameLabel.textColor = Theme.primaryColor; } else { self.usernameLabel.text = NSLocalizedString(@"PROFILE_VIEW_CREATE_USERNAME", diff --git a/Signal/src/ViewControllers/ThreadSettings/OWSConversationSettingsViewController.m b/Signal/src/ViewControllers/ThreadSettings/OWSConversationSettingsViewController.m index c71259abf9..1c3e14a738 100644 --- a/Signal/src/ViewControllers/ThreadSettings/OWSConversationSettingsViewController.m +++ b/Signal/src/ViewControllers/ThreadSettings/OWSConversationSettingsViewController.m @@ -1011,8 +1011,7 @@ const CGFloat kIconViewLength = 24; NSString *_Nullable username = [OWSProfileManager.sharedManager usernameForAddress:recipientAddress]; if (username.length > 0) { - addSubtitle([[NSAttributedString alloc] - initWithString:[CommonStrings.usernamePrefix stringByAppendingString:username]]); + addSubtitle([[NSAttributedString alloc] initWithString:[CommonFormats formatUsername:username]]); } #if DEBUG diff --git a/Signal/src/ViewControllers/UsernameViewController.swift b/Signal/src/ViewControllers/UsernameViewController.swift index 057d37cc17..83bdd5294d 100644 --- a/Signal/src/ViewControllers/UsernameViewController.swift +++ b/Signal/src/ViewControllers/UsernameViewController.swift @@ -175,25 +175,21 @@ class UsernameViewController: OWSViewController { } SSKEnvironment.shared.networkManager.makePromise(request: usernameRequest).done { _ in - DispatchQueue.main.async { - modalView.dismiss { - OWSProfileManager.shared().updateLocalUsername(usernameToUse) - self.usernameSavedOrCanceled() - } + modalView.dismiss { + OWSProfileManager.shared().updateLocalUsername(usernameToUse) + self.usernameSavedOrCanceled() } }.catch { error in if case .taskError(let task, _)? = error as? NetworkManagerError, task.statusCode() == 409 { - DispatchQueue.main.async { modalView.dismiss { self.validationState = .inUse } } + modalView.dismiss { self.validationState = .inUse } return } owsFailDebug("Unexpected username update error \(error)") - DispatchQueue.main.async { - modalView.dismiss { - OWSAlerts.showErrorAlert(message: NSLocalizedString("USERNAME_VIEW_ERROR_UPDATE_FAILED", - comment: "Error moessage shown when a username update fails.")) - } + modalView.dismiss { + OWSAlerts.showErrorAlert(message: NSLocalizedString("USERNAME_VIEW_ERROR_UPDATE_FAILED", + comment: "Error moessage shown when a username update fails.")) } } } diff --git a/SignalMessaging/Views/CommonStrings.swift b/SignalMessaging/Views/CommonStrings.swift index 1c5ad54adb..1885d8cd30 100644 --- a/SignalMessaging/Views/CommonStrings.swift +++ b/SignalMessaging/Views/CommonStrings.swift @@ -32,9 +32,15 @@ import Foundation @objc static public let searchPlaceholder = NSLocalizedString("SEARCH_FIELD_PLACE_HOLDER_TEXT", comment: "placeholder text in an empty search field") +} +@objc +public class CommonFormats: NSObject { @objc - static public let usernamePrefix = NSLocalizedString("USERNAME_PREFIX", comment: "A prefix appeneded to all usernames when displayed") + static public func formatUsername(_ username: String) -> String { + return NSLocalizedString("USERNAME_PREFIX", + comment: "A prefix appeneded to all usernames when displayed") + username.filterForDisplay + } } @objc diff --git a/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.m b/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.m index c9658289d8..046c14ce73 100644 --- a/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.m +++ b/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.m @@ -692,20 +692,20 @@ NS_ASSUME_NONNULL_BEGIN { OWSAssertDebug(username.length > 0); - NSString *path = [NSString stringWithFormat:textSecureSetUsernameAPIFormat, username]; + NSString *path = [NSString stringWithFormat:@"v1/accounts/username/%@", username]; return [TSRequest requestWithUrl:[NSURL URLWithString:path] method:@"PUT" parameters:@{}]; } + (TSRequest *)usernameDeleteRequest { - return [TSRequest requestWithUrl:[NSURL URLWithString:textSecureUsernameAPI] method:@"DELETE" parameters:@{}]; + return [TSRequest requestWithUrl:[NSURL URLWithString:@"v1/accounts/username"] method:@"DELETE" parameters:@{}]; } + (TSRequest *)getProfileRequestWithUsername:(NSString *)username { OWSAssertDebug(username.length > 0); - NSString *path = [NSString stringWithFormat:textSecureProfileByUsernameAPIFormat, username]; + NSString *path = [NSString stringWithFormat:@"v1/profile/username/%@", username]; return [TSRequest requestWithUrl:[NSURL URLWithString:path] method:@"GET" parameters:@{}]; } diff --git a/SignalServiceKit/src/TSConstants.h b/SignalServiceKit/src/TSConstants.h index e28fff3bb9..b7360ac782 100644 --- a/SignalServiceKit/src/TSConstants.h +++ b/SignalServiceKit/src/TSConstants.h @@ -97,9 +97,6 @@ BOOL IsUsingProductionService(void); #define textSecureProfileAvatarFormAPI @"v1/profile/form/avatar" #define textSecure2FAAPI @"v1/accounts/pin" #define textSecureRegistrationLockV2API @"v1/accounts/registration_lock" -#define textSecureSetUsernameAPIFormat @"v1/accounts/username/%@" -#define textSecureProfileByUsernameAPIFormat @"v1/profile/username/%@" -#define textSecureUsernameAPI @"v1/accounts/username" #define SignalApplicationGroup @"group.org.whispersystems.signal.group"