From 6b70d11bc6c754ddd84bc65eaf72f4f2fbb32efa Mon Sep 17 00:00:00 2001 From: Jacob Shandling <61553566+jacobshandling@users.noreply.github.com> Date: Tue, 9 May 2023 10:12:29 -0700 Subject: [PATCH] UI: Login page bugs (#11520) ## Addresses #11338 - Validate emails on login page - Fix jumping error state for no email provided ("Email field must be completed") - Fix jumping error state for password field - Fix jumping error state for Forgot password > email field https://www.loom.com/share/92a238fcd2614d6e8d2655d571aa2757 # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Changes file added for user-visible changes in `changes/` - [x] Added/updated tests - [x] Manual QA for all new/changed functionality --------- Co-authored-by: Jacob Shandling --- changes/11338-login-page-bugs | 1 + .../ForgotPasswordForm.tests.jsx | 12 ++-- .../forms/ForgotPasswordForm/helpers.js | 6 +- .../forms/LoginForm/LoginForm.tests.jsx | 63 +++++++++++++++---- .../components/forms/LoginForm/validate.js | 3 + .../InputFieldWithIcon/InputFieldWithIcon.jsx | 18 +++--- .../fields/InputFieldWithIcon/_styles.scss | 2 - 7 files changed, 73 insertions(+), 32 deletions(-) create mode 100644 changes/11338-login-page-bugs diff --git a/changes/11338-login-page-bugs b/changes/11338-login-page-bugs new file mode 100644 index 0000000000..1fd3771209 --- /dev/null +++ b/changes/11338-login-page-bugs @@ -0,0 +1 @@ +* On the login and password reset pages, added email validation and fixed some minor styling bugs. diff --git a/frontend/components/forms/ForgotPasswordForm/ForgotPasswordForm.tests.jsx b/frontend/components/forms/ForgotPasswordForm/ForgotPasswordForm.tests.jsx index fb9760c90e..2d5596ea37 100644 --- a/frontend/components/forms/ForgotPasswordForm/ForgotPasswordForm.tests.jsx +++ b/frontend/components/forms/ForgotPasswordForm/ForgotPasswordForm.tests.jsx @@ -5,7 +5,7 @@ import { renderWithSetup } from "test/test-utils"; import ForgotPasswordForm from "./ForgotPasswordForm"; -const email = "hi@thegnar.co"; +const [validEmail, invalidEmail] = ["hi@thegnar.co", "invalid-email"]; describe("ForgotPasswordForm - component", () => { const handleSubmit = jest.fn(); @@ -33,7 +33,7 @@ describe("ForgotPasswordForm - component", () => { ).toBeInTheDocument(); }); - it("should test validation for email field", async () => { + it("correctly validates the email field", async () => { const { user } = renderWithSetup( ); @@ -43,10 +43,10 @@ describe("ForgotPasswordForm - component", () => { expect(emailError).toBeInTheDocument(); expect(handleSubmit).not.toHaveBeenCalled(); - await user.type(screen.getByPlaceholderText("Email"), "invalid-email"); + await user.type(screen.getByPlaceholderText("Email"), invalidEmail); await user.click(screen.getByRole("button", { name: "Get instructions" })); - emailError = screen.getByText("invalid-email is not a valid email"); + emailError = screen.getByText("Email must be a valid email address"); expect(emailError).toBeInTheDocument(); expect(handleSubmit).not.toHaveBeenCalled(); }); @@ -56,9 +56,9 @@ describe("ForgotPasswordForm - component", () => { ); - await user.type(screen.getByPlaceholderText("Email"), email); + await user.type(screen.getByPlaceholderText("Email"), validEmail); await user.click(screen.getByRole("button", { name: "Get instructions" })); - expect(handleSubmit).toHaveBeenCalledWith({ email }); + expect(handleSubmit).toHaveBeenCalledWith({ email: validEmail }); }); }); diff --git a/frontend/components/forms/ForgotPasswordForm/helpers.js b/frontend/components/forms/ForgotPasswordForm/helpers.js index d2396a9323..18c2ac8a37 100644 --- a/frontend/components/forms/ForgotPasswordForm/helpers.js +++ b/frontend/components/forms/ForgotPasswordForm/helpers.js @@ -6,12 +6,10 @@ const validate = (formData) => { const { email } = formData; const errors = {}; - if (!validEmail(email)) { - errors.email = `${email} is not a valid email`; - } - if (!validatePresence(email)) { errors.email = "Email field must be completed"; + } else if (!validEmail(email)) { + errors.email = "Email must be a valid email address"; } const valid = !size(errors); diff --git a/frontend/components/forms/LoginForm/LoginForm.tests.jsx b/frontend/components/forms/LoginForm/LoginForm.tests.jsx index 6f0a5163fd..4eaf24ecb6 100644 --- a/frontend/components/forms/LoginForm/LoginForm.tests.jsx +++ b/frontend/components/forms/LoginForm/LoginForm.tests.jsx @@ -5,6 +5,9 @@ import { renderWithSetup } from "test/test-utils"; import LoginForm from "./LoginForm"; +const [validEmail, invalidEmail] = ["hi@thegnar.co", "invalid-email"]; +const password = "p@ssw0rd"; + describe("LoginForm - component", () => { const settings = { sso_enabled: false }; const submitSpy = jest.fn(); @@ -35,7 +38,49 @@ describe("LoginForm - component", () => { expect(screen.getByPlaceholderText("Password")).toBeInTheDocument(); }); - it("it does not submit the form when the form fields have not been filled out", async () => { + it("rejects an empty or invalid email field without submitting", async () => { + const { user } = renderWithSetup( + + ); + + // enter a valid password + await user.type(screen.getByPlaceholderText("Password"), password); + + // try to log in + await user.click(screen.getByRole("button", { name: "Login" })); + expect( + screen.getByText("Email field must be completed") + ).toBeInTheDocument(); + expect(submitSpy).not.toHaveBeenCalled(); + + // enter an invalid email + await user.type(screen.getByPlaceholderText("Email"), invalidEmail); + + // try to log in again + await user.click(screen.getByRole("button", { name: "Login" })); + expect( + screen.getByText("Email must be a valid email address") + ).toBeInTheDocument(); + expect(submitSpy).not.toHaveBeenCalled(); + }); + + it("rejects an empty password field without submitting", async () => { + const { user } = renderWithSetup( + + ); + + await user.type(screen.getByRole("textbox", { name: "Email" }), validEmail); + + // try to log in without entering a password + await user.click(screen.getByRole("button", { name: "Login" })); + + expect( + screen.getByText("Password field must be completed") + ).toBeInTheDocument(); + expect(submitSpy).not.toHaveBeenCalled(); + }); + + it("does not submit the form when both fields are empty", async () => { const { user } = renderWithSetup( ); @@ -43,26 +88,20 @@ describe("LoginForm - component", () => { await user.click(screen.getByRole("button", { name: "Login" })); expect(submitSpy).not.toHaveBeenCalled(); - expect( - screen.getByText("Email field must be completed") - ).toBeInTheDocument(); }); - it("submits the form data when form is submitted", async () => { + it("submits the form data when valid form data is submitted", async () => { const { user } = renderWithSetup( ); - await user.type( - screen.getByRole("textbox", { name: "Email" }), - "my@email.com" - ); - await user.type(screen.getByPlaceholderText("Password"), "p@ssw0rd"); + await user.type(screen.getByRole("textbox", { name: "Email" }), validEmail); + await user.type(screen.getByPlaceholderText("Password"), password); await user.click(screen.getByRole("button", { name: "Login" })); expect(submitSpy).toHaveBeenCalledWith({ - email: "my@email.com", - password: "p@ssw0rd", + email: validEmail, + password, }); }); }); diff --git a/frontend/components/forms/LoginForm/validate.js b/frontend/components/forms/LoginForm/validate.js index 75eb384977..cfc529f07e 100644 --- a/frontend/components/forms/LoginForm/validate.js +++ b/frontend/components/forms/LoginForm/validate.js @@ -1,5 +1,6 @@ import { size } from "lodash"; import validatePresence from "components/forms/validators/validate_presence"; +import validateEmail from "components/forms/validators/valid_email"; const validate = (formData) => { const errors = {}; @@ -7,6 +8,8 @@ const validate = (formData) => { if (!validatePresence(email)) { errors.email = "Email field must be completed"; + } else if (!validateEmail(email)) { + errors.email = "Email must be a valid email address"; } if (!validatePresence(password)) { diff --git a/frontend/components/forms/fields/InputFieldWithIcon/InputFieldWithIcon.jsx b/frontend/components/forms/fields/InputFieldWithIcon/InputFieldWithIcon.jsx index b5b0fe8c43..db76030f6d 100644 --- a/frontend/components/forms/fields/InputFieldWithIcon/InputFieldWithIcon.jsx +++ b/frontend/components/forms/fields/InputFieldWithIcon/InputFieldWithIcon.jsx @@ -28,12 +28,12 @@ class InputFieldWithIcon extends InputField { }; renderHeading = () => { - const { error, placeholder, name, label, tooltip } = this.props; - const labelClasses = classnames(`${baseClass}__label`); + const { error, placeholder, name, tooltip } = this.props; + const label = this.props.label ?? placeholder; - if (error) { - return
{error}
; - } + const labelClasses = classnames(`${baseClass}__label`, { + [`${baseClass}__errors`]: !!error, + }); return ( ); diff --git a/frontend/components/forms/fields/InputFieldWithIcon/_styles.scss b/frontend/components/forms/fields/InputFieldWithIcon/_styles.scss index 749087831c..db4644224a 100644 --- a/frontend/components/forms/fields/InputFieldWithIcon/_styles.scss +++ b/frontend/components/forms/fields/InputFieldWithIcon/_styles.scss @@ -73,8 +73,6 @@ } &__errors { - font-size: $x-small; - font-weight: $bold; color: $core-vibrant-red; }