Make it harder to forget to submit targets/filters subform

app/forms/firewall-rules-common.tsx

@@ -6,7 +6,7 @@
  * Copyright Oxide Computer Company
  */
 
-import { useEffect, type ReactNode } from 'react'
+import { useEffect, useState, type ReactNode } from 'react'
 import { useController, useForm, type Control } from 'react-hook-form'
 
 import {
@@ -40,7 +40,7 @@ import { KEYS } from '~/ui/util/keys'
 import { ALL_ISH } from '~/util/consts'
 import { validateIp, validateIpNet } from '~/util/ip'
 import { links } from '~/util/links'
-import { capitalize } from '~/util/str'
+import { capitalize, commaSeries } from '~/util/str'
 
 import { type FirewallRuleValues } from './firewall-rules-util'
 
@@ -88,10 +88,12 @@ const TargetAndHostFilterSubform = ({
   sectionType,
   control,
   messageContent,
+  updateSubformStates,
 }: {
   sectionType: 'target' | 'host'
   control: Control<FirewallRuleValues>
   messageContent: ReactNode
+  updateSubformStates: (subform: keyof ActiveSubforms, value: boolean) => void
 }) => {
   const { project, vpc } = useVpcSelector()
   // prefetchedApiQueries below are prefetched in firewall-rules-create and -edit
@@ -125,8 +127,11 @@ const TargetAndHostFilterSubform = ({
   // https://github.com/react-hook-form/react-hook-form/blob/9a497a70a/src/logic/createFormControl.ts#L1194-L1203
   const { isSubmitSuccessful: subformSubmitSuccessful } = subform.formState
   useEffect(() => {
-    if (subformSubmitSuccessful) subform.reset(targetAndHostDefaultValues)
-  }, [subformSubmitSuccessful, subform])
+    if (subformSubmitSuccessful) {
+      subform.reset(targetAndHostDefaultValues)
+      updateSubformStates(sectionType, false)
+    }
+  }, [subformSubmitSuccessful, subform, updateSubformStates, sectionType])
 
   const [valueType, value] = subform.watch(['type', 'value'])
   const sectionItems = {
@@ -143,9 +148,15 @@ const TargetAndHostFilterSubform = ({
   // back to validating on submit instead of change. Also resets readyToSubmit.
   const onTypeChange = () => {
     subform.reset({ type: subform.getValues('type'), value: '' })
+    updateSubformStates(sectionType, false)
   }
   const onInputChange = (value: string) => {
     subform.setValue('value', value)
+    updateSubformStates(sectionType, value.length > 0)
+  }
+  const onClear = () => {
+    subform.reset()
+    updateSubformStates(sectionType, false)
   }
 
   return (
@@ -178,6 +189,7 @@ const TargetAndHostFilterSubform = ({
           description="Select an option or enter a custom value"
           control={subformControl}
           onEnter={submitSubform}
+          onChange={() => updateSubformStates(sectionType, true)}
           onInputChange={onInputChange}
           items={items}
           allowArbitraryValues
@@ -212,7 +224,7 @@ const TargetAndHostFilterSubform = ({
       <MiniTable.ClearAndAddButtons
         addButtonCopy={`Add ${sectionType === 'host' ? 'host filter' : 'target'}`}
         disabled={!value}
-        onClear={() => subform.reset()}
+        onClear={onClear}
         onSubmit={submitSubform}
       />
       {field.value.length > 0 && (
@@ -289,14 +301,20 @@ type CommonFieldsProps = {
   control: Control<FirewallRuleValues>
   nameTaken: (name: string) => boolean
   error: ApiError | null
+  updateSubformStates: (subform: keyof ActiveSubforms, value: boolean) => void
 }
 
 const targetAndHostDefaultValues: TargetAndHostFormValues = {
   type: 'vpc',
   value: '',
 }
 
-export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) => {
+export const CommonFields = ({
+  control,
+  nameTaken,
+  error,
+  updateSubformStates,
+}: CommonFieldsProps) => {
   // Ports
   const portRangeForm = useForm({ defaultValues: { portRange: '' } })
   const ports = useController({ name: 'ports', control }).field
@@ -307,7 +325,11 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) =
     // that it is not already in the list
     ports.onChange([...ports.value, portRangeValue])
     portRangeForm.reset()
+    updateSubformStates('port', false)
   })
+  useEffect(() => {
+    updateSubformStates('port', portValue.length > 0)
+  }, [updateSubformStates, portValue])
 
   return (
     <>
@@ -406,6 +428,7 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) =
             </p>
           </>
         }
+        updateSubformStates={updateSubformStates}
       />
 
       <FormDivider />
@@ -453,7 +476,10 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) =
         <MiniTable.ClearAndAddButtons
           addButtonCopy="Add port filter"
           disabled={!portValue}
-          onClear={() => portRangeForm.reset()}
+          onClear={() => {
+            portRangeForm.reset()
+            updateSubformStates('port', false)
+          }}
           onSubmit={submitPortRange}
         />
       </div>
@@ -500,6 +526,7 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) =
             traffic. For an outbound rule, they match the destination.
           </>
         }
+        updateSubformStates={updateSubformStates}
       />
 
       {error && (
@@ -511,3 +538,33 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) =
     </>
   )
 }
+
+export type ActiveSubforms = { target: boolean; port: boolean; host: boolean }
+export const defaultActiveSubforms: ActiveSubforms = {
+  target: false,
+  port: false,
+  host: false,
+}
+
+export function useSubformStates(defaultActiveSubforms: ActiveSubforms) {
+  const [subformStates, setSubformStates] = useState(defaultActiveSubforms)
+  const updateSubformStates = (subform: keyof ActiveSubforms, value: boolean) => {
+    setSubformStates((prev) => ({ ...prev, [subform]: value }))
+  }
+  return { subformStates, updateSubformStates }
+}
+
+export const getActiveSubformList = (subformStates: ActiveSubforms) =>
+  commaSeries(
+    Object.keys(subformStates).filter((key) => subformStates[key as keyof ActiveSubforms]),
+    'and'
+  )
+    .replace('port', 'port filter')
+    .replace('host', 'host filter')
+
+export const submitDisabledMessage = (subformStates: ActiveSubforms) => {
+  const activeSubformList = getActiveSubformList(subformStates)
+  return activeSubformList.length > 0
+    ? `You have an unsaved ${activeSubformList} entry; save or clear ${activeSubformList.includes('and') ? 'them' : 'it'} to create this firewall rule`
+    : ''
+}

app/forms/firewall-rules-create.tsx

@@ -25,7 +25,12 @@ import { addToast } from '~/stores/toast'
 import { ALL_ISH } from '~/util/consts'
 import { pb } from '~/util/path-builder'
 
-import { CommonFields } from './firewall-rules-common'
+import {
+  CommonFields,
+  defaultActiveSubforms,
+  submitDisabledMessage,
+  useSubformStates,
+} from './firewall-rules-common'
 import { valuesToRuleUpdate, type FirewallRuleValues } from './firewall-rules-util'
 
 export const handle = titleCrumb('New Rule')
@@ -73,6 +78,7 @@ export async function clientLoader({ params }: LoaderFunctionArgs) {
 }
 
 export default function CreateFirewallRuleForm() {
+  const { subformStates, updateSubformStates } = useSubformStates(defaultActiveSubforms)
   const vpcSelector = useVpcSelector()
   const queryClient = useApiQueryClient()
 
@@ -121,14 +127,16 @@ export default function CreateFirewallRuleForm() {
         })
       }}
       loading={updateRules.isPending}
-      submitError={updateRules.error}
       submitLabel="Add rule"
+      submitDisabled={submitDisabledMessage(subformStates)}
+      submitError={updateRules.error}
     >
       <CommonFields
         control={form.control}
         // error if name is already in use
         nameTaken={(name) => !!existingRules.find((r) => r.name === name)}
         error={updateRules.error}
+        updateSubformStates={updateSubformStates}
         // TODO: there should also be a form-level error so if the name is off
         // screen, it doesn't look like the submit button isn't working. Maybe
         // instead of setting a root error, it would be more robust to show a

app/forms/firewall-rules-edit.tsx

@@ -30,7 +30,12 @@ import { ALL_ISH } from '~/util/consts'
 import { invariant } from '~/util/invariant'
 import { pb } from '~/util/path-builder'
 
-import { CommonFields } from './firewall-rules-common'
+import {
+  CommonFields,
+  defaultActiveSubforms,
+  submitDisabledMessage,
+  useSubformStates,
+} from './firewall-rules-common'
 import { valuesToRuleUpdate, type FirewallRuleValues } from './firewall-rules-util'
 
 export const handle = titleCrumb('Edit Rule')
@@ -58,6 +63,8 @@ export default function EditFirewallRuleForm() {
   const vpcSelector = useVpcSelector()
   const queryClient = useApiQueryClient()
 
+  const { subformStates, updateSubformStates } = useSubformStates(defaultActiveSubforms)
+
   const { data: firewallRules } = usePrefetchedApiQuery('vpcFirewallRulesView', {
     query: { project, vpc },
   })
@@ -125,13 +132,15 @@ export default function EditFirewallRuleForm() {
       // validationSchema={validationSchema}
       // validateOnBlur
       loading={updateRules.isPending}
+      submitDisabled={submitDisabledMessage(subformStates)}
       submitError={updateRules.error}
     >
       <CommonFields
         control={form.control}
         // error if name is being changed to something that conflicts with some other rule
         nameTaken={(name) => !!otherRules.find((r) => r.name === name)}
         error={updateRules.error}
+        updateSubformStates={updateSubformStates}
       />
     </SideModalForm>
   )