refactor(site): SignInForm without wrapper component (#558)

presleyp · web-flow · commit 7e48df8cb5bf · 2022-03-30T13:08:37.000-04:00
* Remove wrapper from SignInForm

* Spruce up tests

* Add util for form props

* Add back trim

* Add unit tests

* Lint, type fixes

* Pascal case for language

* Arrow functions

* Target text in e2e
diff --git a/site/e2e/pom/SignInPage.ts b/site/e2e/pom/SignInPage.ts
@@ -7,8 +7,8 @@ export class SignInPage extends BasePom {
   }
 
   async submitBuiltInAuthentication(email: string, password: string): Promise<void> {
-    await this.page.fill("id=signin-form-inpt-email", email)
-    await this.page.fill("id=signin-form-inpt-password", password)
-    await this.page.click("id=signin-form-submit")
+    await this.page.fill("text=Email", email)
+    await this.page.fill("text=Password", password)
+    await this.page.click("text=Sign In")
   }
 }
diff --git a/site/package.json b/site/package.json
@@ -48,6 +48,7 @@
     "@storybook/addon-links": "6.4.19",
     "@storybook/react": "6.4.19",
     "@testing-library/react": "12.1.4",
+    "@testing-library/user-event": "^13.5.0",
     "@types/express": "4.17.13",
     "@types/jest": "27.4.1",
     "@types/node": "14.18.12",
diff --git a/site/src/components/Form/index.test.tsx b/site/src/components/Form/index.test.tsx
@@ -0,0 +1,74 @@
+import { FormikContextType } from "formik/dist/types"
+import { getFormHelpers, onChangeTrimmed } from "./index"
+
+interface TestType {
+  untouchedGoodField: string
+  untouchedBadField: string
+  touchedGoodField: string
+  touchedBadField: string
+}
+
+const mockHandleChange = jest.fn()
+
+const form = {
+  errors: {
+    untouchedGoodField: undefined,
+    untouchedBadField: "oops!",
+    touchedGoodField: undefined,
+    touchedBadField: "oops!",
+  },
+  touched: {
+    untouchedGoodField: false,
+    untouchedBadField: false,
+    touchedGoodField: true,
+    touchedBadField: true,
+  },
+  handleChange: mockHandleChange,
+  handleBlur: jest.fn(),
+  getFieldProps: (name: string) => {
+    return {
+      name,
+      onBlur: jest.fn(),
+      onChange: jest.fn(),
+      value: "",
+    }
+  },
+} as unknown as FormikContextType<TestType>
+
+describe("form util functions", () => {
+  describe("getFormHelpers", () => {
+    const untouchedGoodResult = getFormHelpers<TestType>(form, "untouchedGoodField")
+    const untouchedBadResult = getFormHelpers<TestType>(form, "untouchedBadField")
+    const touchedGoodResult = getFormHelpers<TestType>(form, "touchedGoodField")
+    const touchedBadResult = getFormHelpers<TestType>(form, "touchedBadField")
+    it("populates the 'field props'", () => {
+      expect(untouchedGoodResult.name).toEqual("untouchedGoodField")
+      expect(untouchedGoodResult.onBlur).toBeDefined()
+      expect(untouchedGoodResult.onChange).toBeDefined()
+      expect(untouchedGoodResult.value).toBeDefined()
+    })
+    it("sets the id to the name", () => {
+      expect(untouchedGoodResult.id).toEqual("untouchedGoodField")
+    })
+    it("sets error to true if touched and invalid", () => {
+      expect(untouchedGoodResult.error).toBeFalsy
+      expect(untouchedBadResult.error).toBeFalsy
+      expect(touchedGoodResult.error).toBeFalsy
+      expect(touchedBadResult.error).toBeTruthy
+    })
+    it("sets helperText to the error message if touched and invalid", () => {
+      expect(untouchedGoodResult.helperText).toBeUndefined
+      expect(untouchedBadResult.helperText).toBeUndefined
+      expect(touchedGoodResult.helperText).toBeUndefined
+      expect(touchedBadResult.helperText).toEqual("oops!")
+    })
+  })
+
+  describe("onChangeTrimmed", () => {
+    it("calls handleChange with trimmed value", () => {
+      const event = { target: { value: " hello " } } as React.ChangeEvent<HTMLInputElement>
+      onChangeTrimmed<TestType>(form)(event)
+      expect(mockHandleChange).toHaveBeenCalledWith({ target: { value: "hello" } })
+    })
+  })
+})
diff --git a/site/src/components/Form/index.ts b/site/src/components/Form/index.ts
@@ -1,5 +1,37 @@
+import { FormikContextType, getIn } from "formik"
+import { ChangeEvent, ChangeEventHandler, FocusEventHandler } from "react"
+
 export * from "./FormCloseButton"
 export * from "./FormSection"
 export * from "./FormDropdownField"
 export * from "./FormTextField"
 export * from "./FormTitle"
