UX: don't disable "create account" button & display error message for required fields. (#9643)

app/assets/javascripts/discourse/app/components/user-field.js

@@ -6,6 +6,16 @@ export default Component.extend({
   classNameBindings: [":user-field", "field.field_type", "customFieldClass"],
   layoutName: fmt("field.field_type", "components/user-fields/%@"),
 
+  didInsertElement() {
+    this._super(...arguments);
+
+    let element = this.element.querySelector(
+      ".user-field.dropdown .select-kit-header"
+    );
+    element = element || this.element.querySelector("input");
+    this.field.element = element;
+  },
+
   @discourseComputed
   noneLabel() {
     return "user_fields.none";

app/assets/javascripts/discourse/app/controllers/create-account.js

@@ -61,27 +61,9 @@ export default Controller.extend(
       this._createUserFields();
     },
 
-    @discourseComputed(
+    @discourseComputed("formSubmitted")
-      "passwordRequired",
-      "nameValidation.failed",
-      "emailValidation.failed",
-      "usernameValidation.failed",
-      "passwordValidation.failed",
-      "userFieldsValidation.failed",
-      "formSubmitted",
-      "inviteCode"
-    )
     submitDisabled() {
       if (this.formSubmitted) return true;
-      if (this.get("nameValidation.failed")) return true;
-      if (this.get("emailValidation.failed")) return true;
-      if (this.get("usernameValidation.failed") && this.usernameRequired)
-        return true;
-      if (this.get("passwordValidation.failed") && this.passwordRequired)
-        return true;
-      if (this.get("userFieldsValidation.failed")) return true;
-
-      if (this.requireInviteCode && !this.inviteCode) return true;
 
       return false;
     },
@@ -114,18 +96,26 @@ export default Controller.extend(
     // Check the email address
     @discourseComputed("accountEmail", "rejectedEmails.[]")
     emailValidation(email, rejectedEmails) {
+      const failedAttrs = {
+        failed: true,
+        element: document.querySelector("#new-account-email")
+      };
+
       // If blank, fail without a reason
       if (isEmpty(email)) {
-        return EmberObject.create({
+        return EmberObject.create(
-          failed: true
+          Object.assign(failedAttrs, {
-        });
+            message: I18n.t("user.email.required")
+          })
+        );
       }
 
       if (rejectedEmails.includes(email)) {
-        return EmberObject.create({
+        return EmberObject.create(
-          failed: true,
+          Object.assign(failedAttrs, {
             reason: I18n.t("user.email.invalid")
-        });
+          })
+        );
       }
 
       if (
@@ -149,10 +139,11 @@ export default Controller.extend(
         });
       }
 
-      return EmberObject.create({
+      return EmberObject.create(
-        failed: true,
+        Object.assign(failedAttrs, {
           reason: I18n.t("user.email.invalid")
-      });
+        })
+      );
     },
 
     @discourseComputed(
@@ -312,6 +303,34 @@ export default Controller.extend(
       },
 
       createAccount() {
+        this.clearFlash();
+
+        const validation = [
+          this.emailValidation,
+          this.usernameValidation,
+          this.nameValidation,
+          this.passwordValidation,
+          this.userFieldsValidation
+        ].find(v => v.failed);
+
+        if (validation) {
+          if (validation.message) {
+            this.flash(validation.message, "error");
+          }
+
+          const element = validation.element;
+          if (element.tagName === "DIV") {
+            if (element.scrollIntoView) {
+              element.scrollIntoView();
+            }
+            element.click();
+          } else {
+            element.focus();
+          }
+
+          return;
+        }
+
         if (new Date() - this._challengeDate > 1000 * this._challengeExpiry) {
           this.fetchConfirmationValue().then(() =>
             this.performAccountCreation()

app/assets/javascripts/discourse/app/mixins/name-validation.js

@@ -18,7 +18,11 @@ export default Mixin.create({
   @discourseComputed("accountName")
   nameValidation() {
     if (this.siteSettings.full_name_required && isEmpty(this.accountName)) {
-      return EmberObject.create({ failed: true });
+      return EmberObject.create({
+        failed: true,
+        message: I18n.t("user.name.required"),
+        element: document.querySelector("#new-account-name")
+      });
     }
 
     return EmberObject.create({ ok: true });

app/assets/javascripts/discourse/app/mixins/password-validation.js

@@ -43,44 +43,57 @@ export default Mixin.create({
     accountEmail,
     passwordMinLength
   ) {
+    const failedAttrs = {
+      failed: true,
+      element: document.querySelector("#new-account-password")
+    };
+
     if (!passwordRequired) {
       return EmberObject.create({ ok: true });
     }
 
     if (rejectedPasswords.includes(password)) {
-      return EmberObject.create({
+      return EmberObject.create(
-        failed: true,
+        Object.assign(failedAttrs, {
           reason:
             this.rejectedPasswordsMessages.get(password) ||
             I18n.t("user.password.common")
-      });
+        })
+      );
     }
 
     // If blank, fail without a reason
     if (isEmpty(password)) {
-      return EmberObject.create({ failed: true });
+      return EmberObject.create(
+        Object.assign(failedAttrs, {
+          message: I18n.t("user.password.required")
+        })
+      );
     }
 
     // If too short
     if (password.length < passwordMinLength) {
-      return EmberObject.create({
+      return EmberObject.create(
-        failed: true,
+        Object.assign(failedAttrs, {
           reason: I18n.t("user.password.too_short")
-      });
+        })
+      );
     }
 
     if (!isEmpty(accountUsername) && password === accountUsername) {
-      return EmberObject.create({
+      return EmberObject.create(
-        failed: true,
+        Object.assign(failedAttrs, {
           reason: I18n.t("user.password.same_as_username")
-      });
+        })
+      );
     }
 
     if (!isEmpty(accountEmail) && password === accountEmail) {
-      return EmberObject.create({
+      return EmberObject.create(
-        failed: true,
+        Object.assign(failedAttrs, {
           reason: I18n.t("user.password.same_as_email")
-      });
+        })
+      );
     }
 
     // Looks good!

app/assets/javascripts/discourse/app/mixins/user-fields-validation.js

@@ -27,12 +27,17 @@ export default Mixin.create({
       userFields = userFields.filterBy("field.required");
     }
     if (!isEmpty(userFields)) {
-      const anyEmpty = userFields.any(uf => {
+      const emptyUserField = userFields.find(uf => {
         const val = uf.get("value");
         return !val || isEmpty(val);
       });
-      if (anyEmpty) {
+      if (emptyUserField) {
-        return EmberObject.create({ failed: true });
+        const userField = emptyUserField.field;
+        return EmberObject.create({
+          failed: true,
+          message: I18n.t("user_fields.required", { name: userField.name }),
+          element: userField.element
+        });
       }
     }
     return EmberObject.create({ ok: true });

app/assets/javascripts/discourse/app/mixins/username-validation.js

@@ -31,6 +31,10 @@ export default Mixin.create({
 
   @discourseComputed("accountUsername")
   basicUsernameValidation(accountUsername) {
+    const failedAttrs = {
+      failed: true,
+      element: document.querySelector("#new-account-username")
+    };
     this.set("uniqueUsernameValidation", null);
 
     if (accountUsername && accountUsername === this.prefilledUsername) {
@@ -42,31 +46,38 @@ export default Mixin.create({
 
     // If blank, fail without a reason
     if (isEmpty(accountUsername)) {
-      return EmberObject.create({ failed: true });
+      return EmberObject.create(
+        Object.assign(failedAttrs, {
+          message: I18n.t("user.username.required")
+        })
+      );
     }
 
     // If too short
     if (accountUsername.length < this.siteSettings.min_username_length) {
-      return EmberObject.create({
+      return EmberObject.create(
-        failed: true,
+        Object.assign(failedAttrs, {
           reason: I18n.t("user.username.too_short")
-      });
+        })
+      );
     }
 
     // If too long
     if (accountUsername.length > this.maxUsernameLength) {
-      return EmberObject.create({
+      return EmberObject.create(
-        failed: true,
+        Object.assign(failedAttrs, {
           reason: I18n.t("user.username.too_long")
-      });
+        })
+      );
     }
 
     this.checkUsernameAvailability();
     // Let's check it out asynchronously
-    return EmberObject.create({
+    return EmberObject.create(
-      failed: true,
+      Object.assign(failedAttrs, {
         reason: I18n.t("user.username.checking")
-    });
+      })
+    );
   },
 
   shouldCheckUsernameAvailability() {
@@ -93,23 +104,30 @@ export default Mixin.create({
               })
             );
           } else {
+            const failedAttrs = {
+              failed: true,
+              element: document.querySelector("#new-account-username")
+            };
+
             if (result.suggestion) {
               return this.set(
                 "uniqueUsernameValidation",
-                EmberObject.create({
+                EmberObject.create(
-                  failed: true,
+                  Object.assign(failedAttrs, {
                     reason: I18n.t("user.username.not_available", result)
                   })
+                )
               );
             } else {
               return this.set(
                 "uniqueUsernameValidation",
-                EmberObject.create({
+                EmberObject.create(
-                  failed: true,
+                  Object.assign(failedAttrs, {
                     reason: result.errors
                       ? result.errors.join(" ")
                       : I18n.t("user.username.not_available_no_suggestion")
                   })
+                )
               );
             }
           }

app/assets/stylesheets/desktop/login.scss

@@ -39,6 +39,7 @@
 
   #modal-alert {
     max-width: 100%;
+    margin-bottom: 0;
     padding: 8px 16px;
   }
 

config/locales/client.en.yml

@@ -821,6 +821,7 @@ en:
 
     user_fields:
       none: "(select an option)"
+      required: 'Please enter a value for "%{name}"'
 
     user:
       said: "{{username}}:"
@@ -1115,6 +1116,7 @@ en:
         sso_override_instructions: "Email can be updated from SSO provider."
         instructions: "Never shown to the public."
         ok: "We will email you to confirm"
+        required: "Please enter an email address"
         invalid: "Please enter a valid email address"
         authenticated: "Your email has been authenticated by {{provider}}"
         frequency_immediately: "We'll email you immediately if you haven't read the thing we're emailing you about."
@@ -1137,6 +1139,7 @@ en:
         title: "Name"
         instructions: "your full name (optional)"
         instructions_required: "Your full name"
+        required: "Please enter a name"
         too_short: "Your name is too short"
         ok: "Your name looks good"
       username:
@@ -1150,6 +1153,7 @@ en:
         too_long: "Your username is too long"
         checking: "Checking username availability..."
         prefilled: "Email matches this registered username"
+        required: "Please enter a username"
 
       locale:
         title: "Interface language"
@@ -1308,6 +1312,7 @@ en:
         same_as_email: "Your password is the same as your email."
         ok: "Your password looks good."
         instructions: "at least %{count} characters"
+        required: "Please enter a password"
 
       summary:
         title: "Summary"

test/javascripts/acceptance/create-account-user-fields-test.js

@@ -31,10 +31,10 @@ QUnit.test("create account with user fields", async assert => {
 
   assert.ok(exists(".create-account"), "it shows the create account modal");
   assert.ok(exists(".user-field"), "it has at least one user field");
-  assert.ok(
+
-    exists(".modal-footer .btn-primary:disabled"),
+  await click(".modal-footer .btn-primary");
-    "create account is disabled at first"
+  assert.ok(exists("#modal-alert"), "it shows the required field alert");
-  );
+  assert.equal(find("#modal-alert").text(), "Please enter an email address");
 
   await fillIn("#new-account-name", "Dr. Good Tuna");
   await fillIn("#new-account-password", "cool password bro");
@@ -52,28 +52,13 @@ QUnit.test("create account with user fields", async assert => {
     exists("#account-email-validation.good"),
     "the email validation is good"
   );
-  assert.ok(
+
-    exists(".modal-footer .btn-primary:disabled"),
+  await click(".modal-footer .btn-primary");
-    "create account is still disabled due to lack of user fields"
+  assert.equal(find("#modal-alert")[0].style.display, "");
-  );
 
   await fillIn(".user-field input[type=text]:first", "Barky");
-
-  assert.ok(
-    exists(".modal-footer .btn-primary:disabled"),
-    "create account is disabled because field is not checked"
-  );
-
   await click(".user-field input[type=checkbox]");
 
-  assert.ok(
+  await click(".modal-footer .btn-primary");
-    !exists(".modal-footer .btn-primary:disabled"),
+  assert.equal(find("#modal-alert")[0].style.display, "none");
-    "create account is enabled because field is checked"
-  );
-
-  await click(".user-field input[type=checkbox]");
-  assert.ok(
-    exists(".modal-footer .btn-primary:disabled"),
-    "unchecking the checkbox disables the create account button"
-  );
 });

test/javascripts/acceptance/sign-in-test.js

@@ -132,10 +132,6 @@ QUnit.test("create account", async assert => {
   await click("header .sign-up-button");
 
   assert.ok(exists(".create-account"), "it shows the create account modal");
-  assert.ok(
-    exists(".modal-footer .btn-primary:disabled"),
-    "create account is disabled at first"
-  );
 
   await fillIn("#new-account-name", "Dr. Good Tuna");
   await fillIn("#new-account-password", "cool password bro");
@@ -151,20 +147,14 @@ QUnit.test("create account", async assert => {
     exists("#username-validation.bad"),
     "the username validation is bad"
   );
-  assert.ok(
+  await click(".modal-footer .btn-primary");
-    exists(".modal-footer .btn-primary:disabled"),
+  assert.ok(exists("#new-account-username:focus"), "username field is focused");
-    "create account is still disabled"
-  );
 
   await fillIn("#new-account-username", "goodtuna");
   assert.ok(
     exists("#username-validation.good"),
     "the username validation is good"
   );
-  assert.not(
-    exists(".modal-footer .btn-primary:disabled"),
-    "create account is enabled"
-  );
 
   await click(".modal-footer .btn-primary");
   assert.ok(