From 0c5e97774da2d6701ca8260dccf8157279b9dbe9 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Mon, 16 Sep 2024 12:27:30 -0700
Subject: [PATCH 01/10] Add dropdowns to router route form for destinations and
 targets

---
 app/api/hooks.ts                      |  2 +-
 app/forms/vpc-router-route-common.tsx | 64 +++++++++++++++++++++------
 app/forms/vpc-router-route-create.tsx | 19 ++++++--
 app/forms/vpc-router-route-edit.tsx   | 18 +++++---
 app/routes.tsx                        |  1 +
 5 files changed, 81 insertions(+), 23 deletions(-)

diff --git a/app/api/hooks.ts b/app/api/hooks.ts
index 70c70a7778..1d9a92d9ff 100644
--- a/app/api/hooks.ts
+++ b/app/api/hooks.ts
@@ -177,7 +177,7 @@ export const getUsePrefetchedApiQuery =
       `Expected query to be prefetched.
 Key: ${JSON.stringify(queryKey)}
 Ensure the following:
-• loader is running
+• loader is called in routes.tsx and is running
 • query matches in both the loader and the component
 • request isn't erroring-out server-side (check the Networking tab)
 • mock API endpoint is implemented in handlers.ts
diff --git a/app/forms/vpc-router-route-common.tsx b/app/forms/vpc-router-route-common.tsx
index 069ae60f03..016a12a143 100644
--- a/app/forms/vpc-router-route-common.tsx
+++ b/app/forms/vpc-router-route-common.tsx
@@ -8,16 +8,19 @@
 
 import type { UseFormReturn } from 'react-hook-form'
 
-import type {
-  RouteDestination,
-  RouterRouteCreate,
-  RouterRouteUpdate,
-  RouteTarget,
+import {
+  usePrefetchedApiQuery,
+  type RouteDestination,
+  type RouterRouteCreate,
+  type RouterRouteUpdate,
+  type RouteTarget,
 } from '~/api'
+import { ComboboxField } from '~/components/form/fields/ComboboxField'
 import { DescriptionField } from '~/components/form/fields/DescriptionField'
 import { ListboxField } from '~/components/form/fields/ListboxField'
 import { NameField } from '~/components/form/fields/NameField'
 import { TextField } from '~/components/form/fields/TextField'
+import { useVpcRouterSelector } from '~/hooks/use-params'
 import { Message } from '~/ui/lib/Message'
 
 export type RouteFormValues = RouterRouteCreate | Required<RouterRouteUpdate>
@@ -60,7 +63,18 @@ type RouteFormFieldsProps = {
   isDisabled?: boolean
 }
 export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
+  const routerSelector = useVpcRouterSelector()
+  const { project, vpc } = routerSelector
+  // usePrefetchedApiQuery items below are initially fetched in the loaders in the vpc-router-route-create and -edit forms
+  const {
+    data: { items: vpcSubnets },
+  } = usePrefetchedApiQuery('vpcSubnetList', { query: { project, vpc, limit: 1000 } })
+  const {
+    data: { items: instances },
+  } = usePrefetchedApiQuery('instanceList', { query: { project, limit: 1000 } })
+
   const { control } = form
+  const destinationType = form.watch('destination.type')
   const targetType = form.watch('target.type')
   return (
     <>
@@ -76,16 +90,30 @@ export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
         items={toItems(destTypes)}
         placeholder="Select a destination type"
         required
+        onChange={() => {
+          form.setValue('destination.value', '')
+        }}
         disabled={isDisabled}
       />
-      <TextField
-        name="destination.value"
-        label="Destination value"
-        control={control}
-        placeholder="Enter a destination value"
-        required
-        disabled={isDisabled}
-      />
+      {destinationType === 'subnet' ? (
+        <ComboboxField
+          name="destination.value"
+          label="Destination value"
+          control={control}
+          items={vpcSubnets.map(({ name }) => ({ value: name, label: name }))}
+          placeholder="Select a destination value"
+          required
+        />
+      ) : (
+        <TextField
+          name="destination.value"
+          label="Destination value"
+          control={control}
+          placeholder="Enter a destination value"
+          required
+          disabled={isDisabled}
+        />
+      )}
       <ListboxField
         name="target.type"
         label="Target type"
@@ -98,7 +126,15 @@ export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
         }}
         disabled={isDisabled}
       />
-      {targetType !== 'drop' && (
+      {targetType === 'drop' ? null : targetType === 'instance' ? (
+        <ComboboxField
+          name="target.value"
+          label="Target value"
+          control={control}
+          placeholder="Select a target value"
+          items={instances.map(({ name }) => ({ value: name, label: name }))}
+        />
+      ) : (
         <TextField
           name="target.value"
           label="Target value"
diff --git a/app/forms/vpc-router-route-create.tsx b/app/forms/vpc-router-route-create.tsx
index d8c0d07690..4ed2afe6c0 100644
--- a/app/forms/vpc-router-route-create.tsx
+++ b/app/forms/vpc-router-route-create.tsx
@@ -6,13 +6,13 @@
  * Copyright Oxide Computer Company
  */
 import { useForm } from 'react-hook-form'
-import { useNavigate } from 'react-router-dom'
+import { useNavigate, type LoaderFunctionArgs } from 'react-router-dom'
 
-import { useApiMutation, useApiQueryClient } from '@oxide/api'
+import { apiQueryClient, useApiMutation, useApiQueryClient } from '@oxide/api'
 
 import { SideModalForm } from '~/components/form/SideModalForm'
 import { RouteFormFields, type RouteFormValues } from '~/forms/vpc-router-route-common'
-import { useVpcRouterSelector } from '~/hooks/use-params'
+import { getVpcRouterSelector, useVpcRouterSelector } from '~/hooks/use-params'
 import { addToast } from '~/stores/toast'
 import { pb } from '~/util/path-builder'
 
@@ -23,6 +23,19 @@ const defaultValues: RouteFormValues = {
   target: { type: 'ip', value: '' },
 }
 
+CreateRouterRouteSideModalForm.loader = async ({ params }: LoaderFunctionArgs) => {
+  const { project, vpc } = getVpcRouterSelector(params)
+  await Promise.all([
+    apiQueryClient.prefetchQuery('vpcSubnetList', {
+      query: { project, vpc, limit: 1000 },
+    }),
+    apiQueryClient.prefetchQuery('instanceList', {
+      query: { project, limit: 1000 },
+    }),
+  ])
+  return null
+}
+
 export function CreateRouterRouteSideModalForm() {
   const queryClient = useApiQueryClient()
   const routerSelector = useVpcRouterSelector()
diff --git a/app/forms/vpc-router-route-edit.tsx b/app/forms/vpc-router-route-edit.tsx
index 0399a02d4c..46202dcd38 100644
--- a/app/forms/vpc-router-route-edit.tsx
+++ b/app/forms/vpc-router-route-edit.tsx
@@ -27,11 +27,19 @@ import { addToast } from '~/stores/toast'
 import { pb } from '~/util/path-builder'
 
 EditRouterRouteSideModalForm.loader = async ({ params }: LoaderFunctionArgs) => {
-  const { route, ...routerSelector } = getVpcRouterRouteSelector(params)
-  await apiQueryClient.prefetchQuery('vpcRouterRouteView', {
-    path: { route },
-    query: routerSelector,
-  })
+  const { project, vpc, router, route } = getVpcRouterRouteSelector(params)
+  await Promise.all([
+    apiQueryClient.prefetchQuery('vpcRouterRouteView', {
+      path: { route },
+      query: { project, vpc, router },
+    }),
+    apiQueryClient.prefetchQuery('vpcSubnetList', {
+      query: { project, vpc, limit: 1000 },
+    }),
+    apiQueryClient.prefetchQuery('instanceList', {
+      query: { project, limit: 1000 },
+    }),
+  ])
   return null
 }
 
diff --git a/app/routes.tsx b/app/routes.tsx
index 609a39f96c..86b159f5fa 100644
--- a/app/routes.tsx
+++ b/app/routes.tsx
@@ -424,6 +424,7 @@ export const routes = createRoutesFromElements(
           <Route
             path="routes-new"
             element={<CreateRouterRouteSideModalForm />}
+            loader={CreateRouterRouteSideModalForm.loader}
             handle={{ crumb: 'New Route' }}
           />
           <Route

From b166b8b69ad138d5f9aa149e193f1c0a56389111 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Mon, 16 Sep 2024 13:07:12 -0700
Subject: [PATCH 02/10] Update test to use combobox values

---
 test/e2e/vpcs.e2e.ts | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/test/e2e/vpcs.e2e.ts b/test/e2e/vpcs.e2e.ts
index d356ecbc32..331337132e 100644
--- a/test/e2e/vpcs.e2e.ts
+++ b/test/e2e/vpcs.e2e.ts
@@ -7,7 +7,7 @@
  */
 import { expect, test } from '@playwright/test'
 
-import { clickRowAction, expectRowVisible } from './utils'
+import { clickRowAction, expectRowVisible, selectOption } from './utils'
 
 test('can nav to VpcPage from /', async ({ page }) => {
   await page.goto('/')
@@ -248,13 +248,16 @@ test('can create, update, and delete Route', async ({ page }) => {
 
   // update the route by clicking the edit button
   await clickRowAction(page, 'new-route', 'Edit')
-  await page.getByRole('textbox', { name: 'Destination value' }).fill('0.0.0.1')
+  // change the destination type to VPC subnet: `mock-subnet`
+  await selectOption(page, 'Destination type', 'Subnet')
+  await selectOption(page, 'Destination value', 'mock-subnet')
+  await page.getByRole('textbox', { name: 'Target value' }).fill('0.0.0.1')
   await page.getByRole('button', { name: 'Update route' }).click()
   await expect(routeRows).toHaveCount(2)
   await expectRowVisible(table, {
     Name: 'new-route',
-    Destination: 'IP0.0.0.1',
-    Target: 'IP1.1.1.1',
+    Destination: 'VPC subnetmock-subnet',
+    Target: 'IP0.0.0.1',
   })
 
   // delete the route

From f069020bf672c0fb7ed091b3fd8d5c79f87980e2 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Mon, 16 Sep 2024 13:21:36 -0700
Subject: [PATCH 03/10] target value is required

---
 app/forms/vpc-router-route-common.tsx | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/forms/vpc-router-route-common.tsx b/app/forms/vpc-router-route-common.tsx
index 016a12a143..6dfa7cfa49 100644
--- a/app/forms/vpc-router-route-common.tsx
+++ b/app/forms/vpc-router-route-common.tsx
@@ -131,8 +131,9 @@ export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
           name="target.value"
           label="Target value"
           control={control}
-          placeholder="Select a target value"
           items={instances.map(({ name }) => ({ value: name, label: name }))}
+          placeholder="Select a target value"
+          required
         />
       ) : (
         <TextField

From 5e63b850fc50f949a79abcef28db7d1d311de2e7 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Mon, 16 Sep 2024 13:23:12 -0700
Subject: [PATCH 04/10] pass along isDisabled to new comboboxes

---
 app/forms/vpc-router-route-common.tsx | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/forms/vpc-router-route-common.tsx b/app/forms/vpc-router-route-common.tsx
index 6dfa7cfa49..b8bfbc9f10 100644
--- a/app/forms/vpc-router-route-common.tsx
+++ b/app/forms/vpc-router-route-common.tsx
@@ -103,6 +103,7 @@ export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
           items={vpcSubnets.map(({ name }) => ({ value: name, label: name }))}
           placeholder="Select a destination value"
           required
+          isDisabled={isDisabled}
         />
       ) : (
         <TextField
@@ -134,6 +135,7 @@ export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
           items={instances.map(({ name }) => ({ value: name, label: name }))}
           placeholder="Select a target value"
           required
+          isDisabled={isDisabled}
         />
       ) : (
         <TextField

From e78f8247810210e0ed208e5f4e8d746dd76fb281 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Mon, 16 Sep 2024 14:46:41 -0700
Subject: [PATCH 05/10] minor comment edit

---
 app/forms/vpc-router-route-common.tsx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/forms/vpc-router-route-common.tsx b/app/forms/vpc-router-route-common.tsx
index b8bfbc9f10..008040f40e 100644
--- a/app/forms/vpc-router-route-common.tsx
+++ b/app/forms/vpc-router-route-common.tsx
@@ -65,7 +65,7 @@ type RouteFormFieldsProps = {
 export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
   const routerSelector = useVpcRouterSelector()
   const { project, vpc } = routerSelector
-  // usePrefetchedApiQuery items below are initially fetched in the loaders in the vpc-router-route-create and -edit forms
+  // usePrefetchedApiQuery items below are initially fetched in the loaders in vpc-router-route-create and -edit
   const {
     data: { items: vpcSubnets },
   } = usePrefetchedApiQuery('vpcSubnetList', { query: { project, vpc, limit: 1000 } })

From 6f4a1bfba94e26d71d32d25529726eb32274dd01 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Fri, 20 Sep 2024 14:27:38 -0700
Subject: [PATCH 06/10] Simplest possible customization of placeholder text

---
 app/forms/vpc-router-route-common.tsx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/forms/vpc-router-route-common.tsx b/app/forms/vpc-router-route-common.tsx
index 008040f40e..13a335a70e 100644
--- a/app/forms/vpc-router-route-common.tsx
+++ b/app/forms/vpc-router-route-common.tsx
@@ -101,7 +101,7 @@ export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
           label="Destination value"
           control={control}
           items={vpcSubnets.map(({ name }) => ({ value: name, label: name }))}
-          placeholder="Select a destination value"
+          placeholder="Select a subnet"
           required
           isDisabled={isDisabled}
         />
@@ -133,7 +133,7 @@ export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
           label="Target value"
           control={control}
           items={instances.map(({ name }) => ({ value: name, label: name }))}
-          placeholder="Select a target value"
+          placeholder="Select an instance"
           required
           isDisabled={isDisabled}
         />

From 18ed16d2bde7a5e7c44c4c7bb95999744326a292 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Mon, 23 Sep 2024 10:42:41 -0700
Subject: [PATCH 07/10] dry up some form code

---
 app/forms/vpc-router-route-common.tsx | 32 +++++++++++++++------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/app/forms/vpc-router-route-common.tsx b/app/forms/vpc-router-route-common.tsx
index 13a335a70e..6ac1097c05 100644
--- a/app/forms/vpc-router-route-common.tsx
+++ b/app/forms/vpc-router-route-common.tsx
@@ -76,6 +76,22 @@ export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
   const { control } = form
   const destinationType = form.watch('destination.type')
   const targetType = form.watch('target.type')
+  const destinationValuePlaceholder = {
+    ip: 'Enter an IP',
+    ip_net: 'Enter an IP network',
+    subnet: 'Select a subnet',
+    // VPC probably won't ever be used, as VPCs cannot be specified
+    // as a destination in custom routers, but is here for completeness
+    vpc: 'Select a VPC',
+  }[destinationType]
+  const getDestinationValueProps = () => ({
+    name: 'destination.value' as const,
+    label: 'Destination value',
+    control,
+    placeholder: destinationValuePlaceholder,
+    required: true,
+    isDisabled,
+  })
   return (
     <>
       {isDisabled && (
@@ -97,23 +113,11 @@ export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
       />
       {destinationType === 'subnet' ? (
         <ComboboxField
-          name="destination.value"
-          label="Destination value"
-          control={control}
+          {...getDestinationValueProps()}
           items={vpcSubnets.map(({ name }) => ({ value: name, label: name }))}
-          placeholder="Select a subnet"
-          required
-          isDisabled={isDisabled}
         />
       ) : (
-        <TextField
-          name="destination.value"
-          label="Destination value"
-          control={control}
-          placeholder="Enter a destination value"
-          required
-          disabled={isDisabled}
-        />
+        <TextField {...getDestinationValueProps()} />
       )}
       <ListboxField
         name="target.type"

From 89edc126140cfe2e27708e0e5012167e651280dd Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Mon, 23 Sep 2024 13:17:32 -0700
Subject: [PATCH 08/10] Adding more human-friendly copy for clarity

---
 app/forms/vpc-router-route-common.tsx | 88 +++++++++++++++++----------
 1 file changed, 57 insertions(+), 31 deletions(-)

diff --git a/app/forms/vpc-router-route-common.tsx b/app/forms/vpc-router-route-common.tsx
index 6ac1097c05..e710762eb0 100644
--- a/app/forms/vpc-router-route-common.tsx
+++ b/app/forms/vpc-router-route-common.tsx
@@ -55,6 +55,42 @@ const targetTypes: Record<Exclude<RouteTarget['type'], 'subnet' | 'vpc'>, string
   drop: 'Drop',
 }
 
+const getDestinationValuePlaceholder = (destinationType: RouteDestination['type']) =>
+  ({
+    ip: 'Enter an IP',
+    ip_net: 'Enter an IP network',
+    subnet: 'Select a subnet',
+    vpc: undefined,
+  })[destinationType]
+
+const getDestinationValueDescription = (destinationType: RouteDestination['type']) =>
+  ({
+    ip: 'An IP address, like 192.168.1.222',
+    ip_net: 'An IP subnet, like 192.168.0.0/16',
+    subnet: undefined,
+    vpc: undefined,
+  })[destinationType]
+
+const getTargetValuePlaceholder = (destinationType: RouteTarget['type']) =>
+  ({
+    ip: 'Enter an IP',
+    instance: 'Select an instance',
+    internet_gateway: undefined,
+    drop: undefined,
+    subnet: undefined,
+    vpc: undefined,
+  })[destinationType]
+
+const getTargetValueDescription = (targetType: RouteTarget['type']) =>
+  ({
+    ip: 'An IP address, like 10.0.1.5',
+    instance: undefined,
+    internet_gateway: routeFormMessage.internetGatewayTargetValue,
+    drop: undefined,
+    subnet: undefined,
+    vpc: undefined,
+  })[targetType]
+
 const toItems = (mapping: Record<string, string>) =>
   Object.entries(mapping).map(([value, label]) => ({ value, label }))
 
@@ -76,22 +112,26 @@ export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
   const { control } = form
   const destinationType = form.watch('destination.type')
   const targetType = form.watch('target.type')
-  const destinationValuePlaceholder = {
-    ip: 'Enter an IP',
-    ip_net: 'Enter an IP network',
-    subnet: 'Select a subnet',
-    // VPC probably won't ever be used, as VPCs cannot be specified
-    // as a destination in custom routers, but is here for completeness
-    vpc: 'Select a VPC',
-  }[destinationType]
-  const getDestinationValueProps = () => ({
+  const destinationValueProps = {
     name: 'destination.value' as const,
     label: 'Destination value',
     control,
-    placeholder: destinationValuePlaceholder,
+    placeholder: getDestinationValuePlaceholder(destinationType),
     required: true,
-    isDisabled,
-  })
+    disabled: isDisabled,
+    description: getDestinationValueDescription(destinationType),
+  }
+  const targetValueProps = {
+    name: 'target.value' as const,
+    label: 'Target value',
+    control,
+    // possible targetTypes needing placeholders are instances or IPs (internet_gateway has no placeholder)
+    placeholder: getTargetValuePlaceholder(targetType),
+    required: true,
+    // 'internet_gateway' targetTypes can only have the value 'outbound', so we disable the field
+    disabled: isDisabled || targetType === 'internet_gateway',
+    description: getTargetValueDescription(targetType),
+  }
   return (
     <>
       {isDisabled && (
@@ -113,11 +153,12 @@ export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
       />
       {destinationType === 'subnet' ? (
         <ComboboxField
-          {...getDestinationValueProps()}
+          {...destinationValueProps}
           items={vpcSubnets.map(({ name }) => ({ value: name, label: name }))}
+          isDisabled={isDisabled}
         />
       ) : (
-        <TextField {...getDestinationValueProps()} />
+        <TextField {...destinationValueProps} />
       )}
       <ListboxField
         name="target.type"
@@ -133,27 +174,12 @@ export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
       />
       {targetType === 'drop' ? null : targetType === 'instance' ? (
         <ComboboxField
-          name="target.value"
-          label="Target value"
-          control={control}
+          {...targetValueProps}
           items={instances.map(({ name }) => ({ value: name, label: name }))}
-          placeholder="Select an instance"
-          required
           isDisabled={isDisabled}
         />
       ) : (
-        <TextField
-          name="target.value"
-          label="Target value"
-          control={control}
-          placeholder="Enter a target value"
-          required
-          // 'internet_gateway' targetTypes can only have the value 'outbound', so we disable the field
-          disabled={isDisabled || targetType === 'internet_gateway'}
-          description={
-            targetType === 'internet_gateway' && routeFormMessage.internetGatewayTargetValue
-          }
-        />
+        <TextField {...targetValueProps} />
       )}
     </>
   )

From c4821bf9c0e493ea0041511bd521f8e09331f281 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Mon, 23 Sep 2024 14:59:08 -0700
Subject: [PATCH 09/10] update props based on new Combobox disabled prop

---
 app/forms/vpc-router-route-common.tsx | 20 +++++++++-----------
 app/forms/vpc-router-route-edit.tsx   |  6 +++---
 2 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/app/forms/vpc-router-route-common.tsx b/app/forms/vpc-router-route-common.tsx
index e710762eb0..31571de4ae 100644
--- a/app/forms/vpc-router-route-common.tsx
+++ b/app/forms/vpc-router-route-common.tsx
@@ -96,9 +96,9 @@ const toItems = (mapping: Record<string, string>) =>
 
 type RouteFormFieldsProps = {
   form: UseFormReturn<RouteFormValues>
-  isDisabled?: boolean
+  disabled?: boolean
 }
-export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
+export const RouteFormFields = ({ form, disabled }: RouteFormFieldsProps) => {
   const routerSelector = useVpcRouterSelector()
   const { project, vpc } = routerSelector
   // usePrefetchedApiQuery items below are initially fetched in the loaders in vpc-router-route-create and -edit
@@ -118,7 +118,7 @@ export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
     control,
     placeholder: getDestinationValuePlaceholder(destinationType),
     required: true,
-    disabled: isDisabled,
+    disabled,
     description: getDestinationValueDescription(destinationType),
   }
   const targetValueProps = {
@@ -129,16 +129,16 @@ export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
     placeholder: getTargetValuePlaceholder(targetType),
     required: true,
     // 'internet_gateway' targetTypes can only have the value 'outbound', so we disable the field
-    disabled: isDisabled || targetType === 'internet_gateway',
+    disabled: disabled || targetType === 'internet_gateway',
     description: getTargetValueDescription(targetType),
   }
   return (
     <>
-      {isDisabled && (
+      {disabled && (
         <Message variant="info" content={routeFormMessage.vpcSubnetNotModifiable} />
       )}
-      <NameField name="name" control={control} disabled={isDisabled} />
-      <DescriptionField name="description" control={control} disabled={isDisabled} />
+      <NameField name="name" control={control} disabled={disabled} />
+      <DescriptionField name="description" control={control} disabled={disabled} />
       <ListboxField
         name="destination.type"
         label="Destination type"
@@ -149,13 +149,12 @@ export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
         onChange={() => {
           form.setValue('destination.value', '')
         }}
-        disabled={isDisabled}
+        disabled={disabled}
       />
       {destinationType === 'subnet' ? (
         <ComboboxField
           {...destinationValueProps}
           items={vpcSubnets.map(({ name }) => ({ value: name, label: name }))}
-          isDisabled={isDisabled}
         />
       ) : (
         <TextField {...destinationValueProps} />
@@ -170,13 +169,12 @@ export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
         onChange={(value) => {
           form.setValue('target.value', value === 'internet_gateway' ? 'outbound' : '')
         }}
-        disabled={isDisabled}
+        disabled={disabled}
       />
       {targetType === 'drop' ? null : targetType === 'instance' ? (
         <ComboboxField
           {...targetValueProps}
           items={instances.map(({ name }) => ({ value: name, label: name }))}
-          isDisabled={isDisabled}
         />
       ) : (
         <TextField {...targetValueProps} />
diff --git a/app/forms/vpc-router-route-edit.tsx b/app/forms/vpc-router-route-edit.tsx
index 46202dcd38..19ac9934e2 100644
--- a/app/forms/vpc-router-route-edit.tsx
+++ b/app/forms/vpc-router-route-edit.tsx
@@ -59,7 +59,7 @@ export function EditRouterRouteSideModalForm() {
     'destination',
   ])
   const form = useForm({ defaultValues })
-  const isDisabled = route?.kind === 'vpc_subnet'
+  const disabled = route?.kind === 'vpc_subnet'
 
   const updateRouterRoute = useApiMutation('vpcRouterRouteUpdate', {
     onSuccess() {
@@ -90,9 +90,9 @@ export function EditRouterRouteSideModalForm() {
       }
       loading={updateRouterRoute.isPending}
       submitError={updateRouterRoute.error}
-      submitDisabled={isDisabled ? routeFormMessage.vpcSubnetNotModifiable : undefined}
+      submitDisabled={disabled ? routeFormMessage.vpcSubnetNotModifiable : undefined}
     >
-      <RouteFormFields form={form} isDisabled={isDisabled} />
+      <RouteFormFields form={form} disabled={disabled} />
     </SideModalForm>
   )
 }

From 409830b9922e014e31d7033ae5a441ec56aaedcd Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Mon, 23 Sep 2024 16:13:07 -0700
Subject: [PATCH 10/10] Use records for placeholders and descriptions

---
 app/forms/vpc-router-route-common.tsx | 91 +++++++++++++--------------
 1 file changed, 43 insertions(+), 48 deletions(-)

diff --git a/app/forms/vpc-router-route-common.tsx b/app/forms/vpc-router-route-common.tsx
index 31571de4ae..6207d8b305 100644
--- a/app/forms/vpc-router-route-common.tsx
+++ b/app/forms/vpc-router-route-common.tsx
@@ -10,10 +10,12 @@ import type { UseFormReturn } from 'react-hook-form'
 
 import {
   usePrefetchedApiQuery,
+  type Instance,
   type RouteDestination,
   type RouterRouteCreate,
   type RouterRouteUpdate,
   type RouteTarget,
+  type VpcSubnet,
 } from '~/api'
 import { ComboboxField } from '~/components/form/fields/ComboboxField'
 import { DescriptionField } from '~/components/form/fields/DescriptionField'
@@ -55,45 +57,45 @@ const targetTypes: Record<Exclude<RouteTarget['type'], 'subnet' | 'vpc'>, string
   drop: 'Drop',
 }
 
-const getDestinationValuePlaceholder = (destinationType: RouteDestination['type']) =>
-  ({
-    ip: 'Enter an IP',
-    ip_net: 'Enter an IP network',
-    subnet: 'Select a subnet',
-    vpc: undefined,
-  })[destinationType]
+const destinationValuePlaceholder: Record<RouteDestination['type'], string | undefined> = {
+  ip: 'Enter an IP',
+  ip_net: 'Enter an IP network',
+  subnet: 'Select a subnet',
+  vpc: undefined,
+}
 
-const getDestinationValueDescription = (destinationType: RouteDestination['type']) =>
-  ({
-    ip: 'An IP address, like 192.168.1.222',
-    ip_net: 'An IP subnet, like 192.168.0.0/16',
-    subnet: undefined,
-    vpc: undefined,
-  })[destinationType]
+const destinationValueDescription: Record<RouteDestination['type'], string | undefined> = {
+  ip: 'An IP address, like 192.168.1.222',
+  ip_net: 'An IP network, like 192.168.0.0/16',
+  subnet: undefined,
+  vpc: undefined,
+}
 
-const getTargetValuePlaceholder = (destinationType: RouteTarget['type']) =>
-  ({
-    ip: 'Enter an IP',
-    instance: 'Select an instance',
-    internet_gateway: undefined,
-    drop: undefined,
-    subnet: undefined,
-    vpc: undefined,
-  })[destinationType]
+/** possible targetTypes needing placeholders are instances or IPs (internet_gateway has no placeholder) */
+const targetValuePlaceholder: Record<RouteTarget['type'], string | undefined> = {
+  ip: 'Enter an IP',
+  instance: 'Select an instance',
+  internet_gateway: undefined,
+  drop: undefined,
+  subnet: undefined,
+  vpc: undefined,
+}
 
-const getTargetValueDescription = (targetType: RouteTarget['type']) =>
-  ({
-    ip: 'An IP address, like 10.0.1.5',
-    instance: undefined,
-    internet_gateway: routeFormMessage.internetGatewayTargetValue,
-    drop: undefined,
-    subnet: undefined,
-    vpc: undefined,
-  })[targetType]
+const targetValueDescription: Record<RouteTarget['type'], string | undefined> = {
+  ip: 'An IP address, like 10.0.1.5',
+  instance: undefined,
+  internet_gateway: routeFormMessage.internetGatewayTargetValue,
+  drop: undefined,
+  subnet: undefined,
+  vpc: undefined,
+}
 
-const toItems = (mapping: Record<string, string>) =>
+const toListboxItems = (mapping: Record<string, string>) =>
   Object.entries(mapping).map(([value, label]) => ({ value, label }))
 
+const toComboboxItems = (items: Array<Instance | VpcSubnet>) =>
+  items.map(({ name }) => ({ value: name, label: name }))
+
 type RouteFormFieldsProps = {
   form: UseFormReturn<RouteFormValues>
   disabled?: boolean
@@ -116,21 +118,20 @@ export const RouteFormFields = ({ form, disabled }: RouteFormFieldsProps) => {
     name: 'destination.value' as const,
     label: 'Destination value',
     control,
-    placeholder: getDestinationValuePlaceholder(destinationType),
+    placeholder: destinationValuePlaceholder[destinationType],
     required: true,
     disabled,
-    description: getDestinationValueDescription(destinationType),
+    description: destinationValueDescription[destinationType],
   }
   const targetValueProps = {
     name: 'target.value' as const,
     label: 'Target value',
     control,
-    // possible targetTypes needing placeholders are instances or IPs (internet_gateway has no placeholder)
-    placeholder: getTargetValuePlaceholder(targetType),
+    placeholder: targetValuePlaceholder[targetType],
     required: true,
     // 'internet_gateway' targetTypes can only have the value 'outbound', so we disable the field
     disabled: disabled || targetType === 'internet_gateway',
-    description: getTargetValueDescription(targetType),
+    description: targetValueDescription[targetType],
   }
   return (
     <>
@@ -143,7 +144,7 @@ export const RouteFormFields = ({ form, disabled }: RouteFormFieldsProps) => {
         name="destination.type"
         label="Destination type"
         control={control}
-        items={toItems(destTypes)}
+        items={toListboxItems(destTypes)}
         placeholder="Select a destination type"
         required
         onChange={() => {
@@ -152,10 +153,7 @@ export const RouteFormFields = ({ form, disabled }: RouteFormFieldsProps) => {
         disabled={disabled}
       />
       {destinationType === 'subnet' ? (
-        <ComboboxField
-          {...destinationValueProps}
-          items={vpcSubnets.map(({ name }) => ({ value: name, label: name }))}
-        />
+        <ComboboxField {...destinationValueProps} items={toComboboxItems(vpcSubnets)} />
       ) : (
         <TextField {...destinationValueProps} />
       )}
@@ -163,7 +161,7 @@ export const RouteFormFields = ({ form, disabled }: RouteFormFieldsProps) => {
         name="target.type"
         label="Target type"
         control={control}
-        items={toItems(targetTypes)}
+        items={toListboxItems(targetTypes)}
         placeholder="Select a target type"
         required
         onChange={(value) => {
@@ -172,10 +170,7 @@ export const RouteFormFields = ({ form, disabled }: RouteFormFieldsProps) => {
         disabled={disabled}
       />
       {targetType === 'drop' ? null : targetType === 'instance' ? (
-        <ComboboxField
-          {...targetValueProps}
-          items={instances.map(({ name }) => ({ value: name, label: name }))}
-        />
+        <ComboboxField {...targetValueProps} items={toComboboxItems(instances)} />
       ) : (
         <TextField {...targetValueProps} />
       )}