+
+interface FormHelpers {
+  name: string
+  onBlur: FocusEventHandler
+  onChange: ChangeEventHandler
+  id: string
+  value?: string | number
+  error: boolean
+  helperText?: string
+}
+
+export const getFormHelpers = <T>(form: FormikContextType<T>, name: string): FormHelpers => {
+  // getIn is a util function from Formik that gets at any depth of nesting, and is necessary for the types to work
+  const touched = getIn(form.touched, name)
+  const errors = getIn(form.errors, name)
+  return {
+    ...form.getFieldProps(name),
+    id: name,
+    error: touched && Boolean(errors),
+    helperText: touched && errors,
+  }
+}
+
+export const onChangeTrimmed =
+  <T>(form: FormikContextType<T>) =>
+  (event: ChangeEvent<HTMLInputElement>): void => {
+    event.target.value = event.target.value.trim()
+    form.handleChange(event)
+  }
diff --git a/site/src/components/SignIn/SignInForm.tsx b/site/src/components/SignIn/SignInForm.tsx
@@ -4,9 +4,10 @@ import React from "react"
 import * as Yup from "yup"
 
 import { Welcome } from "./Welcome"
-import { FormTextField } from "../Form"
 import FormHelperText from "@material-ui/core/FormHelperText"
 import { LoadingButton } from "./../Button"
