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 `<FormTextField<FormValues> form={form} formFieldName="some-field-in-form" />` - but it doesn't require special construction.
This commit is contained in:
Bryan 2022-01-25 14:00:19 -08:00 committed by GitHub
parent 9cf4f7c1ac
commit bbd8b8ffa8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 56 additions and 66 deletions

View File

@ -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<FormValues>()
export const Component: React.FC<Omit<FormTextFieldProps<FormValues>, "form" | "formFieldName">> = (props) => {
const form = useFormik<FormValues>({
initialValues: {
@ -26,7 +24,7 @@ namespace Helpers {
}),
})
return <FormTextField {...props} form={form} formFieldName="name" />
return <FormTextField<FormValues> {...props} form={form} formFieldName="name" />
}
}

View File

@ -104,67 +104,61 @@ export interface FormTextFieldProps<T>
* )
* }
*/
export const formTextFieldFactory = <T,>(): React.FC<FormTextFieldProps<T>> => {
const component: React.FC<FormTextFieldProps<T>> = ({
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 = <T,>({
children,
disabled,
displayValueOverride,
eventTransform,
form,
formFieldName,
helperText,
isPassword = false,
InputProps,
onChange,
type,
variant = "outlined",
...rest
}: FormTextFieldProps<T>): 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 (
<Component
{...rest}
variant={variant}
disabled={disabled || form.isSubmitting}
error={isError}
helperText={isError ? form.errors[formFieldName] : helperText}
id={fieldId}
InputProps={isPassword ? undefined : InputProps}
name={fieldId}
onBlur={form.handleBlur}
onChange={(e) => {
if (typeof onChange !== "undefined") {
onChange(e)
}
return (
<Component
{...rest}
variant={variant}
disabled={disabled || form.isSubmitting}
error={isError}
helperText={isError ? form.errors[formFieldName] : helperText}
id={fieldId}
InputProps={isPassword ? undefined : InputProps}
name={fieldId}
onBlur={form.handleBlur}
onChange={(e) => {
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}
</Component>
)
}
// 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}
</Component>
)
}

View File

@ -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<BuiltInAuthFormValues>()
const useStyles = makeStyles((theme) => ({
loginBtnWrapper: {
marginTop: theme.spacing(6),