From bbd8b8ffa877e92eed987e75745fe7a58dddb0a1 Mon Sep 17 00:00:00 2001 From: Bryan Date: Tue, 25 Jan 2022 14:00:19 -0800 Subject: [PATCH] refactor: Refactor FormTextField to not require a HoC (#70) Prompted from discussion here: https://github.com/coder/coder/pull/60/files#r792124373 Our current FormTextField implementation requires a [higher-order component](https://reactjs.org/docs/higher-order-components.html), which can be complicated to understand. This experiments with moving it to not require being a HoC. The only difference in usage is that sometimes, you need to provide the type like ` form={form} formFieldName="some-field-in-form" />` - but it doesn't require special construction. --- site/components/Form/FormTextField.test.tsx | 6 +- site/components/Form/FormTextField.tsx | 112 +++++++++----------- site/components/SignIn/SignInForm.tsx | 4 +- 3 files changed, 56 insertions(+), 66 deletions(-) diff --git a/site/components/Form/FormTextField.test.tsx b/site/components/Form/FormTextField.test.tsx index 87653463c6..3cb02cdcbb 100644 --- a/site/components/Form/FormTextField.test.tsx +++ b/site/components/Form/FormTextField.test.tsx @@ -2,7 +2,7 @@ import { act, fireEvent, render, screen } from "@testing-library/react" import { useFormik } from "formik" import React from "react" import * as yup from "yup" -import { formTextFieldFactory, FormTextFieldProps } from "./FormTextField" +import { FormTextField, FormTextFieldProps } from "./FormTextField" namespace Helpers { export interface FormValues { @@ -11,8 +11,6 @@ namespace Helpers { export const requiredValidationMsg = "required" - const FormTextField = formTextFieldFactory() - export const Component: React.FC, "form" | "formFieldName">> = (props) => { const form = useFormik({ initialValues: { @@ -26,7 +24,7 @@ namespace Helpers { }), }) - return + return {...props} form={form} formFieldName="name" /> } } diff --git a/site/components/Form/FormTextField.tsx b/site/components/Form/FormTextField.tsx index db76173846..34b0c083fe 100644 --- a/site/components/Form/FormTextField.tsx +++ b/site/components/Form/FormTextField.tsx @@ -104,67 +104,61 @@ export interface FormTextFieldProps * ) * } */ -export const formTextFieldFactory = (): React.FC> => { - const component: React.FC> = ({ - children, - disabled, - displayValueOverride, - eventTransform, - form, - formFieldName, - helperText, - isPassword = false, - InputProps, - onChange, - type, - variant = "outlined", - ...rest - }) => { - const isError = form.touched[formFieldName] && Boolean(form.errors[formFieldName]) +export const FormTextField = ({ + children, + disabled, + displayValueOverride, + eventTransform, + form, + formFieldName, + helperText, + isPassword = false, + InputProps, + onChange, + type, + variant = "outlined", + ...rest +}: FormTextFieldProps): React.ReactElement => { + const isError = form.touched[formFieldName] && Boolean(form.errors[formFieldName]) - // Conversion to a string primitive is necessary as formFieldName is an in - // indexable type such as a string, number or enum. - const fieldId = String(formFieldName) + // Conversion to a string primitive is necessary as formFieldName is an in + // indexable type such as a string, number or enum. + const fieldId = String(formFieldName) - const Component = isPassword ? PasswordField : TextField - const inputType = isPassword ? undefined : type + const Component = isPassword ? PasswordField : TextField + const inputType = isPassword ? undefined : type - return ( - { - if (typeof onChange !== "undefined") { - onChange(e) - } + return ( + { + if (typeof onChange !== "undefined") { + onChange(e) + } - const event = e - if (typeof eventTransform !== "undefined") { - // TODO(Grey): Asserting the type as a string here is not quite - // right in that when an input is of type="number", the value will - // be a number. Type asserting is better than conversion for this - // reason, but perhaps there's a better way to do this without any - // assertions. - event.target.value = eventTransform(e.target.value) as string - } - form.handleChange(event) - }} - type={inputType} - value={displayValueOverride || form.values[formFieldName]} - > - {children} - - ) - } - - // Required when using an anonymous factory function - component.displayName = "FormTextField" - return component + const event = e + if (typeof eventTransform !== "undefined") { + // TODO(Grey): Asserting the type as a string here is not quite + // right in that when an input is of type="number", the value will + // be a number. Type asserting is better than conversion for this + // reason, but perhaps there's a better way to do this without any + // assertions. + event.target.value = eventTransform(e.target.value) as string + } + form.handleChange(event) + }} + type={inputType} + value={displayValueOverride || form.values[formFieldName]} + > + {children} + + ) } diff --git a/site/components/SignIn/SignInForm.tsx b/site/components/SignIn/SignInForm.tsx index 5cb71baca2..b0fefdc45d 100644 --- a/site/components/SignIn/SignInForm.tsx +++ b/site/components/SignIn/SignInForm.tsx @@ -6,7 +6,7 @@ import { useSWRConfig } from "swr" import * as Yup from "yup" import { Welcome } from "./Welcome" -import { formTextFieldFactory } from "../Form" +import { FormTextField } from "../Form" import * as API from "./../../api" import { LoadingButton } from "./../Button" @@ -25,8 +25,6 @@ const validationSchema = Yup.object({ password: Yup.string(), }) -const FormTextField = formTextFieldFactory() - const useStyles = makeStyles((theme) => ({ loginBtnWrapper: { marginTop: theme.spacing(6),