+import TextField from "@material-ui/core/TextField"
+import { getFormHelpers, onChangeTrimmed } from "../Form"
 
 /**
  * BuiltInAuthFormValues describes a form using built-in (email/password)
@@ -18,8 +19,17 @@ interface BuiltInAuthFormValues {
   password: string
 }
 
+export const Language = {
+  emailLabel: "Email",
+  passwordLabel: "Password",
+  emailInvalid: "Please enter a valid email address.",
+  emailRequired: "Please enter an email address.",
+  authErrorMessage: "Incorrect email or password.",
+  signIn: "Sign In",
+}
+
 const validationSchema = Yup.object({
-  email: Yup.string().required("Email is required."),
+  email: Yup.string().trim().email(Language.emailInvalid).required(Language.emailRequired),
   password: Yup.string(),
 })
 
@@ -59,50 +69,30 @@ export const SignInForm: React.FC<SignInFormProps> = ({ isLoading, authErrorMess
     <>
       <Welcome />
       <form onSubmit={form.handleSubmit}>
-        <div>
-          <FormTextField
-            label="Email"
-            autoComplete="email"
-            autoFocus
-            className={styles.loginTextField}
-            eventTransform={(email: string) => email.trim()}
-            form={form}
-            formFieldName="email"
-            fullWidth
-            inputProps={{
-              id: "signin-form-inpt-email",
-            }}
-            variant="outlined"
-          />
-          <FormTextField
-            label="Password"
-            autoComplete="current-password"
-            className={styles.loginTextField}
-            form={form}
-            formFieldName="password"
-            fullWidth
-            inputProps={{
-              id: "signin-form-inpt-password",
-            }}
-            isPassword
-            variant="outlined"
-          />
-          {authErrorMessage && (
-            <FormHelperText data-testid="sign-in-error" error>
-              {authErrorMessage}
-            </FormHelperText>
-          )}
-        </div>
+        <TextField
+          {...getFormHelpers<BuiltInAuthFormValues>(form, "email")}
+          onChange={onChangeTrimmed(form)}
+          autoFocus
+          autoComplete="email"
+          className={styles.loginTextField}
+          fullWidth
+          label={Language.emailLabel}
+          variant="outlined"
+        />
+        <TextField
+          {...getFormHelpers<BuiltInAuthFormValues>(form, "password")}
+          autoComplete="current-password"
+          className={styles.loginTextField}
+          fullWidth
+          id="password"
+          label={Language.passwordLabel}
+          type="password"
+          variant="outlined"
+        />
+        {authErrorMessage && <FormHelperText error>{Language.authErrorMessage}</FormHelperText>}
         <div className={styles.submitBtn}>
-          <LoadingButton
-            color="primary"
-            loading={isLoading}
-            fullWidth
-            id="signin-form-submit"
-            type="submit"
-            variant="contained"
-          >
-            {isLoading ? "" : "Sign In"}
+          <LoadingButton color="primary" loading={isLoading} fullWidth type="submit" variant="contained">
+            {isLoading ? "" : Language.signIn}
           </LoadingButton>
         </div>
       </form>
diff --git a/site/src/pages/login.test.tsx b/site/src/pages/login.test.tsx
@@ -1,9 +1,11 @@
 import React from "react"
-import { act, fireEvent, screen } from "@testing-library/react"
+import { act, screen } from "@testing-library/react"
+import userEvent from "@testing-library/user-event"
 import { history, render } from "../test_helpers"
 import { SignInPage } from "./login"
 import { server } from "../test_helpers/server"
 import { rest } from "msw"
+import { Language } from "../components/SignIn/SignInForm"
 
 describe("SignInPage", () => {
   beforeEach(() => {
@@ -21,12 +23,12 @@ describe("SignInPage", () => {
     render(<SignInPage />)
 
     // Then
-    await screen.findByText("Sign In", { exact: false })
+    await screen.findByText(Language.signIn, { exact: false })
   })
 
   it("shows an error message if SignIn fails", async () => {
     // Given
-    const { container } = render(<SignInPage />)
+    render(<SignInPage />)
     // Make login fail
     server.use(
       rest.post("/api/v2/users/login", async (req, res, ctx) => {
@@ -35,17 +37,18 @@ describe("SignInPage", () => {
     )
 
     // When
-    // Set username / password
-    const [username, password] = container.querySelectorAll("input")
-    fireEvent.change(username, { target: { value: "test@coder.com" } })
-    fireEvent.change(password, { target: { value: "password" } })
+    // Set email / password
+    const email = screen.getByLabelText(Language.emailLabel)
+    const password = screen.getByLabelText(Language.passwordLabel)
+    userEvent.type(email, "test@coder.com")
+    userEvent.type(password, "password")
     // Click sign-in
-    const signInButton = await screen.findByText("Sign In")
+    const signInButton = await screen.findByText(Language.signIn)
     act(() => signInButton.click())
 
     // Then
     // Finding error by test id because it comes from the backend
-    const errorMessage = await screen.findByTestId("sign-in-error")
+    const errorMessage = await screen.findByText(Language.authErrorMessage)
     expect(errorMessage).toBeDefined()
     expect(history.location.pathname).toEqual("/login")
   })
diff --git a/site/yarn.lock b/site/yarn.lock
@@ -2731,6 +2731,13 @@
     "@testing-library/dom" "^8.0.0"
     "@types/react-dom" "*"
 
+"@testing-library/user-event@^13.5.0":
+  version "13.5.0"
+  resolved "https://registry.yarnpkg.com/@testing-library/user-event/-/user-event-13.5.0.tgz#69d77007f1e124d55314a2b73fd204b333b13295"
+  integrity sha512-5Kwtbo3Y/NowpkbRuSepbyMFkZmHgD+vPzYB/RJ4oxt5Gj/avFFBYjhw27cqSVPVw/3a67NK1PbiIr9k4Gwmdg==
+  dependencies:
+    "@babel/runtime" "^7.12.5"
+
 "@tootallnate/once@1":
   version "1.1.2"
   resolved "https://registry.yarnpkg.com/@tootallnate/once/-/once-1.1.2.tgz#ccb91445360179a04e7fe6aff78c00ffc1eeaf82"

Original file line number	Diff line number	Diff line change
`@@ -7,8 +7,8 @@ export class SignInPage extends BasePom {`
`7`	`7`	`}`
`8`	`8`
`9`	`9`	`async submitBuiltInAuthentication(email: string, password: string): Promise<void> {`
`10`		`- await this.page.fill("id=signin-form-inpt-email", email)`
`11`		`- await this.page.fill("id=signin-form-inpt-password", password)`
`12`		`- await this.page.click("id=signin-form-submit")`
	`10`	`+ await this.page.fill("text=Email", email)`
	`11`	`+ await this.page.fill("text=Password", password)`
	`12`	`+ await this.page.click("text=Sign In")`
`13`	`13`	`}`
`14`	`14`	`}`