Merge pull request #596 from wordpress-mobile/issue/wc-4034-fix-login-ui

Fix Site Address UI Bugs When Displaying In-Page Errors

Author: shiki

WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift

@@ -19,6 +19,15 @@ final class SiteAddressViewController: LoginViewController {
     private var errorMessage: String?
     private var shouldChangeVoiceOverFocus: Bool = false
 
+    /// A state variable that is `true` if network calls are currently happening and so the
+    /// view should be showing a loading indicator.
+    ///
+    /// This should only be modified within `configureViewLoading(_ loading:)`.
+    ///
+    /// This state is mainly used in `configureSubmitButton()` to determine whether the button
+    /// should show an activity indicator.
+    private var viewIsLoading: Bool = false
+
     // MARK: - Actions
     @IBAction func handleContinueButtonTapped(_ sender: NUXButton) {
         tracker.track(click: .submit)
@@ -36,15 +45,15 @@ final class SiteAddressViewController: LoginViewController {
         localizePrimaryButton()
         registerTableViewCells()
         loadRows()
-        configureSubmitButton(animating: false)
+        configureSubmitButton()
         configureForAccessibility()
     }
 
     override func viewWillAppear(_ animated: Bool) {
         super.viewWillAppear(animated)
 
         siteURLField?.text = loginFields.siteAddress
-        configureSubmitButton(animating: false)
+        configureSubmitButton()
     }
 
     override func viewDidAppear(_ animated: Bool) {
@@ -82,6 +91,14 @@ final class SiteAddressViewController: LoginViewController {
         return WordPressAuthenticator.shared.unifiedStyle?.statusBarStyle ?? WordPressAuthenticator.shared.style.statusBarStyle
     }
 
+    /// Configures the appearance and state of the submit button.
+    ///
+    /// Use this instead of the overridden `configureSubmitButton(animating:)` since this uses the
+    /// _current_ `viewIsLoading` state.
+    private func configureSubmitButton() {
+        configureSubmitButton(animating: viewIsLoading)
+    }
+
     /// Configures the appearance and state of the submit button.
     ///
     override func configureSubmitButton(animating: Bool) {
@@ -120,9 +137,11 @@ final class SiteAddressViewController: LoginViewController {
     /// - Parameter loading: True if the form should be configured to a "loading" state.
     ///
     override func configureViewLoading(_ loading: Bool) {
+        viewIsLoading = loading
+
         siteURLField?.isEnabled = !loading
 
-        configureSubmitButton(animating: loading)
+        configureSubmitButton()
         navigationItem.hidesBackButton = loading
     }
 
@@ -183,21 +202,6 @@ extension SiteAddressViewController: UITableViewDataSource {
     }
 }
 
-// MARK: - UITableViewDelegate conformance
-extension SiteAddressViewController: UITableViewDelegate {
-    /// After the site address textfield cell is done displaying, remove the textfield reference.
-    ///
-    func tableView(_ tableView: UITableView, didEndDisplaying cell: UITableViewCell, forRowAt indexPath: IndexPath) {
-        guard let row = rows[safe: indexPath.row] else {
-            return
-        }
-
-        if row == .siteAddress {
-            siteURLField = nil
-        }
-    }
-}
-
 // MARK: - Keyboard Notifications
 extension SiteAddressViewController: NUXKeyboardResponder {
     @objc func handleKeyboardWillShow(_ notification: Foundation.Notification) {
@@ -304,9 +308,10 @@ private extension SiteAddressViewController {
         // Save a reference to the first textField so it can becomeFirstResponder.
         siteURLField = cell.textField
         cell.textField.delegate = self
+        cell.textField.text = loginFields.siteAddress
         cell.onChangeSelectionHandler = { [weak self] textfield in
             self?.loginFields.siteAddress = textfield.nonNilTrimmedText()
-            self?.configureSubmitButton(animating: false)
+            self?.configureSubmitButton()
         }
 
         SigninEditingState.signinEditingStateActive = true
@@ -462,7 +467,6 @@ private extension SiteAddressViewController {
     }
 
     func fetchSiteInfo() {
-        print("🔴 SAVC > fetchSiteInfo")
         let baseSiteUrl = WordPressAuthenticator.baseSiteURL(string: loginFields.siteAddress)
         let service = WordPressComBlogService()
 
@@ -489,6 +493,26 @@ private extension SiteAddressViewController {
     }
 
     func presentNextControllerIfPossible(siteInfo: WordPressComSiteInfo?) {
+
+        // Ensure that we're using the verified URL before passing the `loginFields` to the next
+        // view controller.
+        //
+        // In some scenarios, the text field change callback in `configureTextField()` gets executed
+        // right after we validated and modified `loginFields.siteAddress` in `validateForm()`. And
+        // this causes the value of `loginFields.siteAddress` to be reset to what the user entered.
+        //
+        // Using the user-entered `loginFields.siteAddress` causes problems when we try to log
+        // the user in especially if they just use a domain. For example, validating their
+        // self-hosted site credentials fails because the
+        // `WordPressOrgXMLRPCValidator.guessXMLRPCURLForSite` expects a complete site URL.
+        //
+        // This routine fixes that problem. We'll use what we already validated from
+        // `fetchSiteInfo()`.
+        //
+        if let verifiedSiteAddress = siteInfo?.url {
+            loginFields.siteAddress = verifiedSiteAddress
+        }
+
         guard siteInfo?.isWPCom == false else {
             showGetStarted()
             return

WordPressAuthenticatorTests/Authenticator/WordPressAuthenticatorTests.swift

@@ -49,6 +49,12 @@ class WordPressAuthenticatorTests: XCTestCase {
         XCTAssert(url == punycode)
     }
 
+    func testBaseSiteURLKeepsHTTPSchemeForNonWPSites() {
+        let url = "http://selfhostedsite.com"
+        let correctedURL = WordPressAuthenticator.baseSiteURL(string: url)
+        XCTAssertEqual(correctedURL, url)
+    }
+
     // MARK: WordPressAuthenticator Notification Tests
     func testDispatchesSupportPushNotificationReceived() {
         let authenticator = WordpressAuthenticatorProvider.getWordpressAuthenticator()