fix: UX issues in template settings form's default auto-stop field (#5330)

* Fix helper text

- handles 0 ttl
- uses helper text typography
- pluralizes
- still doesn't override error (once considered touched)

* Show user friendly field name in error text

* Format

* Override label through Yup instead

* Switch to i18n - wip

* Fix i18n by thunking schema

* Fix template settings tests

* Replace third arg to getFieldHelpers -is used after all

site/src/i18n/en/index.ts

@@ -8,6 +8,7 @@ import agent from "./agent.json"
 import buildPage from "./buildPage.json"
 import workspacesPage from "./workspacesPage.json"
 import usersPage from "./usersPage.json"
+import templateSettingsPage from "./templateSettingsPage.json"
 import templateVersionPage from "./templateVersionPage.json"
 import loginPage from "./loginPage.json"
 import workspaceChangeVersionPage from "./workspaceChangeVersionPage.json"
@@ -24,6 +25,7 @@ export const en = {
   buildPage,
   workspacesPage,
   usersPage,
+  templateSettingsPage,
   templateVersionPage,
   loginPage,
   workspaceChangeVersionPage,

site/src/i18n/en/templatePage.json

@@ -1,16 +1,4 @@
 {
   "deleteSuccess": "Template successfully deleted.",
-  "createdVersion": "created the version",
-  "templateSettings": {
-    "title": "Template settings",
-    "dangerZone": {
-      "dangerZoneHeader": "Danger Zone",
-      "deleteTemplateHeader": "Delete this template",
-      "deleteTemplateCaption": "Do you want to permanently delete this template?",
-      "deleteCta": "Delete Template"
-    }
-  },
-  "displayNameLabel": "Display name",
-  "allowUserCancelWorkspaceJobsLabel": "Allow users to cancel in-progress workspace jobs.",
-  "allowUserCancelWorkspaceJobsNotice": "Depending on your template, canceling builds may leave workspaces in an unhealthy state. This option isn't recommended for most use cases."
+  "createdVersion": "created the version"
 }

site/src/i18n/en/templateSettingsPage.json

@@ -0,0 +1,24 @@
+{
+  "title": "Template settings",
+  "nameLabel": "Name",
+  "displayNameLabel": "Display name",
+  "descriptionLabel": "Description",
+  "descriptionMaxError": "Please enter a description that is less than or equal to 128 characters.",
+  "defaultTtlLabel": "Auto-stop default",
+  "iconLabel": "Icon",
+  "formAriaLabel": "Template settings form",
+  "selectEmoji": "Select emoji",
+  "ttlMaxError": "Please enter a limit that is less than or equal to 168 hours (7 days).",
+  "ttlMinError": "Default time until auto-stop must not be less than 0.",
+  "ttlHelperText_zero": "Workspaces created from this template will run until stopped manually.",
+  "ttlHelperText_one": "Workspaces created from this template will default to stopping after {{count}} hour.",
+  "ttlHelperText_other": "Workspaces created from this template will default to stopping after {{count}} hours.",
+  "allowUserCancelWorkspaceJobsLabel": "Allow users to cancel in-progress workspace jobs.",
+  "allowUserCancelWorkspaceJobsNotice": "Depending on your template, canceling builds may leave workspaces in an unhealthy state. This option isn't recommended for most use cases.",
+  "dangerZone": {
+    "dangerZoneHeader": "Danger Zone",
+    "deleteTemplateHeader": "Delete this template",
+    "deleteTemplateCaption": "Do you want to permanently delete this template?",
+    "deleteCta": "Delete Template"
+  }
+}

site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx

@@ -24,43 +24,44 @@ import {
 import * as Yup from "yup"
 import i18next from "i18next"
 import { useTranslation } from "react-i18next"
+import { Maybe } from "components/Conditionals/Maybe"
 
-export const Language = {
-  nameLabel: "Name",
-  descriptionLabel: "Description",
-  defaultTtlLabel: "Auto-stop default",
-  iconLabel: "Icon",
-  formAriaLabel: "Template settings form",
-  selectEmoji: "Select emoji",
-  ttlMaxError:
-    "Please enter a limit that is less than or equal to 168 hours (7 days).",
-  descriptionMaxError:
-    "Please enter a description that is less than or equal to 128 characters.",
-  ttlHelperText: (ttl: number): string =>
-    `Workspaces created from this template will default to stopping after ${ttl} hours.`,
+const TTLHelperText = ({ ttl }: { ttl?: number }) => {
+  const { t } = useTranslation("templateSettingsPage")
+  const count = typeof ttl !== "number" ? 0 : ttl
+  return (
+    // no helper text if ttl is negative - error will show once field is considered touched
+    <Maybe condition={count >= 0}>
+      <span>{t("ttlHelperText", { count })}</span>
+    </Maybe>
+  )
 }
 
 const MAX_DESCRIPTION_CHAR_LIMIT = 128
 const MAX_TTL_DAYS = 7
 const MS_HOUR_CONVERSION = 3600000
 
-export const validationSchema = Yup.object({
-  name: nameValidator(Language.nameLabel),
-  display_name: templateDisplayNameValidator(
-    i18next.t("displayNameLabel", {
-      ns: "templatePage",
-    }),
-  ),
-  description: Yup.string().max(
-    MAX_DESCRIPTION_CHAR_LIMIT,
-    Language.descriptionMaxError,
-  ),
-  default_ttl_ms: Yup.number()
-    .integer()
-    .min(0)
-    .max(24 * MAX_TTL_DAYS /* 7 days in hours */, Language.ttlMaxError),
-  allow_user_cancel_workspace_jobs: Yup.boolean(),
-})
+export const getValidationSchema = (): Yup.AnyObjectSchema =>
+  Yup.object({
+    name: nameValidator(i18next.t("nameLabel", { ns: "templateSettingsPage" })),
+    display_name: templateDisplayNameValidator(
+      i18next.t("displayNameLabel", {
+        ns: "templateSettingsPage",
+      }),
+    ),
+    description: Yup.string().max(
+      MAX_DESCRIPTION_CHAR_LIMIT,
+      i18next.t("descriptionMaxError", { ns: "templateSettingsPage" }),
+    ),
+    default_ttl_ms: Yup.number()
+      .integer()
+      .min(0, i18next.t("ttlMinError", { ns: "templateSettingsPage" }))
+      .max(
+        24 * MAX_TTL_DAYS /* 7 days in hours */,
+        i18next.t("ttlMaxError", { ns: "templateSettingsPage" }),
+      ),
+    allow_user_cancel_workspace_jobs: Yup.boolean(),
+  })
 
 export interface TemplateSettingsForm {
   template: Template
@@ -81,6 +82,7 @@ export const TemplateSettingsForm: FC<TemplateSettingsForm> = ({
   initialTouched,
 }) => {
   const [isEmojiPickerOpen, setIsEmojiPickerOpen] = useState(false)
+  const validationSchema = getValidationSchema()
   const form: FormikContextType<UpdateTemplateMeta> =
     useFormik<UpdateTemplateMeta>({
       initialValues: {
@@ -110,10 +112,10 @@ export const TemplateSettingsForm: FC<TemplateSettingsForm> = ({
   const hasIcon = form.values.icon && form.values.icon !== ""
   const emojiButtonRef = useRef<HTMLButtonElement>(null)
 
-  const { t } = useTranslation("templatePage")
+  const { t } = useTranslation("templateSettingsPage")
 
   return (
-    <form onSubmit={form.handleSubmit} aria-label={Language.formAriaLabel}>
+    <form onSubmit={form.handleSubmit} aria-label={t("formAriaLabel")}>
       <Stack>
         <TextField
           {...getFieldHelpers("name")}
@@ -121,7 +123,7 @@ export const TemplateSettingsForm: FC<TemplateSettingsForm> = ({
           onChange={onChangeTrimmed(form)}
           autoFocus
           fullWidth
-          label={Language.nameLabel}
+          label={t("nameLabel")}
           variant="outlined"
         />
 
@@ -138,7 +140,7 @@ export const TemplateSettingsForm: FC<TemplateSettingsForm> = ({
           multiline
           disabled={isSubmitting}
           fullWidth
-          label={Language.descriptionLabel}
+          label={t("descriptionLabel")}
           variant="outlined"
           rows={2}
         />
@@ -148,7 +150,7 @@ export const TemplateSettingsForm: FC<TemplateSettingsForm> = ({
             {...getFieldHelpers("icon")}
             disabled={isSubmitting}
             fullWidth
-            label={Language.iconLabel}
+            label={t("iconLabel")}
             variant="outlined"
             InputProps={{
               endAdornment: hasIcon ? (
@@ -177,7 +179,7 @@ export const TemplateSettingsForm: FC<TemplateSettingsForm> = ({
               setIsEmojiPickerOpen((v) => !v)
             }}
           >
-            {Language.selectEmoji}
+            {t("selectEmoji")}
           </Button>
 
           <Popover
@@ -204,20 +206,17 @@ export const TemplateSettingsForm: FC<TemplateSettingsForm> = ({
         </div>
 
         <TextField
-          {...getFieldHelpers("default_ttl_ms")}
+          {...getFieldHelpers(
+            "default_ttl_ms",
+            <TTLHelperText ttl={form.values.default_ttl_ms} />,
+          )}
           disabled={isSubmitting}
           fullWidth
           inputProps={{ min: 0, step: 1 }}
-          label={Language.defaultTtlLabel}
+          label={t("defaultTtlLabel")}
           variant="outlined"
           type="number"
         />
-        {/* If a value for default_ttl_ms has been entered and
-        there are no validation errors for that field, display helper text.
-        We do not use the MUI helper-text prop because it overrides the validation error */}
-        {form.values.default_ttl_ms && !form.errors.default_ttl_ms && (
-          <span>{Language.ttlHelperText(form.values.default_ttl_ms)}</span>
-        )}
 
         <Box display="flex">
           <div>

site/src/pages/TemplateSettingsPage/TemplateSettingsPage.test.tsx

@@ -5,20 +5,20 @@ import { UpdateTemplateMeta } from "api/typesGenerated"
 import { Language as FooterFormLanguage } from "components/FormFooter/FormFooter"
 import { MockTemplate } from "../../testHelpers/entities"
 import { renderWithAuth } from "../../testHelpers/renderHelpers"
-import {
-  Language as FormLanguage,
-  validationSchema,
-} from "./TemplateSettingsForm"
+import { getValidationSchema } from "./TemplateSettingsForm"
 import { TemplateSettingsPage } from "./TemplateSettingsPage"
 import i18next from "i18next"
 
+const { t } = i18next
+
 const renderTemplateSettingsPage = async () => {
   const renderResult = renderWithAuth(<TemplateSettingsPage />, {
     route: `/templates/${MockTemplate.name}/settings`,
     path: `/templates/:templateId/settings`,
   })
   // Wait the form to be rendered
-  await screen.findAllByLabelText(FormLanguage.nameLabel)
+  const label = t("nameLabel", { ns: "templateSettingsPage" })
+  await screen.findAllByLabelText(label)
   return renderResult
 }
 
@@ -39,28 +39,29 @@ const fillAndSubmitForm = async ({
   icon,
   allow_user_cancel_workspace_jobs,
 }: Required<UpdateTemplateMeta>) => {
-  const nameField = await screen.findByLabelText(FormLanguage.nameLabel)
+  const label = t("nameLabel", { ns: "templateSettingsPage" })
+  const nameField = await screen.findByLabelText(label)
   await userEvent.clear(nameField)
   await userEvent.type(nameField, name)
 
-  const { t } = i18next
-  const displayNameLabel = t("displayNameLabel", { ns: "templatePage" })
+  const displayNameLabel = t("displayNameLabel", { ns: "templateSettingsPage" })
 
   const displayNameField = await screen.findByLabelText(displayNameLabel)
   await userEvent.clear(displayNameField)
   await userEvent.type(displayNameField, display_name)
 
-  const descriptionField = await screen.findByLabelText(
-    FormLanguage.descriptionLabel,
-  )
+  const descriptionLabel = t("descriptionLabel", { ns: "templateSettingsPage" })
+  const descriptionField = await screen.findByLabelText(descriptionLabel)
   await userEvent.clear(descriptionField)
   await userEvent.type(descriptionField, description)
 
-  const iconField = await screen.findByLabelText(FormLanguage.iconLabel)
+  const iconLabel = t("iconLabel", { ns: "templateSettingsPage" })
+  const iconField = await screen.findByLabelText(iconLabel)
   await userEvent.clear(iconField)
   await userEvent.type(iconField, icon)
 
-  const maxTtlField = await screen.findByLabelText(FormLanguage.defaultTtlLabel)
+  const defaultTtlLabel = t("defaultTtlLabel", { ns: "templateSettingsPage" })
+  const maxTtlField = await screen.findByLabelText(defaultTtlLabel)
   await userEvent.clear(maxTtlField)
   await userEvent.type(maxTtlField, default_ttl_ms.toString())
 
@@ -79,8 +80,8 @@ const fillAndSubmitForm = async ({
 describe("TemplateSettingsPage", () => {
   it("renders", async () => {
     const { t } = i18next
-    const pageTitle = t("templateSettings.title", {
-      ns: "templatePage",
+    const pageTitle = t("title", {
+      ns: "templateSettingsPage",
     })
     await renderTemplateSettingsPage()
     const element = await screen.findByText(pageTitle)
@@ -90,8 +91,8 @@ describe("TemplateSettingsPage", () => {
   it("allows an admin to delete a template", async () => {
     const { t } = i18next
     await renderTemplateSettingsPage()
-    const deleteCta = t("templateSettings.dangerZone.deleteCta", {
-      ns: "templatePage",
+    const deleteCta = t("dangerZone.deleteCta", {
+      ns: "templateSettingsPage",
     })
     const deleteButton = await screen.findByText(deleteCta)
     expect(deleteButton).toBeDefined()
@@ -137,7 +138,7 @@ describe("TemplateSettingsPage", () => {
       ...validFormValues,
       default_ttl_ms: 24 * 7,
     }
-    const validate = () => validationSchema.validateSync(values)
+    const validate = () => getValidationSchema().validateSync(values)
     expect(validate).not.toThrowError()
   })
 
@@ -146,7 +147,7 @@ describe("TemplateSettingsPage", () => {
       ...validFormValues,
       default_ttl_ms: 0,
     }
-    const validate = () => validationSchema.validateSync(values)
+    const validate = () => getValidationSchema().validateSync(values)
     expect(validate).not.toThrowError()
   })
 
@@ -155,8 +156,10 @@ describe("TemplateSettingsPage", () => {
       ...validFormValues,
       default_ttl_ms: 24 * 7 + 1,
     }
-    const validate = () => validationSchema.validateSync(values)
-    expect(validate).toThrowError(FormLanguage.ttlMaxError)
+    const validate = () => getValidationSchema().validateSync(values)
+    expect(validate).toThrowError(
+      t("ttlMaxError", { ns: "templateSettingsPage" }),
+    )
   })
 
   it("allows a description of 128 chars", () => {
@@ -165,7 +168,7 @@ describe("TemplateSettingsPage", () => {
       description:
         "Nam quis nulla. Integer malesuada. In in enim a arcu imperdiet malesuada. Sed vel lectus. Donec odio urna, tempus molestie, port",
     }
-    const validate = () => validationSchema.validateSync(values)
+    const validate = () => getValidationSchema().validateSync(values)
     expect(validate).not.toThrowError()
   })
 
@@ -175,7 +178,9 @@ describe("TemplateSettingsPage", () => {
       description:
         "Nam quis nulla. Integer malesuada. In in enim a arcu imperdiet malesuada. Sed vel lectus. Donec odio urna, tempus molestie, port a",
     }
-    const validate = () => validationSchema.validateSync(values)
-    expect(validate).toThrowError(FormLanguage.descriptionMaxError)
+    const validate = () => getValidationSchema().validateSync(values)
+    expect(validate).toThrowError(
+      t("descriptionMaxError", { ns: "templateSettingsPage" }),
+    )
   })
 })

site/src/pages/TemplateSettingsPage/TemplateSettingsPage.tsx

@@ -2,17 +2,15 @@ import { useMachine } from "@xstate/react"
 import { useOrganizationId } from "hooks/useOrganizationId"
 import { FC } from "react"
 import { Helmet } from "react-helmet-async"
+import { useTranslation } from "react-i18next"
 import { useNavigate, useParams } from "react-router-dom"
 import { pageTitle } from "util/page"
 import { templateSettingsMachine } from "xServices/templateSettings/templateSettingsXService"
 import { TemplateSettingsPageView } from "./TemplateSettingsPageView"
 
-const Language = {
-  title: "Template Settings",
-}
-
 export const TemplateSettingsPage: FC = () => {
   const { template: templateName } = useParams() as { template: string }
+  const { t } = useTranslation("templateSettingsPage")
   const navigate = useNavigate()
   const organizationId = useOrganizationId()
   const [state, send] = useMachine(templateSettingsMachine, {
@@ -34,7 +32,7 @@ export const TemplateSettingsPage: FC = () => {
   return (
     <>
       <Helmet>
-        <title>{pageTitle(Language.title)}</title>
+        <title>{pageTitle(t("title"))}</title>
       </Helmet>
       <TemplateSettingsPageView
         isSubmitting={state.hasTag("submitting")}

site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.tsx

@@ -47,14 +47,14 @@ export const TemplateSettingsPageView: FC<TemplateSettingsPageViewProps> = ({
 }) => {
   const classes = useStyles()
   const isLoading = !template && !errors.getTemplateError
-  const { t } = useTranslation("templatePage")
+  const { t } = useTranslation("templateSettingsPage")
 
   if (isDeleted) {
     return <Navigate to="/templates" />
   }
 
   return (
-    <FullPageForm title={t("templateSettings.title")} onCancel={onCancel}>
+    <FullPageForm title={t("title")} onCancel={onCancel}>
       {Boolean(errors.getTemplateError) && (
         <Stack className={classes.errorContainer}>
           <AlertBanner severity="error" error={errors.getTemplateError} />
@@ -78,24 +78,22 @@ export const TemplateSettingsPageView: FC<TemplateSettingsPageViewProps> = ({
           />
           <Stack className={classes.dangerContainer}>
             <div className={classes.dangerHeader}>
-              {t("templateSettings.dangerZone.dangerZoneHeader")}
+              {t("dangerZone.dangerZoneHeader")}
             </div>
 
             <Stack className={classes.dangerBorder}>
               <Stack spacing={0}>
                 <p className={classes.deleteTemplateHeader}>
-                  {t("templateSettings.dangerZone.deleteTemplateHeader")}
+                  {t("dangerZone.deleteTemplateHeader")}
                 </p>
-                <span>
-                  {t("templateSettings.dangerZone.deleteTemplateCaption")}
-                </span>
+                <span>{t("dangerZone.deleteTemplateCaption")}</span>
               </Stack>
               <Button
                 className={classes.deleteButton}
                 onClick={onDelete}
-                aria-label={t("templateSettings.dangerZone.deleteCta")}
+                aria-label={t("dangerZone.deleteCta")}
               >
-                {t("templateSettings.dangerZone.deleteCta")}
+                {t("dangerZone.deleteCta")}
               </Button>
             </Stack>
           </Stack>

site/src/util/formUtils.ts

@@ -14,7 +14,7 @@ import * as Yup from "yup"
 
 export const Language = {
   nameRequired: (name: string): string => {
-    return `Please enter a ${name.toLowerCase()}.`
+    return name ? `Please enter a ${name.toLowerCase()}.` : "Required"
   },
   nameInvalidChars: (name: string): string => {
     return `${name} must start with a-Z or 0-9 and can contain a-Z, 0-9 or -`
@@ -37,7 +37,6 @@ interface FormHelpers {
   helperText?: ReactNode
 }
 
-// backendErrorName can be used if the backend names a field differently than the frontend does
 export const getFormHelpers =
   <T>(form: FormikContextType<T>, error?: Error | unknown) =>
   (
@@ -54,6 +53,7 @@ export const getFormHelpers =
         `name must be type of string, instead received '${typeof name}'`,
       )
     }
+
     const apiErrorName = backendErrorName ?? name
 
     // getIn is a util function from Formik that gets at any depth of nesting