Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move confirmation prompt to a static function #21006

Closed
wants to merge 1 commit into from
Closed

Move confirmation prompt to a static function #21006

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 49 additions & 47 deletions ...ress/Classes/ViewRelated/Me/My Profile/Change Username/ChangeUsernameViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import Combine
import WordPressAuthenticator

class ChangeUsernameViewController: SignupUsernameTableViewController {
Expand All @@ -15,7 +16,6 @@ class ChangeUsernameViewController: SignupUsernameTableViewController {
}
return saveItem
}()
private var changeUsernameAction: UIAlertAction?

init(service: AccountSettingsService, settings: AccountSettings?, completionBlock: @escaping CompletionBlock) {
self.viewModel = ChangeUsernameViewModel(service: service, settings: settings)
Expand Down Expand Up @@ -105,7 +105,10 @@ private extension ChangeUsernameViewController {
}

func save() {
present(changeUsernameConfirmationPrompt(), animated: true)
let controller = UIAlertController.confirmationPrompt(forSelectedUsername: viewModel.selectedUsername) { [weak self] in
self?.changeUsername()
}
present(controller, animated: true)
}

func changeUsername() {
Expand All @@ -129,50 +132,6 @@ private extension ChangeUsernameViewController {
}
}

func changeUsernameConfirmationPrompt() -> UIAlertController {
let alertController = UIAlertController(title: Constants.Alert.title,
message: "",
preferredStyle: .alert)
alertController.addAttributeMessage(String(format: Constants.Alert.message, viewModel.selectedUsername),
highlighted: viewModel.selectedUsername)
alertController.addCancelActionWithTitle(Constants.Alert.cancel, handler: { [weak alertController] _ in
if let textField = alertController?.textFields?.first {
NotificationCenter.default.removeObserver(textField, name: UITextField.textDidChangeNotification, object: nil)
}
DDLogInfo("User cancelled alert")
})
changeUsernameAction = alertController.addDefaultActionWithTitle(Constants.Alert.change, handler: { [weak alertController] _ in
guard let textField = alertController?.textFields?.first,
textField.text == self.viewModel.selectedUsername else {
DDLogInfo("Username confirmation failed")
return
}
DDLogInfo("User changes username")
NotificationCenter.default.removeObserver(textField, name: UITextField.textDidChangeNotification, object: nil)
self.changeUsername()
})
changeUsernameAction?.isEnabled = false
alertController.addTextField { [weak self] textField in
textField.placeholder = Constants.Alert.confirm
NotificationCenter.default.addObserver(forName: UITextField.textDidChangeNotification,
object: textField,
queue: .main) {_ in
if let text = textField.text,
!text.isEmpty,
let username = self?.viewModel.selectedUsername,
text == username {
self?.changeUsernameAction?.isEnabled = true
textField.textColor = .success
return
}
self?.changeUsernameAction?.isEnabled = false
textField.textColor = .text
}
}
DDLogInfo("Prompting user for confirmation of change username")
return alertController
}

enum Constants {
static let actionButtonTitle = NSLocalizedString("Save", comment: "Settings Text save button title")
static let username = NSLocalizedString("Username", comment: "The header and main title")
Expand All @@ -189,7 +148,50 @@ private extension ChangeUsernameViewController {
}
}

fileprivate extension UIAlertController {
private extension UIAlertController {
static var cancellableKey = 0

static func confirmationPrompt(forSelectedUsername selectedUsername: String, confirmed: @escaping () -> Void) -> UIAlertController {
typealias Constants = ChangeUsernameViewController.Constants
let alertController = UIAlertController(title: Constants.Alert.title, message: "", preferredStyle: .alert)
alertController.addAttributeMessage(String(format: Constants.Alert.message, selectedUsername),
highlighted: selectedUsername)
alertController.addCancelActionWithTitle(Constants.Alert.cancel, handler: { _ in
DDLogInfo("User cancelled alert")
})

let action = alertController.addDefaultActionWithTitle(Constants.Alert.change, handler: { [weak alertController] _ in
guard let alertController else { return }
guard let textField = alertController.textFields?.first,
textField.text == selectedUsername else {
DDLogInfo("Username confirmation failed")
return
}
DDLogInfo("User changes username")
confirmed()
})
action.isEnabled = false

alertController.addTextField { [weak action, unowned alertController] textField in
textField.placeholder = Constants.Alert.confirm

let cancellable = NotificationCenter.default
.publisher(for: UITextField.textDidChangeNotification, object: textField)
.sink(receiveValue: { [weak action, weak textField] _ in
let enabled = textField?.text?.isEmpty == false && textField?.text == selectedUsername
action?.isEnabled = enabled
textField?.textColor = enabled ? .success : .text
})
// Bind the subscription with the lifetime of the alert. When the alert instance is released, `cancellable` will be
// released too which will automatically unregister the notification observer.
objc_setAssociatedObject(alertController, &UIAlertController.cancellableKey, cancellable, .OBJC_ASSOCIATION_RETAIN_NONATOMIC)
Comment on lines +185 to +187
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@crazytonyli @jkmassel I generally prefer having an extension on UIAlertController that encapsulates all the configuration logic for a customize instance.

However, in this particular instance, the approach comes with the need for this legit but unusual objc_setAssociatedObject call.

All in all, I think the original version from #21003 might be best because of it's straightforwardness.

I'll leave it up to @jkmassel to decide. Also, I think given this address an relatively unfrequent use case (changing the user name) we don't need to rush into merging either version. Or, if we really want to rest assured that the bug is fixed ASAP, we could merge either for 22.8, and then refine if necessary.

}

DDLogInfo("Prompting user for confirmation of change username")

return alertController
}

func addAttributeMessage(_ message: String, highlighted text: String) {
let paragraph = String(format: message, text)
let font = WPStyleGuide.fontForTextStyle(.footnote)
Expand Down