Skip to content

Commit

Permalink
refactor: Refactor FormTextField to not require a HoC (#70)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bryphe-coder committed Jan 25, 2022
1 parent 9cf4f7c commit bbd8b8f
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 66 deletions.
6 changes: 2 additions & 4 deletions site/components/Form/FormTextField.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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: {
Expand All @@ -26,7 +24,7 @@ namespace Helpers {
}),
})

return <FormTextField {...props} form={form} formFieldName="name" />
return <FormTextField<FormValues> {...props} form={form} formFieldName="name" />
}
}

Expand Down
112 changes: 53 additions & 59 deletions site/components/Form/FormTextField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
)
}
4 changes: 1 addition & 3 deletions site/components/SignIn/SignInForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -25,8 +25,6 @@ const validationSchema = Yup.object({
password: Yup.string(),
})

const FormTextField = formTextFieldFactory<BuiltInAuthFormValues>()

const useStyles = makeStyles((theme) => ({
loginBtnWrapper: {
marginTop: theme.spacing(6),
Expand Down

0 comments on commit bbd8b8f

Please sign in to comment.