From 17d0dc2b684bf50ce06b792ea7895902f888094c Mon Sep 17 00:00:00 2001 From: McKayla Washburn Date: Wed, 10 Jan 2024 21:37:15 +0000 Subject: [PATCH 1/3] feat: add a character counter for fields with length limits - refactors`getFormHelpers` to accept an options object - adds a `maxLength` option which will display a message and character counter for fields with length limits - set `maxLength` option for template description fields --- .../CreateTemplatePage/CreateTemplateForm.tsx | 48 +++++------ .../pages/CreateUserPage/CreateUserForm.tsx | 18 ++--- .../AppearanceSettingsPageView.tsx | 8 +- .../pages/GroupsPage/CreateGroupPageView.tsx | 7 +- .../GroupsPage/SettingsGroupPageView.tsx | 14 ++-- .../pages/TemplateSettingsPage/Sidebar.tsx | 7 +- .../TemplateSettingsForm.tsx | 8 +- .../TemplateScheduleForm.tsx | 79 ++++++++++--------- .../TemplateSettingsLayout.tsx | 14 +--- .../TemplateVariablesForm.tsx | 2 +- .../WorkspaceScheduleForm.tsx | 5 +- site/src/utils/formUtils.test.ts | 61 +++++++++++--- site/src/utils/formUtils.ts | 61 +++++++++++--- 13 files changed, 205 insertions(+), 127 deletions(-) diff --git a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx index 6c5bc538310d1..466d4baa1d6eb 100644 --- a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx +++ b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx @@ -337,7 +337,9 @@ export const CreateTemplateForm: FC = (props) => { /> = (props) => { , - )} + {...getFieldHelpers("default_ttl_hours", { + helperText: ( + + ), + })} disabled={isSubmitting} onChange={onChangeTrimmed(form)} fullWidth @@ -377,12 +380,13 @@ export const CreateTemplateForm: FC = (props) => { , - )} + {...getFieldHelpers("autostop_requirement_days_of_week", { + helperText: ( + + ), + })} disabled={ isSubmitting || form.values.use_max_ttl || @@ -408,13 +412,14 @@ export const CreateTemplateForm: FC = (props) => { , - )} + {...getFieldHelpers("autostop_requirement_weeks", { + helperText: ( + + ), + })} disabled={ isSubmitting || form.values.use_max_ttl || @@ -453,9 +458,8 @@ export const CreateTemplateForm: FC = (props) => { ) : ( <> @@ -463,7 +467,7 @@ export const CreateTemplateForm: FC = (props) => { Learn more. ), - )} + })} disabled={ isSubmitting || !form.values.use_max_ttl || diff --git a/site/src/pages/CreateUserPage/CreateUserForm.tsx b/site/src/pages/CreateUserPage/CreateUserForm.tsx index c5afa5ffe025a..5a355d031ddb6 100644 --- a/site/src/pages/CreateUserPage/CreateUserForm.tsx +++ b/site/src/pages/CreateUserPage/CreateUserForm.tsx @@ -132,10 +132,9 @@ export const CreateUserForm: FC< label={Language.emailLabel} /> = ({ label="Name" /> diff --git a/site/src/pages/GroupsPage/SettingsGroupPageView.tsx b/site/src/pages/GroupsPage/SettingsGroupPageView.tsx index 34ddab4841470..33410e87b8bc1 100644 --- a/site/src/pages/GroupsPage/SettingsGroupPageView.tsx +++ b/site/src/pages/GroupsPage/SettingsGroupPageView.tsx @@ -73,10 +73,9 @@ const UpdateGroupForm: FC = ({ ) : ( <> = ({ )} = ({ template }) => { - - + } title={template.display_name || template.name} linkTo={`/templates/${template.name}`} diff --git a/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsForm.tsx b/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsForm.tsx index 39f722f59bdb1..5845538f61ee7 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsForm.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsForm.tsx @@ -29,6 +29,8 @@ import { import { EnterpriseBadge } from "components/Badges/Badges"; const MAX_DESCRIPTION_CHAR_LIMIT = 128; +const MAX_DESCRIPTION_MESSAGE = + "Please enter a description that is no longer than 128 characters."; export const getValidationSchema = (): Yup.AnyObjectSchema => Yup.object({ @@ -36,7 +38,7 @@ export const getValidationSchema = (): Yup.AnyObjectSchema => display_name: templateDisplayNameValidator("Display name"), description: Yup.string().max( MAX_DESCRIPTION_CHAR_LIMIT, - "Please enter a description that is less than or equal to 128 characters.", + MAX_DESCRIPTION_MESSAGE, ), allow_user_cancel_workspace_jobs: Yup.boolean(), icon: iconValidator, @@ -119,7 +121,9 @@ export const TemplateSettingsForm: FC = ({ /> = ({ > , - )} + {...getFieldHelpers("default_ttl_ms", { + helperText: ( + + ), + })} disabled={isSubmitting} fullWidth inputProps={{ min: 0, step: 1 }} @@ -375,12 +376,13 @@ export const TemplateScheduleForm: FC = ({ > , - )} + {...getFieldHelpers("autostop_requirement_days_of_week", { + helperText: ( + + ), + })} disabled={isSubmitting || form.values.use_max_ttl} fullWidth select @@ -402,13 +404,14 @@ export const TemplateScheduleForm: FC = ({ , - )} + {...getFieldHelpers("autostop_requirement_weeks", { + helperText: ( + + ), + })} disabled={ isSubmitting || form.values.use_max_ttl || @@ -463,9 +466,8 @@ export const TemplateScheduleForm: FC = ({ ) : ( <> @@ -473,7 +475,7 @@ export const TemplateScheduleForm: FC = ({ Learn more. ), - )} + })} disabled={ isSubmitting || !form.values.use_max_ttl || @@ -581,10 +583,11 @@ export const TemplateScheduleForm: FC = ({ label="Enable Failure Cleanup" /> , - )} + {...getFieldHelpers("failure_ttl_ms", { + helperText: ( + + ), + })} disabled={isSubmitting || !form.values.failure_cleanup_enabled} fullWidth inputProps={{ min: 0, step: "any" }} @@ -610,12 +613,13 @@ export const TemplateScheduleForm: FC = ({ label="Enable Dormancy Threshold" /> , - )} + {...getFieldHelpers("time_til_dormant_ms", { + helperText: ( + + ), + })} disabled={ isSubmitting || !form.values.inactivity_cleanup_enabled } @@ -643,12 +647,13 @@ export const TemplateScheduleForm: FC = ({ label="Enable Dormancy Auto-Deletion" /> , - )} + {...getFieldHelpers("time_til_dormant_autodelete_ms", { + helperText: ( + + ), + })} disabled={ isSubmitting || !form.values.dormant_autodeletion_cleanup_enabled diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsLayout.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsLayout.tsx index 876539f6109b3..75e20440ed72d 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateSettingsLayout.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsLayout.tsx @@ -56,13 +56,7 @@ export const TemplateSettingsLayout: FC = () => { - + {templateQuery.isError || permissionsQuery.isError ? ( ) : ( @@ -74,11 +68,7 @@ export const TemplateSettingsLayout: FC = () => { > }> -
+
diff --git a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesForm.tsx b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesForm.tsx index 8746559169599..a455b3dae4618 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesForm.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesForm.tsx @@ -74,7 +74,7 @@ export const TemplateVariablesForm: FC = ({ if (templateVariable.sensitive) { fieldHelpers = getFieldHelpers( "user_variable_values[" + index + "].value", - , + { helperText: }, ); } else { fieldHelpers = getFieldHelpers( diff --git a/site/src/pages/WorkspaceSettingsPage/WorkspaceSchedulePage/WorkspaceScheduleForm.tsx b/site/src/pages/WorkspaceSettingsPage/WorkspaceSchedulePage/WorkspaceScheduleForm.tsx index 85b261521b9cc..fc72ffb3089e1 100644 --- a/site/src/pages/WorkspaceSettingsPage/WorkspaceSchedulePage/WorkspaceScheduleForm.tsx +++ b/site/src/pages/WorkspaceSettingsPage/WorkspaceSchedulePage/WorkspaceScheduleForm.tsx @@ -399,7 +399,10 @@ export const WorkspaceScheduleForm: FC< label={Language.stopSwitch} /> { + getFieldProps: (name: keyof TestType) => { return { name, onBlur: jest.fn(), onChange: jest.fn(), - value: "", + value: form.values[name] ?? "", }; }, } as unknown as FormikContextType; @@ -46,6 +64,15 @@ describe("form util functions", () => { const untouchedBadResult = getFieldHelpers("untouchedBadField"); const touchedGoodResult = getFieldHelpers("touchedGoodField"); const touchedBadResult = getFieldHelpers("touchedBadField"); + const maxLengthOk = getFieldHelpers("maxLengthOk", { + maxLength: 32, + }); + const maxLengthClose = getFieldHelpers("maxLengthClose", { + maxLength: 32, + }); + const maxLengthOver = getFieldHelpers("maxLengthOver", { + maxLength: 32, + }); it("populates the 'field props'", () => { expect(untouchedGoodResult.name).toEqual("untouchedGoodField"); expect(untouchedGoodResult.onBlur).toBeDefined(); @@ -56,17 +83,29 @@ describe("form util functions", () => { 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; + 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(untouchedGoodResult.helperText).toBeUndefined(); + expect(untouchedBadResult.helperText).toBeUndefined(); + expect(touchedGoodResult.helperText).toBeUndefined(); expect(touchedBadResult.helperText).toEqual("oops!"); }); + it("allows short entries", () => { + expect(maxLengthOk.error).toBe(false); + expect(maxLengthOk.helperText).toBeUndefined(); + }); + it("warns on entries close to the limit", () => { + expect(maxLengthClose.error).toBe(false); + expect(maxLengthClose.helperText).toBeDefined(); + }); + it("reports an error for entries that are too long", () => { + expect(maxLengthOver.error).toBe(true); + expect(maxLengthOver.helperText).toBeDefined(); + }); }); describe("with API errors", () => { it("shows an error if there is only an API error", () => { @@ -129,7 +168,7 @@ describe("form util functions", () => { }); it("allows a 32-letter name", () => { - const input = Array(32).fill("a").join(""); + const input = "a".repeat(33); const validate = () => nameSchema.validateSync(input); expect(validate).not.toThrow(); }); @@ -145,7 +184,7 @@ describe("form util functions", () => { }); it("disallows a 33-letter name", () => { - const input = Array(33).fill("a").join(""); + const input = "a".repeat(33); const validate = () => nameSchema.validateSync(input); expect(validate).toThrow(); }); diff --git a/site/src/utils/formUtils.ts b/site/src/utils/formUtils.ts index 1de43b825c14b..12eb5ea341f43 100644 --- a/site/src/utils/formUtils.ts +++ b/site/src/utils/formUtils.ts @@ -23,10 +23,25 @@ const Language = { }, }; +interface GetFormHelperOptions { + helperText?: ReactNode; + /** + * backendFieldName remaps the name in the form, for when it doesn't match the + * name used by the backend + */ + backendFieldName?: string; + /** + * maxLength is used for showing helper text on fields that have a limited length, + * which will let the user know how much space they have left, or how much they are + * over the limit. Zero and negative values will be ignored. + */ + maxLength?: number; +} + interface FormHelpers { name: string; - onBlur: FocusEventHandler; - onChange: ChangeEventHandler; + onBlur: FocusEventHandler; + onChange: ChangeEventHandler; id: string; value?: string | number; error: boolean; @@ -37,10 +52,14 @@ export const getFormHelpers = (form: FormikContextType, error?: unknown) => ( fieldName: keyof TFormValues | string, - helperText?: ReactNode, - // backendFieldName is used when the value in the form is named different from the backend - backendFieldName?: string, + options: GetFormHelperOptions = {}, ): FormHelpers => { + const { + backendFieldName, + helperText: defaultHelperText, + maxLength, + } = options; + let helperText = defaultHelperText; const apiValidationErrors = isApiValidationError(error) ? (mapApiErrorToFieldErrors( error.response.data, @@ -49,17 +68,39 @@ export const getFormHelpers = // Since the fieldName can be a path string like parameters[0].value we need to use getIn const touched = Boolean(getIn(form.touched, fieldName.toString())); const formError = getIn(form.errors, fieldName.toString()); - // Since the field in the form can be diff from the backend, we need to + // Since the field in the form can be different from the backend, we need to // check for both when getting the error const apiField = backendFieldName ?? fieldName; const apiError = apiValidationErrors?.[apiField.toString()]; - const errorToDisplay = apiError ?? formError; + + const fieldProps = form.getFieldProps(fieldName); + const value = fieldProps.value; + + let lengthError: ReactNode = null; + // Show a message if the input is approaching or over the maximum length. + if ( + maxLength && + maxLength > 0 && + typeof value === "string" && + value.length > maxLength - 30 + ) { + helperText = `This cannot be longer than ${maxLength} characters. (${value.length}/${maxLength})`; + // Show it as an error, rather than a hint + if (value.length > maxLength) { + lengthError = helperText; + } + } + + // API and regular validation errors should wait to be shown, but length errors should + // be more responsive. + const errorToDisplay = + (touched && apiError) || lengthError || (touched && formError); return { - ...form.getFieldProps(fieldName), + ...fieldProps, id: fieldName.toString(), - error: touched && Boolean(errorToDisplay), - helperText: touched ? errorToDisplay ?? helperText : helperText, + error: Boolean(errorToDisplay), + helperText: errorToDisplay || helperText, }; }; From 01fe4660b71ffed80a4946ccbc293aecbb09bea8 Mon Sep 17 00:00:00 2001 From: McKayla Washburn Date: Thu, 11 Jan 2024 16:53:32 +0000 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=A7=AA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../TemplateSettingsPage.test.tsx | 6 ++---- site/src/utils/formUtils.test.ts | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPage.test.tsx b/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPage.test.tsx index ee6153646584a..59a474870bc4c 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPage.test.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPage.test.tsx @@ -82,7 +82,7 @@ const fillAndSubmitForm = async ({ await userEvent.type(iconField, icon); const allowCancelJobsField = screen.getByRole("checkbox", { - name: "Allow users to cancel in-progress workspace jobs. Depending on your template, canceling builds may leave workspaces in an unhealthy state. This option isn't recommended for most use cases.", + name: /allow users to cancel in-progress workspace jobs/i, }); // checkbox is checked by default, so it must be clicked to get unchecked if (!allow_user_cancel_workspace_jobs) { @@ -123,8 +123,6 @@ describe("TemplateSettingsPage", () => { "Nam quis nulla. Integer malesuada. In in enim a arcu imperdiet malesuada. Sed vel lectus. Donec odio urna, tempus molestie, port a", }; const validate = () => getValidationSchema().validateSync(values); - expect(validate).toThrowError( - "Please enter a description that is less than or equal to 128 characters.", - ); + expect(validate).toThrowError(); }); }); diff --git a/site/src/utils/formUtils.test.ts b/site/src/utils/formUtils.test.ts index 2337c217799d4..2460512947fb9 100644 --- a/site/src/utils/formUtils.test.ts +++ b/site/src/utils/formUtils.test.ts @@ -168,7 +168,7 @@ describe("form util functions", () => { }); it("allows a 32-letter name", () => { - const input = "a".repeat(33); + const input = "a".repeat(32); const validate = () => nameSchema.validateSync(input); expect(validate).not.toThrow(); }); From 985d8d0474b8ea5b3d65b3ce496dd44b6935c858 Mon Sep 17 00:00:00 2001 From: McKayla Washburn Date: Thu, 11 Jan 2024 19:06:46 +0000 Subject: [PATCH 3/3] add story --- .../TemplateSettingsPageView.stories.tsx | 2 +- site/src/utils/formUtils.stories.tsx | 69 +++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 site/src/utils/formUtils.stories.tsx diff --git a/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPageView.stories.tsx b/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPageView.stories.tsx index 613b90154467a..d63dca49e26fa 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPageView.stories.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPageView.stories.tsx @@ -1,6 +1,6 @@ +import type { Meta, StoryObj } from "@storybook/react"; import { mockApiError, MockTemplate } from "testHelpers/entities"; import { TemplateSettingsPageView } from "./TemplateSettingsPageView"; -import type { Meta, StoryObj } from "@storybook/react"; const meta: Meta = { title: "pages/TemplateSettingsPage", diff --git a/site/src/utils/formUtils.stories.tsx b/site/src/utils/formUtils.stories.tsx new file mode 100644 index 0000000000000..f1cb43ce51797 --- /dev/null +++ b/site/src/utils/formUtils.stories.tsx @@ -0,0 +1,69 @@ +import type { Meta, StoryObj } from "@storybook/react"; +import { type FC } from "react"; +import TextField from "@mui/material/TextField"; +import { Form } from "components/Form/Form"; +import { getFormHelpers } from "./formUtils"; +import { useFormik } from "formik"; +import { action } from "@storybook/addon-actions"; + +interface ExampleFormProps { + value?: string; + maxLength?: number; +} + +const ExampleForm: FC = ({ value, maxLength }) => { + const form = useFormik({ + initialValues: { + value, + }, + onSubmit: action("submit"), + }); + + const getFieldHelpers = getFormHelpers(form, null); + + return ( +
+ + + ); +}; + +const meta: Meta = { + title: "utilities/getFormHelpers", + component: ExampleForm, +}; + +export default meta; +type Story = StoryObj; + +export const UnderMaxLength: Story = { + args: { + value: "a".repeat(98), + maxLength: 128, + }, +}; + +export const CloseToMaxLength: Story = { + args: { + value: "a".repeat(99), + maxLength: 128, + }, +}; + +export const AtMaxLength: Story = { + args: { + value: "a".repeat(128), + maxLength: 128, + }, +}; + +export const OverMaxLength: Story = { + args: { + value: "a".repeat(129), + maxLength: 128, + }, +};