From b4ce08d86e60c3bdce0772ddf110bd7041d52b6d Mon Sep 17 00:00:00 2001
From: Benjamin Leonard <benji@oxide.computer>
Date: Wed, 5 Mar 2025 12:14:47 +0000
Subject: [PATCH 01/16] Attach disk modal instead of side modal

---
 app/components/form/ModalForm.tsx             | 105 ++++++++++++++++++
 .../form/fields/DisksTableField.tsx           |   4 +-
 app/forms/disk-attach.tsx                     |  23 ++--
 app/pages/project/instances/StorageTab.tsx    |  68 +++++-------
 4 files changed, 150 insertions(+), 50 deletions(-)
 create mode 100644 app/components/form/ModalForm.tsx

diff --git a/app/components/form/ModalForm.tsx b/app/components/form/ModalForm.tsx
new file mode 100644
index 0000000000..1054a55105
--- /dev/null
+++ b/app/components/form/ModalForm.tsx
@@ -0,0 +1,105 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, you can obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * Copyright Oxide Computer Company
+ */
+
+import { useId, type ReactNode } from 'react'
+import type { FieldValues, UseFormReturn } from 'react-hook-form'
+
+import type { ApiError } from '@oxide/api'
+
+import { Message } from '~/ui/lib/Message'
+import { Modal, type ModalProps } from '~/ui/lib/Modal'
+
+type ModalFormProps<TFieldValues extends FieldValues> = {
+  form: UseFormReturn<TFieldValues>
+  /**
+   * A function that returns the fields.
+   *
+   * Implemented as a function so we can pass `control` to the fields in the
+   * calling code. We could do that internally with `cloneElement` instead, but
+   * then in the calling code, the field would not infer `TFieldValues` and
+   * constrain the `name` prop to paths in the values object.
+   */
+  children: ReactNode
+  resourceName: string
+  /** Must be provided with a reason describing why it's disabled */
+  submitDisabled?: string
+
+  // require loading and error so we can't forget to hook them up. there are a
+  // few forms that don't need them, so we'll use dummy values
+
+  /** Error from the API call */
+  submitError: ApiError | null
+  loading: boolean
+
+  /** Only needed if you need to override the default title (Create/Edit ${resourceName}) */
+  subtitle?: ReactNode
+  onSubmit: (values: TFieldValues) => void
+
+  submitLabel?: string
+} & Omit<ModalProps, 'isOpen'>
+
+export function ModalForm<TFieldValues extends FieldValues>({
+  form,
+  children,
+  onDismiss,
+  resourceName,
+  submitDisabled,
+  submitError,
+  title,
+  onSubmit,
+  submitLabel = 'Save',
+  loading,
+  subtitle,
+  width = 'medium',
+  overlay = true,
+}: ModalFormProps<TFieldValues>) {
+  const id = useId()
+
+  const { isSubmitting } = form.formState
+
+  const modalTitle = title || `Create ${resourceName}`
+
+  return (
+    <>
+      <Modal
+        isOpen
+        onDismiss={onDismiss}
+        title={modalTitle}
+        width={width}
+        overlay={overlay}
+      >
+        <Modal.Body>
+          <Modal.Section>
+            {subtitle && <div className="mb-4">{subtitle}</div>}
+            {submitError && (
+              <Message variant="error" title="Error" content={submitError.message} />
+            )}
+            <form
+              id={id}
+              className="ox-form"
+              autoComplete="off"
+              onSubmit={(e) => {
+                if (!onSubmit) return
+                form.handleSubmit(onSubmit)(e)
+              }}
+            >
+              {children}
+            </form>
+          </Modal.Section>
+        </Modal.Body>
+        <Modal.Footer
+          onDismiss={onDismiss}
+          formId={id}
+          actionText={submitLabel}
+          disabled={!!submitDisabled}
+          actionLoading={loading || isSubmitting}
+        />
+      </Modal>
+    </>
+  )
+}
diff --git a/app/components/form/fields/DisksTableField.tsx b/app/components/form/fields/DisksTableField.tsx
index 2d9aa12708..6134b505f3 100644
--- a/app/components/form/fields/DisksTableField.tsx
+++ b/app/components/form/fields/DisksTableField.tsx
@@ -10,7 +10,7 @@ import { useController, type Control } from 'react-hook-form'
 
 import type { DiskCreate } from '@oxide/api'
 
-import { AttachDiskSideModalForm } from '~/forms/disk-attach'
+import { AttachDiskModalForm } from '~/forms/disk-attach'
 import { CreateDiskSideModalForm } from '~/forms/disk-create'
 import type { InstanceCreateInput } from '~/forms/instance-create'
 import { Badge } from '~/ui/lib/Badge'
@@ -115,7 +115,7 @@ export function DisksTableField({
         />
       )}
       {showDiskAttach && (
-        <AttachDiskSideModalForm
+        <AttachDiskModalForm
           onDismiss={() => setShowDiskAttach(false)}
           onSubmit={(values) => {
             onChange([...items, { type: 'attach', ...values }])
diff --git a/app/forms/disk-attach.tsx b/app/forms/disk-attach.tsx
index 5a6eeaaaf7..9dd6a695f8 100644
--- a/app/forms/disk-attach.tsx
+++ b/app/forms/disk-attach.tsx
@@ -11,7 +11,7 @@ import { useForm } from 'react-hook-form'
 import { useApiQuery, type ApiError } from '@oxide/api'
 
 import { ComboboxField } from '~/components/form/fields/ComboboxField'
-import { SideModalForm } from '~/components/form/SideModalForm'
+import { ModalForm } from '~/components/form/ModalForm'
 import { useProjectSelector } from '~/hooks/use-params'
 import { toComboboxItems } from '~/ui/lib/Combobox'
 import { ALL_ISH } from '~/util/consts'
@@ -31,7 +31,7 @@ type AttachDiskProps = {
  * Can be used with either a `setState` or a real mutation as `onSubmit`, hence
  * the optional `loading` and `submitError`
  */
-export function AttachDiskSideModalForm({
+export function AttachDiskModalForm({
   onSubmit,
   onDismiss,
   diskNamesToExclude = [],
@@ -40,7 +40,7 @@ export function AttachDiskSideModalForm({
 }: AttachDiskProps) {
   const { project } = useProjectSelector()
 
-  const { data } = useApiQuery('diskList', {
+  const { data, isPending } = useApiQuery('diskList', {
     query: { project, limit: ALL_ISH },
   })
   const detachedDisks = useMemo(
@@ -54,17 +54,19 @@ export function AttachDiskSideModalForm({
   )
 
   const form = useForm({ defaultValues })
+  const { control } = form
 
   return (
-    <SideModalForm
+    <ModalForm
       form={form}
-      formType="create"
+      onDismiss={onDismiss}
       resourceName="disk"
+      submitLabel="Attach disk"
+      submitError={submitError}
+      loading={loading}
       title="Attach disk"
       onSubmit={onSubmit}
-      loading={loading}
-      submitError={submitError}
-      onDismiss={onDismiss}
+      width="medium"
     >
       <ComboboxField
         label="Disk name"
@@ -72,8 +74,9 @@ export function AttachDiskSideModalForm({
         name="name"
         items={detachedDisks}
         required
-        control={form.control}
+        control={control}
+        isLoading={isPending}
       />
-    </SideModalForm>
+    </ModalForm>
   )
 }
diff --git a/app/pages/project/instances/StorageTab.tsx b/app/pages/project/instances/StorageTab.tsx
index fb58644b78..42a094dfa2 100644
--- a/app/pages/project/instances/StorageTab.tsx
+++ b/app/pages/project/instances/StorageTab.tsx
@@ -25,7 +25,7 @@ import { Storage24Icon } from '@oxide/design-system/icons/react'
 
 import { HL } from '~/components/HL'
 import { DiskStateBadge } from '~/components/StateBadge'
-import { AttachDiskSideModalForm } from '~/forms/disk-attach'
+import { AttachDiskModalForm } from '~/forms/disk-attach'
 import { CreateDiskSideModalForm } from '~/forms/disk-create'
 import { getInstanceSelector, useInstanceSelector } from '~/hooks/use-params'
 import { confirmAction } from '~/stores/confirm-action'
@@ -295,13 +295,6 @@ export default function StorageTab() {
       setShowDiskCreate(false)
       setShowDiskAttach(false)
     },
-    onError(err) {
-      addToast({
-        title: 'Failed to attach disk',
-        content: err.message,
-        variant: 'error',
-      })
-    },
   })
 
   const bootDisksTable = useReactTable({
@@ -341,35 +334,33 @@ export default function StorageTab() {
 
       <CardBlock>
         <CardBlock.Header title="Additional disks" titleId="other-disks-label">
-          <div className="flex gap-3">
-            <Button
-              variant="secondary"
-              size="sm"
-              onClick={() => setShowDiskAttach(true)}
-              disabledReason={
-                <>
-                  Instance must be <span className="text-raise">stopped</span> to attach a
-                  disk
-                </>
-              }
-              disabled={!instanceCan.attachDisk(instance)}
-            >
-              Attach existing disk
-            </Button>
-            <Button
-              size="sm"
-              onClick={() => setShowDiskCreate(true)}
-              disabledReason={
-                <>
-                  Instance must be <span className="text-raise">stopped</span> to create and
-                  attach a disk
-                </>
-              }
-              disabled={!instanceCan.attachDisk(instance)}
-            >
-              Create disk
-            </Button>
-          </div>
+          <Button
+            variant="secondary"
+            size="sm"
+            onClick={() => setShowDiskAttach(true)}
+            disabledReason={
+              <>
+                Instance must be <span className="text-raise">stopped</span> to attach a
+                disk
+              </>
+            }
+            disabled={!instanceCan.attachDisk(instance)}
+          >
+            Attach existing disk
+          </Button>
+          <Button
+            size="sm"
+            onClick={() => setShowDiskCreate(true)}
+            disabledReason={
+              <>
+                Instance must be <span className="text-raise">stopped</span> to create and
+                attach a disk
+              </>
+            }
+            disabled={!instanceCan.attachDisk(instance)}
+          >
+            Create disk
+          </Button>
         </CardBlock.Header>
         <CardBlock.Body>
           {otherDisks.length > 0 ? (
@@ -395,13 +386,14 @@ export default function StorageTab() {
         />
       )}
       {showDiskAttach && (
-        <AttachDiskSideModalForm
+        <AttachDiskModalForm
           onDismiss={() => setShowDiskAttach(false)}
           onSubmit={({ name }) => {
             attachDisk.mutate({ ...instancePathQuery, body: { disk: name } })
           }}
           loading={attachDisk.isPending}
           submitError={attachDisk.error}
+          instance={instance}
         />
       )}
     </div>

From 61438c6845f0a34eaaaa015c9c35533a90aafbd0 Mon Sep 17 00:00:00 2001
From: Benjamin Leonard <benji@oxide.computer>
Date: Wed, 5 Mar 2025 12:16:42 +0000
Subject: [PATCH 02/16] Experimental stop instance in modal

---
 app/components/StopInstancePrompt.tsx | 110 ++++++++++++++++++++++++++
 app/forms/disk-attach.tsx             |  11 ++-
 app/ui/lib/Button.tsx                 |   7 +-
 app/ui/styles/components/button.css   |   4 +
 app/ui/styles/components/spinner.css  |  24 ++----
 5 files changed, 133 insertions(+), 23 deletions(-)
 create mode 100644 app/components/StopInstancePrompt.tsx

diff --git a/app/components/StopInstancePrompt.tsx b/app/components/StopInstancePrompt.tsx
new file mode 100644
index 0000000000..54d129f44f
--- /dev/null
+++ b/app/components/StopInstancePrompt.tsx
@@ -0,0 +1,110 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, you can obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * Copyright Oxide Computer Company
+ */
+import { useEffect, useState, type ReactNode } from 'react'
+
+import {
+  instanceTransitioning,
+  useApiMutation,
+  useApiQuery,
+  useApiQueryClient,
+  type Instance,
+} from '@oxide/api'
+
+import { HL } from '~/components/HL'
+import { addToast } from '~/stores/toast'
+import { Button } from '~/ui/lib/Button'
+import { Message } from '~/ui/lib/Message'
+
+const POLL_INTERVAL_FAST = 2000 // 2 seconds
+
+type StopInstancePromptProps = {
+  instance: Instance
+  children: ReactNode
+}
+
+export function StopInstancePrompt({ instance, children }: StopInstancePromptProps) {
+  const queryClient = useApiQueryClient()
+  const [isStoppingInstance, setIsStoppingInstance] = useState(false)
+
+  const { data } = useApiQuery(
+    'instanceView',
+    {
+      path: { instance: instance.name },
+      query: { project: instance.projectId },
+    },
+    {
+      refetchInterval:
+        isStoppingInstance || instanceTransitioning(instance) ? POLL_INTERVAL_FAST : false,
+    }
+  )
+
+  const { mutateAsync: stopInstanceAsync } = useApiMutation('instanceStop', {
+    onSuccess: () => {
+      setIsStoppingInstance(true)
+      addToast(
+        <>
+          Stopping instance <HL>{instance.name}</HL>
+        </>
+      )
+    },
+    onError: (error) => {
+      addToast({
+        variant: 'error',
+        title: `Error stopping instance '${instance.name}'`,
+        content: error.message,
+      })
+      setIsStoppingInstance(false)
+    },
+  })
+
+  const handleStopInstance = () => {
+    stopInstanceAsync({
+      path: { instance: instance.name },
+      query: { project: instance.projectId },
+    })
+  }
+
+  const currentInstance = data || instance
+
+  useEffect(() => {
+    if (!data) {
+      return
+    }
+    if (isStoppingInstance && data.runState === 'stopped') {
+      queryClient.invalidateQueries('instanceView')
+      setIsStoppingInstance(false)
+    }
+  }, [isStoppingInstance, data, queryClient])
+
+  if (
+    !currentInstance ||
+    (currentInstance.runState !== 'stopping' && currentInstance.runState !== 'running')
+  ) {
+    return null
+  }
+
+  return (
+    <Message
+      variant="notice"
+      content={
+        <>
+          {children}{' '}
+          <Button
+            size="xs"
+            className="mt-3"
+            variant="notice"
+            onClick={handleStopInstance}
+            loading={isStoppingInstance}
+          >
+            Stop instance
+          </Button>
+        </>
+      }
+    />
+  )
+}
diff --git a/app/forms/disk-attach.tsx b/app/forms/disk-attach.tsx
index 9dd6a695f8..2e2007d46c 100644
--- a/app/forms/disk-attach.tsx
+++ b/app/forms/disk-attach.tsx
@@ -8,10 +8,11 @@
 import { useMemo } from 'react'
 import { useForm } from 'react-hook-form'
 
-import { useApiQuery, type ApiError } from '@oxide/api'
+import { useApiQuery, type ApiError, type Instance } from '@oxide/api'
 
 import { ComboboxField } from '~/components/form/fields/ComboboxField'
 import { ModalForm } from '~/components/form/ModalForm'
+import { StopInstancePrompt } from '~/components/StopInstancePrompt'
 import { useProjectSelector } from '~/hooks/use-params'
 import { toComboboxItems } from '~/ui/lib/Combobox'
 import { ALL_ISH } from '~/util/consts'
@@ -25,6 +26,7 @@ type AttachDiskProps = {
   diskNamesToExclude?: string[]
   loading?: boolean
   submitError?: ApiError | null
+  instance: Instance
 }
 
 /**
@@ -37,6 +39,7 @@ export function AttachDiskModalForm({
   diskNamesToExclude = [],
   loading = false,
   submitError = null,
+  instance,
 }: AttachDiskProps) {
   const { project } = useProjectSelector()
 
@@ -67,7 +70,13 @@ export function AttachDiskModalForm({
       title="Attach disk"
       onSubmit={onSubmit}
       width="medium"
+      submitDisabled={
+        instance.runState !== 'stopped' ? 'Instance must be stopped' : undefined
+      }
     >
+      <StopInstancePrompt instance={instance}>
+        An instance must be stopped to attach a disk.
+      </StopInstancePrompt>
       <ComboboxField
         label="Disk name"
         placeholder="Select a disk"
diff --git a/app/ui/lib/Button.tsx b/app/ui/lib/Button.tsx
index e7b27f383d..90141be0da 100644
--- a/app/ui/lib/Button.tsx
+++ b/app/ui/lib/Button.tsx
@@ -13,13 +13,14 @@ import { Spinner } from '~/ui/lib/Spinner'
 import { Tooltip } from '~/ui/lib/Tooltip'
 import { Wrap } from '~/ui/util/wrap'
 
-export const buttonSizes = ['sm', 'icon', 'base'] as const
-export const variants = ['primary', 'secondary', 'ghost', 'danger'] as const
+export const buttonSizes = ['xs', 'sm', 'icon', 'base'] as const
+export const variants = ['primary', 'secondary', 'ghost', 'danger', 'notice'] as const
 
 export type ButtonSize = (typeof buttonSizes)[number]
 export type Variant = (typeof variants)[number]
 
 const sizeStyle: Record<ButtonSize, string> = {
+  xs: 'h-6 px-2 text-mono-xs',
   sm: 'h-8 px-3 text-mono-sm [&>svg]:w-4',
   // meant for buttons that only contain a single icon
   icon: 'h-8 w-8 text-mono-sm [&>svg]:w-4',
@@ -115,7 +116,7 @@ export const Button = ({
             animate={{ opacity: 1, y: '-50%', x: '-50%' }}
             initial={{ opacity: 0, y: 'calc(-50% - 25px)', x: '-50%' }}
             transition={{ type: 'spring', duration: 0.3, bounce: 0 }}
-            className="absolute left-1/2 top-1/2"
+            className="absolute left-1/2 top-1/2 flex items-center justify-center"
           >
             <Spinner variant={variant} />
           </m.span>
diff --git a/app/ui/styles/components/button.css b/app/ui/styles/components/button.css
index 9575fde921..9ad91e8caa 100644
--- a/app/ui/styles/components/button.css
+++ b/app/ui/styles/components/button.css
@@ -44,6 +44,10 @@
   @apply hidden;
 }
 
+.btn-notice.ox-button:after {
+  @apply opacity-40;
+}
+
 /**
  * A class to make it very visually obvious that a button style is missing
  */
diff --git a/app/ui/styles/components/spinner.css b/app/ui/styles/components/spinner.css
index 495e45363e..636bc94bdd 100644
--- a/app/ui/styles/components/spinner.css
+++ b/app/ui/styles/components/spinner.css
@@ -13,6 +13,11 @@
   animation: rotate 5s linear infinite;
 }
 
+.spinner .path,
+.spinner .bg {
+  stroke: currentColor;
+}
+
 .spinner.spinner-md {
   --radius: 8;
   --circumference: calc(var(--PI) * var(--radius) * 2px);
@@ -27,7 +32,6 @@
   stroke-dasharray: var(--circumference);
   transform-origin: center;
   animation: dash 8s ease-in-out infinite;
-  stroke: var(--content-accent-tertiary);
 }
 
 @media (prefers-reduced-motion) {
@@ -50,24 +54,6 @@
   }
 }
 
-.spinner-ghost .bg,
-.spinner-secondary .bg {
-  stroke: var(--content-default);
-}
-
-.spinner-secondary .path {
-  stroke: var(--content-secondary);
-}
-
-.spinner-primary .bg {
-  stroke: var(--content-accent);
-}
-
-.spinner-danger .bg,
-.spinner-danger .path {
-  stroke: var(--content-destructive-tertiary);
-}
-
 @keyframes rotate {
   100% {
     transform: rotate(360deg);

From 036b2956179a4d44f0daa80ca5d064187afa575c Mon Sep 17 00:00:00 2001
From: Benjamin Leonard <benji@oxide.computer>
Date: Wed, 5 Mar 2025 12:17:03 +0000
Subject: [PATCH 03/16] Tweak disabled text button colour

---
 app/ui/styles/components/button.css | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/ui/styles/components/button.css b/app/ui/styles/components/button.css
index 9575fde921..0ad83873ef 100644
--- a/app/ui/styles/components/button.css
+++ b/app/ui/styles/components/button.css
@@ -22,7 +22,7 @@
 }
 
 .btn-secondary {
-  @apply text-default bg-secondary hover:bg-hover disabled:text-tertiary disabled:bg-secondary;
+  @apply text-default bg-secondary hover:bg-hover disabled:text-disabled disabled:bg-secondary;
 }
 .btn-secondary:disabled > .spinner,
 .btn-secondary.visually-disabled > .spinner {

From 5bdb549fe86e117d4094f7fb9c88ec60b0043150 Mon Sep 17 00:00:00 2001
From: Benjamin Leonard <benji@oxide.computer>
Date: Wed, 5 Mar 2025 12:20:18 +0000
Subject: [PATCH 04/16] Type fix

---
 app/pages/project/instances/StorageTab.tsx | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/pages/project/instances/StorageTab.tsx b/app/pages/project/instances/StorageTab.tsx
index 42a094dfa2..b02b81f6db 100644
--- a/app/pages/project/instances/StorageTab.tsx
+++ b/app/pages/project/instances/StorageTab.tsx
@@ -393,7 +393,6 @@ export default function StorageTab() {
           }}
           loading={attachDisk.isPending}
           submitError={attachDisk.error}
-          instance={instance}
         />
       )}
     </div>

From b1dc6054ddeab0061d40eaf1f350f2230b8fb9f1 Mon Sep 17 00:00:00 2001
From: Benjamin Leonard <benji@oxide.computer>
Date: Wed, 5 Mar 2025 12:27:54 +0000
Subject: [PATCH 05/16] Cleanup

---
 app/forms/disk-attach.tsx                  |  4 ++--
 app/pages/project/instances/StorageTab.tsx | 13 +------------
 2 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/app/forms/disk-attach.tsx b/app/forms/disk-attach.tsx
index 2e2007d46c..a6bafcc4c0 100644
--- a/app/forms/disk-attach.tsx
+++ b/app/forms/disk-attach.tsx
@@ -8,7 +8,7 @@
 import { useMemo } from 'react'
 import { useForm } from 'react-hook-form'
 
-import { useApiQuery, type ApiError, type Instance } from '@oxide/api'
+import { instanceCan, useApiQuery, type ApiError, type Instance } from '@oxide/api'
 
 import { ComboboxField } from '~/components/form/fields/ComboboxField'
 import { ModalForm } from '~/components/form/ModalForm'
@@ -71,7 +71,7 @@ export function AttachDiskModalForm({
       onSubmit={onSubmit}
       width="medium"
       submitDisabled={
-        instance.runState !== 'stopped' ? 'Instance must be stopped' : undefined
+        !instanceCan.attachDisk(instance) ? 'Instance must be stopped' : undefined
       }
     >
       <StopInstancePrompt instance={instance}>
diff --git a/app/pages/project/instances/StorageTab.tsx b/app/pages/project/instances/StorageTab.tsx
index 42a094dfa2..a1090c0bea 100644
--- a/app/pages/project/instances/StorageTab.tsx
+++ b/app/pages/project/instances/StorageTab.tsx
@@ -334,18 +334,7 @@ export default function StorageTab() {
 
       <CardBlock>
         <CardBlock.Header title="Additional disks" titleId="other-disks-label">
-          <Button
-            variant="secondary"
-            size="sm"
-            onClick={() => setShowDiskAttach(true)}
-            disabledReason={
-              <>
-                Instance must be <span className="text-raise">stopped</span> to attach a
-                disk
-              </>
-            }
-            disabled={!instanceCan.attachDisk(instance)}
-          >
+          <Button variant="secondary" size="sm" onClick={() => setShowDiskAttach(true)}>
             Attach existing disk
           </Button>
           <Button

From a0e2b0c5eb8416908eeb476fffba2a5888b92839 Mon Sep 17 00:00:00 2001
From: Benjamin Leonard <benji@oxide.computer>
Date: Wed, 5 Mar 2025 12:42:24 +0000
Subject: [PATCH 06/16] Also needed on `ModalForm`

---
 app/components/form/ModalForm.tsx | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/app/components/form/ModalForm.tsx b/app/components/form/ModalForm.tsx
index 1054a55105..73fbe9cc32 100644
--- a/app/components/form/ModalForm.tsx
+++ b/app/components/form/ModalForm.tsx
@@ -85,6 +85,12 @@ export function ModalForm<TFieldValues extends FieldValues>({
               autoComplete="off"
               onSubmit={(e) => {
                 if (!onSubmit) return
+                // This modal being in a portal doesn't prevent the submit event
+                // from bubbling up out of the portal. Normally that's not a
+                // problem, but sometimes (e.g., instance create) we render the
+                // SideModalForm from inside another form, in which case submitting
+                // the inner form submits the outer form unless we stop propagation
+                e.stopPropagation()
                 form.handleSubmit(onSubmit)(e)
               }}
             >

From 58241576f1b31b8231dfcc3a2806c527b4377b70 Mon Sep 17 00:00:00 2001
From: Benjamin Leonard <benji@oxide.computer>
Date: Wed, 5 Mar 2025 18:02:50 +0000
Subject: [PATCH 07/16] Already defaults to medium

---
 app/forms/disk-attach.tsx | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/forms/disk-attach.tsx b/app/forms/disk-attach.tsx
index 9dd6a695f8..bd53792ab3 100644
--- a/app/forms/disk-attach.tsx
+++ b/app/forms/disk-attach.tsx
@@ -66,7 +66,6 @@ export function AttachDiskModalForm({
       loading={loading}
       title="Attach disk"
       onSubmit={onSubmit}
-      width="medium"
     >
       <ComboboxField
         label="Disk name"

From 13ac403eab37d4f61b16bca4fde7d3cb36a3e4b5 Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Wed, 5 Mar 2025 16:16:05 -0600
Subject: [PATCH 08/16] convert image promote modal

---
 app/pages/SiloImagesPage.tsx | 77 ++++++++++++++++++------------------
 1 file changed, 38 insertions(+), 39 deletions(-)

diff --git a/app/pages/SiloImagesPage.tsx b/app/pages/SiloImagesPage.tsx
index 82f0ca89dd..4a71dd538e 100644
--- a/app/pages/SiloImagesPage.tsx
+++ b/app/pages/SiloImagesPage.tsx
@@ -24,6 +24,7 @@ import { DocsPopover } from '~/components/DocsPopover'
 import { ComboboxField } from '~/components/form/fields/ComboboxField'
 import { toImageComboboxItem } from '~/components/form/fields/ImageSelectField'
 import { ListboxField } from '~/components/form/fields/ListboxField'
+import { ModalForm } from '~/components/form/ModalForm'
 import { HL } from '~/components/HL'
 import { confirmDelete } from '~/stores/confirm-delete'
 import { addToast } from '~/stores/toast'
@@ -129,7 +130,7 @@ type Values = { project: string | null; image: string | null }
 const defaultValues: Values = { project: null, image: null }
 
 const PromoteImageModal = ({ onDismiss }: { onDismiss: () => void }) => {
-  const { control, handleSubmit, watch, resetField } = useForm({ defaultValues })
+  const form = useForm({ defaultValues })
 
   const queryClient = useApiQueryClient()
 
@@ -146,7 +147,7 @@ const PromoteImageModal = ({ onDismiss }: { onDismiss: () => void }) => {
 
   const projects = useApiQuery('projectList', {})
   const projectItems = useMemo(() => toComboboxItems(projects.data?.items), [projects.data])
-  const selectedProject = watch('project')
+  const selectedProject = form.watch('project')
 
   // can only fetch images if a project is selected
   const images = useApiQuery(
@@ -165,44 +166,42 @@ const PromoteImageModal = ({ onDismiss }: { onDismiss: () => void }) => {
   }
 
   return (
-    <Modal isOpen onDismiss={onDismiss} title="Promote image">
-      <Modal.Body>
-        <Modal.Section>
-          <form autoComplete="off" onSubmit={handleSubmit(onSubmit)} className="space-y-4">
-            <ComboboxField
-              placeholder="Select a project"
-              name="project"
-              label="Project"
-              items={projectItems}
-              onChange={() => {
-                resetField('image') // reset image field when the project changes
-              }}
-              isLoading={projects.isPending}
-              required
-              control={control}
-            />
-            <ListboxField
-              control={control}
-              name="image"
-              placeholder="Select an image"
-              items={imageItems}
-              isLoading={images.isPending}
-              required
-              disabled={!selectedProject}
-            />
-          </form>
-          <Message
-            variant="info"
-            content="Once an image has been promoted it is visible to all projects in a silo"
-          />
-        </Modal.Section>
-      </Modal.Body>
-      <Modal.Footer
-        onDismiss={onDismiss}
-        onAction={handleSubmit(onSubmit)}
-        actionText="Promote"
+    <ModalForm
+      title="Promote image"
+      form={form}
+      resourceName="Image"
+      loading={promoteImage.isPending}
+      submitError={promoteImage.error}
+      onSubmit={onSubmit}
+      onDismiss={onDismiss}
+      submitLabel="Promote"
+    >
+      <ComboboxField
+        placeholder="Select a project"
+        name="project"
+        label="Project"
+        items={projectItems}
+        onChange={() => {
+          form.resetField('image') // reset image field when the project changes
+        }}
+        isLoading={projects.isPending}
+        required
+        control={form.control}
       />
-    </Modal>
+      <ListboxField
+        control={form.control}
+        name="image"
+        placeholder="Select an image"
+        items={imageItems}
+        isLoading={images.isPending}
+        required
+        disabled={!selectedProject}
+      />
+      <Message
+        variant="info"
+        content="Once an image has been promoted it is visible to all projects in a silo"
+      />
+    </ModalForm>
   )
 }
 

From f591a2b1b85442efa9358a83f9f3ef6b799d667b Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Wed, 5 Mar 2025 16:25:27 -0600
Subject: [PATCH 09/16] clean up props a bit

---
 app/components/form/ModalForm.tsx     | 31 +++------------------------
 app/components/form/SideModalForm.tsx |  8 -------
 app/forms/disk-attach.tsx             |  1 -
 app/pages/SiloImagesPage.tsx          | 11 ++++------
 4 files changed, 7 insertions(+), 44 deletions(-)

diff --git a/app/components/form/ModalForm.tsx b/app/components/form/ModalForm.tsx
index 73fbe9cc32..284a824ebc 100644
--- a/app/components/form/ModalForm.tsx
+++ b/app/components/form/ModalForm.tsx
@@ -16,45 +16,29 @@ import { Modal, type ModalProps } from '~/ui/lib/Modal'
 
 type ModalFormProps<TFieldValues extends FieldValues> = {
   form: UseFormReturn<TFieldValues>
-  /**
-   * A function that returns the fields.
-   *
-   * Implemented as a function so we can pass `control` to the fields in the
-   * calling code. We could do that internally with `cloneElement` instead, but
-   * then in the calling code, the field would not infer `TFieldValues` and
-   * constrain the `name` prop to paths in the values object.
-   */
   children: ReactNode
-  resourceName: string
   /** Must be provided with a reason describing why it's disabled */
   submitDisabled?: string
-
+  onSubmit: (values: TFieldValues) => void
+  submitLabel: string
   // require loading and error so we can't forget to hook them up. there are a
   // few forms that don't need them, so we'll use dummy values
 
   /** Error from the API call */
   submitError: ApiError | null
   loading: boolean
-
-  /** Only needed if you need to override the default title (Create/Edit ${resourceName}) */
-  subtitle?: ReactNode
-  onSubmit: (values: TFieldValues) => void
-
-  submitLabel?: string
 } & Omit<ModalProps, 'isOpen'>
 
 export function ModalForm<TFieldValues extends FieldValues>({
   form,
   children,
   onDismiss,
-  resourceName,
   submitDisabled,
   submitError,
   title,
   onSubmit,
   submitLabel = 'Save',
   loading,
-  subtitle,
   width = 'medium',
   overlay = true,
 }: ModalFormProps<TFieldValues>) {
@@ -62,20 +46,11 @@ export function ModalForm<TFieldValues extends FieldValues>({
 
   const { isSubmitting } = form.formState
 
-  const modalTitle = title || `Create ${resourceName}`
-
   return (
     <>
-      <Modal
-        isOpen
-        onDismiss={onDismiss}
-        title={modalTitle}
-        width={width}
-        overlay={overlay}
-      >
+      <Modal isOpen onDismiss={onDismiss} title={title} width={width} overlay={overlay}>
         <Modal.Body>
           <Modal.Section>
-            {subtitle && <div className="mb-4">{subtitle}</div>}
             {submitError && (
               <Message variant="error" title="Error" content={submitError.message} />
             )}
diff --git a/app/components/form/SideModalForm.tsx b/app/components/form/SideModalForm.tsx
index 3e353052e6..5d8f15ba5e 100644
--- a/app/components/form/SideModalForm.tsx
+++ b/app/components/form/SideModalForm.tsx
@@ -30,14 +30,6 @@ type EditFormProps = {
 
 type SideModalFormProps<TFieldValues extends FieldValues> = {
   form: UseFormReturn<TFieldValues>
-  /**
-   * A function that returns the fields.
-   *
-   * Implemented as a function so we can pass `control` to the fields in the
-   * calling code. We could do that internally with `cloneElement` instead, but
-   * then in the calling code, the field would not infer `TFieldValues` and
-   * constrain the `name` prop to paths in the values object.
-   */
   children: ReactNode
   onDismiss: () => void
   resourceName: string
diff --git a/app/forms/disk-attach.tsx b/app/forms/disk-attach.tsx
index 9dd6a695f8..af2cc482c2 100644
--- a/app/forms/disk-attach.tsx
+++ b/app/forms/disk-attach.tsx
@@ -60,7 +60,6 @@ export function AttachDiskModalForm({
     <ModalForm
       form={form}
       onDismiss={onDismiss}
-      resourceName="disk"
       submitLabel="Attach disk"
       submitError={submitError}
       loading={loading}
diff --git a/app/pages/SiloImagesPage.tsx b/app/pages/SiloImagesPage.tsx
index 4a71dd538e..94267ebcd0 100644
--- a/app/pages/SiloImagesPage.tsx
+++ b/app/pages/SiloImagesPage.tsx
@@ -160,19 +160,16 @@ const PromoteImageModal = ({ onDismiss }: { onDismiss: () => void }) => {
     [images.data]
   )
 
-  const onSubmit = ({ image, project }: Values) => {
-    if (!image || !project) return
-    promoteImage.mutate({ path: { image } })
-  }
-
   return (
     <ModalForm
       title="Promote image"
       form={form}
-      resourceName="Image"
       loading={promoteImage.isPending}
       submitError={promoteImage.error}
-      onSubmit={onSubmit}
+      onSubmit={({ image, project }) => {
+        if (!image || !project) return // shouldn't happen because of validation
+        promoteImage.mutate({ path: { image } })
+      }}
       onDismiss={onDismiss}
       submitLabel="Promote"
     >

From 1cecbcc37c0d7525d5d0f8a44721c87dc3cd5997 Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Wed, 5 Mar 2025 16:44:37 -0600
Subject: [PATCH 10/16] convert image demote too

---
 app/pages/SiloImagesPage.tsx | 85 ++++++++++++++++--------------------
 1 file changed, 37 insertions(+), 48 deletions(-)

diff --git a/app/pages/SiloImagesPage.tsx b/app/pages/SiloImagesPage.tsx
index 94267ebcd0..24190341d0 100644
--- a/app/pages/SiloImagesPage.tsx
+++ b/app/pages/SiloImagesPage.tsx
@@ -7,7 +7,7 @@
  */
 import { createColumnHelper } from '@tanstack/react-table'
 import { useCallback, useMemo, useState } from 'react'
-import { useForm, type FieldValues } from 'react-hook-form'
+import { useForm } from 'react-hook-form'
 import { Outlet } from 'react-router'
 
 import {
@@ -36,7 +36,6 @@ import { Button } from '~/ui/lib/Button'
 import { toComboboxItems } from '~/ui/lib/Combobox'
 import { EmptyMessage } from '~/ui/lib/EmptyMessage'
 import { Message } from '~/ui/lib/Message'
-import { Modal } from '~/ui/lib/Modal'
 import { PageHeader, PageTitle } from '~/ui/lib/PageHeader'
 import { TableActions } from '~/ui/lib/Table'
 import { docLinks } from '~/util/links'
@@ -202,6 +201,10 @@ const PromoteImageModal = ({ onDismiss }: { onDismiss: () => void }) => {
   )
 }
 
+type DemoteFormValues = {
+  project: string | undefined
+}
+
 const DemoteImageModal = ({
   onDismiss,
   image,
@@ -209,20 +212,17 @@ const DemoteImageModal = ({
   onDismiss: () => void
   image: Image
 }) => {
-  const { control, handleSubmit, watch } = useForm()
+  const defaultValues: DemoteFormValues = { project: undefined }
+  const form = useForm({ defaultValues })
 
-  const selectedProject: string | undefined = watch('project')
+  const selectedProject: string | undefined = form.watch('project')
 
   const queryClient = useApiQueryClient()
 
   const demoteImage = useApiMutation('imageDemote', {
     onSuccess(data) {
       addToast({
-        content: (
-          <>
-            Image <HL>{data.name}</HL> demoted
-          </>
-        ),
+        content: <>Image <HL>{data.name}</HL> demoted</>, // prettier-ignore
         cta: selectedProject
           ? {
               text: `View images in ${selectedProject}`,
@@ -239,51 +239,40 @@ const DemoteImageModal = ({
     onSettled: onDismiss,
   })
 
-  const onSubmit = (data: FieldValues) => {
-    demoteImage.mutate({ path: { image: image.id }, query: { project: data.project } })
-  }
-
   const projects = useApiQuery('projectList', {})
   const projectItems = useMemo(() => toComboboxItems(projects.data?.items), [projects.data])
 
   return (
-    <Modal isOpen onDismiss={onDismiss} title="Demote image">
-      <Modal.Body>
-        <Modal.Section>
-          <form
-            autoComplete="off"
-            onSubmit={(e) => {
-              e.stopPropagation()
-              handleSubmit(onSubmit)(e)
-            }}
-            className="space-y-4"
-          >
-            <p>
-              Demoting: <span className="text-sans-semi-md text-raise">{image.name}</span>
-            </p>
+    <ModalForm
+      title="Demote image"
+      form={form}
+      loading={demoteImage.isPending}
+      submitError={demoteImage.error}
+      onSubmit={({ project }) => {
+        if (!project) return // shouldn't happen because of validation
+        demoteImage.mutate({ path: { image: image.id }, query: { project } })
+      }}
+      onDismiss={onDismiss}
+      submitLabel="Demote"
+    >
+      <p>
+        Demoting: <span className="text-sans-semi-md text-raise">{image.name}</span>
+      </p>
 
-            <Message
-              variant="info"
-              content="Once an image has been demoted it is only visible to the project that it is demoted into. This will not affect disks already created with the image."
-            />
+      <Message
+        variant="info"
+        content="Once an image has been demoted it is only visible to the project that it is demoted into. This will not affect disks already created with the image."
+      />
 
-            <ComboboxField
-              placeholder="Select project for image"
-              name="project"
-              label="Project"
-              items={projectItems}
-              isLoading={projects.isPending}
-              required
-              control={control}
-            />
-          </form>
-        </Modal.Section>
-      </Modal.Body>
-      <Modal.Footer
-        onDismiss={onDismiss}
-        onAction={handleSubmit(onSubmit)}
-        actionText="Demote"
+      <ComboboxField
+        placeholder="Select project for image"
+        name="project"
+        label="Project"
+        items={projectItems}
+        isLoading={projects.isPending}
+        required
+        control={form.control}
       />
-    </Modal>
+    </ModalForm>
   )
 }

From 1b568f984053055c30790dda58b68eead63eb0eb Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Wed, 5 Mar 2025 16:49:27 -0600
Subject: [PATCH 11/16] demotion notice copy tweak

---
 app/pages/SiloImagesPage.tsx | 2 +-
 test/e2e/images.e2e.ts       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/pages/SiloImagesPage.tsx b/app/pages/SiloImagesPage.tsx
index 24190341d0..30b3a565b5 100644
--- a/app/pages/SiloImagesPage.tsx
+++ b/app/pages/SiloImagesPage.tsx
@@ -261,7 +261,7 @@ const DemoteImageModal = ({
 
       <Message
         variant="info"
-        content="Once an image has been demoted it is only visible to the project that it is demoted into. This will not affect disks already created with the image."
+        content="Once an image has been demoted it is only visible within the project that it is demoted into. This will not affect disks already created with the image."
       />
 
       <ComboboxField
diff --git a/test/e2e/images.e2e.ts b/test/e2e/images.e2e.ts
index d78f83ca0a..4ab898f4f8 100644
--- a/test/e2e/images.e2e.ts
+++ b/test/e2e/images.e2e.ts
@@ -98,7 +98,7 @@ test('can demote an image from silo', async ({ page }) => {
   await clickRowAction(page, 'arch-2022-06-01', 'Demote')
 
   // Notice is visible
-  await expect(page.getByText('only visible to the project')).toBeVisible()
+  await expect(page.getByText('only visible within the project')).toBeVisible()
 
   // Correct image is selected
   await expect(page.getByText('Demoting: arch-2022-06-01')).toBeVisible()

From aa7ecdb09ea6a46264a8d77ff10e4f49aab9f2ac Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Wed, 5 Mar 2025 16:53:32 -0600
Subject: [PATCH 12/16] convert floating IP attach

---
 .../project/floating-ips/FloatingIpsPage.tsx  | 70 +++++++++----------
 1 file changed, 33 insertions(+), 37 deletions(-)

diff --git a/app/pages/project/floating-ips/FloatingIpsPage.tsx b/app/pages/project/floating-ips/FloatingIpsPage.tsx
index a7c5f4d4b2..b4a6caa74c 100644
--- a/app/pages/project/floating-ips/FloatingIpsPage.tsx
+++ b/app/pages/project/floating-ips/FloatingIpsPage.tsx
@@ -23,6 +23,7 @@ import { IpGlobal16Icon, IpGlobal24Icon } from '@oxide/design-system/icons/react
 
 import { DocsPopover } from '~/components/DocsPopover'
 import { ListboxField } from '~/components/form/fields/ListboxField'
+import { ModalForm } from '~/components/form/ModalForm'
 import { HL } from '~/components/HL'
 import { makeCrumb } from '~/hooks/use-crumbs'
 import { getProjectSelector, useProjectSelector } from '~/hooks/use-params'
@@ -38,7 +39,6 @@ import { CopyableIp } from '~/ui/lib/CopyableIp'
 import { CreateLink } from '~/ui/lib/CreateButton'
 import { EmptyMessage } from '~/ui/lib/EmptyMessage'
 import { Message } from '~/ui/lib/Message'
-import { Modal } from '~/ui/lib/Modal'
 import { PageHeader, PageTitle } from '~/ui/lib/PageHeader'
 import { TableActions } from '~/ui/lib/Table'
 import { ALL_ISH } from '~/util/consts'
@@ -258,45 +258,41 @@ const AttachFloatingIpModal = ({
       addToast({ title: 'Error', content: err.message, variant: 'error' })
     },
   })
+
   const form = useForm({ defaultValues: { instanceId: '' } })
-  const instanceId = form.watch('instanceId')
 
   return (
-    <Modal isOpen title="Attach floating IP" onDismiss={onDismiss}>
-      <Modal.Body>
-        <Modal.Section>
-          <Message
-            variant="info"
-            content={
-              <>
-                The selected instance will be reachable at <HL>{address}</HL>
-              </>
-            }
-          />
-          <form>
-            <ListboxField
-              control={form.control}
-              name="instanceId"
-              items={instances.map((i) => ({ value: i.id, label: i.name }))}
-              label="Instance"
-              required
-              placeholder="Select an instance"
-            />
-          </form>
-        </Modal.Section>
-      </Modal.Body>
-      <Modal.Footer
-        actionText="Attach"
-        disabled={!instanceId}
-        onAction={() =>
-          floatingIpAttach.mutate({
-            path: { floatingIp },
-            query: { project },
-            body: { kind: 'instance', parent: instanceId },
-          })
+    <ModalForm
+      title="Attach floating IP"
+      form={form}
+      onSubmit={({ instanceId }) => {
+        floatingIpAttach.mutate({
+          path: { floatingIp },
+          query: { project },
+          body: { kind: 'instance', parent: instanceId },
+        })
+      }}
+      submitLabel="Attach"
+      submitError={floatingIpAttach.error}
+      loading={floatingIpAttach.isPending}
+      onDismiss={onDismiss}
+    >
+      <Message
+        variant="info"
+        content={
+          <>
+            The selected instance will be reachable at <HL>{address}</HL>
+          </>
         }
-        onDismiss={onDismiss}
-      ></Modal.Footer>
-    </Modal>
+      />
+      <ListboxField
+        control={form.control}
+        name="instanceId"
+        items={instances.map((i) => ({ value: i.id, label: i.name }))}
+        label="Instance"
+        required
+        placeholder="Select an instance"
+      />
+    </ModalForm>
   )
 }

From f17a1ac9a29675938f54c0666de726b100732bd2 Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Wed, 5 Mar 2025 17:01:34 -0600
Subject: [PATCH 13/16] remove unnecessary fragment

---
 app/components/form/ModalForm.tsx | 70 +++++++++++++++----------------
 1 file changed, 33 insertions(+), 37 deletions(-)

diff --git a/app/components/form/ModalForm.tsx b/app/components/form/ModalForm.tsx
index 284a824ebc..26be149123 100644
--- a/app/components/form/ModalForm.tsx
+++ b/app/components/form/ModalForm.tsx
@@ -43,44 +43,40 @@ export function ModalForm<TFieldValues extends FieldValues>({
   overlay = true,
 }: ModalFormProps<TFieldValues>) {
   const id = useId()
-
   const { isSubmitting } = form.formState
-
   return (
-    <>
-      <Modal isOpen onDismiss={onDismiss} title={title} width={width} overlay={overlay}>
-        <Modal.Body>
-          <Modal.Section>
-            {submitError && (
-              <Message variant="error" title="Error" content={submitError.message} />
-            )}
-            <form
-              id={id}
-              className="ox-form"
-              autoComplete="off"
-              onSubmit={(e) => {
-                if (!onSubmit) return
-                // This modal being in a portal doesn't prevent the submit event
-                // from bubbling up out of the portal. Normally that's not a
-                // problem, but sometimes (e.g., instance create) we render the
-                // SideModalForm from inside another form, in which case submitting
-                // the inner form submits the outer form unless we stop propagation
-                e.stopPropagation()
-                form.handleSubmit(onSubmit)(e)
-              }}
-            >
-              {children}
-            </form>
-          </Modal.Section>
-        </Modal.Body>
-        <Modal.Footer
-          onDismiss={onDismiss}
-          formId={id}
-          actionText={submitLabel}
-          disabled={!!submitDisabled}
-          actionLoading={loading || isSubmitting}
-        />
-      </Modal>
-    </>
+    <Modal isOpen onDismiss={onDismiss} title={title} width={width} overlay={overlay}>
+      <Modal.Body>
+        <Modal.Section>
+          {submitError && (
+            <Message variant="error" title="Error" content={submitError.message} />
+          )}
+          <form
+            id={id}
+            className="ox-form"
+            autoComplete="off"
+            onSubmit={(e) => {
+              if (!onSubmit) return
+              // This modal being in a portal doesn't prevent the submit event
+              // from bubbling up out of the portal. Normally that's not a
+              // problem, but sometimes (e.g., instance create) we render the
+              // SideModalForm from inside another form, in which case submitting
+              // the inner form submits the outer form unless we stop propagation
+              e.stopPropagation()
+              form.handleSubmit(onSubmit)(e)
+            }}
+          >
+            {children}
+          </form>
+        </Modal.Section>
+      </Modal.Body>
+      <Modal.Footer
+        onDismiss={onDismiss}
+        formId={id}
+        actionText={submitLabel}
+        disabled={!!submitDisabled}
+        actionLoading={loading || isSubmitting}
+      />
+    </Modal>
   )
 }

From 2f722387531b96a861facaf9c6cd1ade1cff3210 Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Wed, 5 Mar 2025 17:33:42 -0600
Subject: [PATCH 14/16] simplify some of the instance state business

---
 app/components/StopInstancePrompt.tsx      | 72 ++++------------------
 app/forms/disk-attach.tsx                  | 14 +++--
 app/pages/project/instances/StorageTab.tsx |  1 +
 3 files changed, 23 insertions(+), 64 deletions(-)

diff --git a/app/components/StopInstancePrompt.tsx b/app/components/StopInstancePrompt.tsx
index 54d129f44f..162bb1d9fc 100644
--- a/app/components/StopInstancePrompt.tsx
+++ b/app/components/StopInstancePrompt.tsx
@@ -5,52 +5,28 @@
  *
  * Copyright Oxide Computer Company
  */
-import { useEffect, useState, type ReactNode } from 'react'
+import { type ReactNode } from 'react'
 
-import {
-  instanceTransitioning,
-  useApiMutation,
-  useApiQuery,
-  useApiQueryClient,
-  type Instance,
-} from '@oxide/api'
+import { apiQueryClient, useApiMutation, type Instance } from '@oxide/api'
 
 import { HL } from '~/components/HL'
 import { addToast } from '~/stores/toast'
 import { Button } from '~/ui/lib/Button'
 import { Message } from '~/ui/lib/Message'
 
-const POLL_INTERVAL_FAST = 2000 // 2 seconds
-
 type StopInstancePromptProps = {
   instance: Instance
   children: ReactNode
 }
 
 export function StopInstancePrompt({ instance, children }: StopInstancePromptProps) {
-  const queryClient = useApiQueryClient()
-  const [isStoppingInstance, setIsStoppingInstance] = useState(false)
+  const isStoppingInstance = instance.runState === 'stopping'
 
-  const { data } = useApiQuery(
-    'instanceView',
-    {
-      path: { instance: instance.name },
-      query: { project: instance.projectId },
-    },
-    {
-      refetchInterval:
-        isStoppingInstance || instanceTransitioning(instance) ? POLL_INTERVAL_FAST : false,
-    }
-  )
-
-  const { mutateAsync: stopInstanceAsync } = useApiMutation('instanceStop', {
+  const stopInstance = useApiMutation('instanceStop', {
     onSuccess: () => {
-      setIsStoppingInstance(true)
-      addToast(
-        <>
-          Stopping instance <HL>{instance.name}</HL>
-        </>
-      )
+      // trigger polling by the top level InstancePage one
+      apiQueryClient.invalidateQueries('instanceView')
+      addToast(<>Stopping instance <HL>{instance.name}</HL></>) // prettier-ignore
     },
     onError: (error) => {
       addToast({
@@ -58,36 +34,9 @@ export function StopInstancePrompt({ instance, children }: StopInstancePromptPro
         title: `Error stopping instance '${instance.name}'`,
         content: error.message,
       })
-      setIsStoppingInstance(false)
     },
   })
 
-  const handleStopInstance = () => {
-    stopInstanceAsync({
-      path: { instance: instance.name },
-      query: { project: instance.projectId },
-    })
-  }
-
-  const currentInstance = data || instance
-
-  useEffect(() => {
-    if (!data) {
-      return
-    }
-    if (isStoppingInstance && data.runState === 'stopped') {
-      queryClient.invalidateQueries('instanceView')
-      setIsStoppingInstance(false)
-    }
-  }, [isStoppingInstance, data, queryClient])
-
-  if (
-    !currentInstance ||
-    (currentInstance.runState !== 'stopping' && currentInstance.runState !== 'running')
-  ) {
-    return null
-  }
-
   return (
     <Message
       variant="notice"
@@ -98,7 +47,12 @@ export function StopInstancePrompt({ instance, children }: StopInstancePromptPro
             size="xs"
             className="mt-3"
             variant="notice"
-            onClick={handleStopInstance}
+            onClick={() =>
+              stopInstance.mutateAsync({
+                path: { instance: instance.name },
+                query: { project: instance.projectId },
+              })
+            }
             loading={isStoppingInstance}
           >
             Stop instance
diff --git a/app/forms/disk-attach.tsx b/app/forms/disk-attach.tsx
index cfb2facea4..53b199ba14 100644
--- a/app/forms/disk-attach.tsx
+++ b/app/forms/disk-attach.tsx
@@ -26,7 +26,7 @@ type AttachDiskProps = {
   diskNamesToExclude?: string[]
   loading?: boolean
   submitError?: ApiError | null
-  instance: Instance
+  instance?: Instance
 }
 
 /**
@@ -70,12 +70,16 @@ export function AttachDiskModalForm({
       onSubmit={onSubmit}
       width="medium"
       submitDisabled={
-        !instanceCan.attachDisk(instance) ? 'Instance must be stopped' : undefined
+        instance && !instanceCan.attachDisk(instance)
+          ? 'Instance must be stopped'
+          : undefined
       }
     >
-      <StopInstancePrompt instance={instance}>
-        An instance must be stopped to attach a disk.
-      </StopInstancePrompt>
+      {instance && ['stopping', 'running'].includes(instance.runState) && (
+        <StopInstancePrompt instance={instance}>
+          An instance must be stopped to attach a disk.
+        </StopInstancePrompt>
+      )}
       <ComboboxField
         label="Disk name"
         placeholder="Select a disk"
diff --git a/app/pages/project/instances/StorageTab.tsx b/app/pages/project/instances/StorageTab.tsx
index e86ccd2d63..a1090c0bea 100644
--- a/app/pages/project/instances/StorageTab.tsx
+++ b/app/pages/project/instances/StorageTab.tsx
@@ -382,6 +382,7 @@ export default function StorageTab() {
           }}
           loading={attachDisk.isPending}
           submitError={attachDisk.error}
+          instance={instance}
         />
       )}
     </div>

From 39ac93a4f0b80a7f457b88656bc60330923e6dba Mon Sep 17 00:00:00 2001
From: Benjamin Leonard <benji@oxide.computer>
Date: Thu, 6 Mar 2025 10:28:48 +0000
Subject: [PATCH 15/16] Attach floating IP modal and prop tweaks

---
 app/components/AttachFloatingIpModal.tsx | 72 ++++++++++++------------
 app/components/form/ModalForm.tsx        |  9 ++-
 app/ui/lib/Modal.tsx                     |  4 +-
 3 files changed, 44 insertions(+), 41 deletions(-)

diff --git a/app/components/AttachFloatingIpModal.tsx b/app/components/AttachFloatingIpModal.tsx
index 87fa708b71..a2022fe94c 100644
--- a/app/components/AttachFloatingIpModal.tsx
+++ b/app/components/AttachFloatingIpModal.tsx
@@ -13,9 +13,10 @@ import { ListboxField } from '~/components/form/fields/ListboxField'
 import { HL } from '~/components/HL'
 import { addToast } from '~/stores/toast'
 import { Message } from '~/ui/lib/Message'
-import { Modal } from '~/ui/lib/Modal'
 import { Slash } from '~/ui/lib/Slash'
 
+import { ModalForm } from './form/ModalForm'
+
 function FloatingIpLabel({ fip }: { fip: FloatingIp }) {
   return (
     <div className="text-secondary selected:text-accent-secondary">
@@ -60,40 +61,39 @@ export const AttachFloatingIpModal = ({
   const floatingIp = form.watch('floatingIp')
 
   return (
-    <Modal isOpen title="Attach floating IP" onDismiss={onDismiss}>
-      <Modal.Body>
-        <Modal.Section>
-          <Message
-            variant="info"
-            content={`Instance ‘${instance.name}’ will be reachable at the selected IP address`}
-          />
-          <form>
-            <ListboxField
-              control={form.control}
-              name="floatingIp"
-              label="Floating IP"
-              placeholder="Select a floating IP"
-              items={floatingIps.map((ip) => ({
-                value: ip.id,
-                label: <FloatingIpLabel fip={ip} />,
-                selectedLabel: ip.name,
-              }))}
-              required
-            />
-          </form>
-        </Modal.Section>
-      </Modal.Body>
-      <Modal.Footer
-        actionText="Attach"
-        disabled={!floatingIp}
-        onAction={() =>
-          floatingIpAttach.mutate({
-            path: { floatingIp }, // note that this is an ID!
-            body: { kind: 'instance', parent: instance.id },
-          })
-        }
-        onDismiss={onDismiss}
-      ></Modal.Footer>
-    </Modal>
+    <ModalForm
+      form={form}
+      onDismiss={onDismiss}
+      submitLabel="Attach floating IP"
+      submitError={floatingIpAttach.error}
+      loading={floatingIpAttach.isPending}
+      title="Attach floating IP"
+      onSubmit={() =>
+        floatingIpAttach.mutate({
+          path: { floatingIp }, // note that this is an ID!
+          body: { kind: 'instance', parent: instance.id },
+        })
+      }
+      submitDisabled={!floatingIp}
+    >
+      <Message
+        variant="info"
+        content={`Instance ‘${instance.name}’ will be reachable at the selected IP address`}
+      />
+      <form>
+        <ListboxField
+          control={form.control}
+          name="floatingIp"
+          label="Floating IP"
+          placeholder="Select a floating IP"
+          items={floatingIps.map((ip) => ({
+            value: ip.id,
+            label: <FloatingIpLabel fip={ip} />,
+            selectedLabel: ip.name,
+          }))}
+          required
+        />
+      </form>
+    </ModalForm>
   )
 }
diff --git a/app/components/form/ModalForm.tsx b/app/components/form/ModalForm.tsx
index 26be149123..be08971967 100644
--- a/app/components/form/ModalForm.tsx
+++ b/app/components/form/ModalForm.tsx
@@ -17,10 +17,12 @@ import { Modal, type ModalProps } from '~/ui/lib/Modal'
 type ModalFormProps<TFieldValues extends FieldValues> = {
   form: UseFormReturn<TFieldValues>
   children: ReactNode
+
   /** Must be provided with a reason describing why it's disabled */
-  submitDisabled?: string
+  submitDisabled?: boolean
   onSubmit: (values: TFieldValues) => void
   submitLabel: string
+
   // require loading and error so we can't forget to hook them up. there are a
   // few forms that don't need them, so we'll use dummy values
 
@@ -33,7 +35,7 @@ export function ModalForm<TFieldValues extends FieldValues>({
   form,
   children,
   onDismiss,
-  submitDisabled,
+  submitDisabled = false,
   submitError,
   title,
   onSubmit,
@@ -44,6 +46,7 @@ export function ModalForm<TFieldValues extends FieldValues>({
 }: ModalFormProps<TFieldValues>) {
   const id = useId()
   const { isSubmitting } = form.formState
+
   return (
     <Modal isOpen onDismiss={onDismiss} title={title} width={width} overlay={overlay}>
       <Modal.Body>
@@ -74,7 +77,7 @@ export function ModalForm<TFieldValues extends FieldValues>({
         onDismiss={onDismiss}
         formId={id}
         actionText={submitLabel}
-        disabled={!!submitDisabled}
+        disabled={submitDisabled}
         actionLoading={loading || isSubmitting}
       />
     </Modal>
diff --git a/app/ui/lib/Modal.tsx b/app/ui/lib/Modal.tsx
index ba48886dde..9247c40b7b 100644
--- a/app/ui/lib/Modal.tsx
+++ b/app/ui/lib/Modal.tsx
@@ -123,7 +123,7 @@ Modal.Footer = ({
   actionText,
   actionLoading,
   cancelText,
-  disabled = false,
+  disabled,
   formId,
 }: FooterProps) => (
   <footer className="flex items-center justify-between border-t px-4 py-3 border-secondary">
@@ -138,7 +138,7 @@ Modal.Footer = ({
         size="sm"
         variant={actionType}
         onClick={onAction}
-        disabled={disabled}
+        disabled={!!disabled}
         loading={actionLoading}
       >
         {actionText}

From 5ba0578def7ab2d440abee8cf361bcb7ebaed7ab Mon Sep 17 00:00:00 2001
From: Benjamin Leonard <benji@oxide.computer>
Date: Thu, 6 Mar 2025 10:58:41 +0000
Subject: [PATCH 16/16] Update test

---
 test/e2e/click-everything.e2e.ts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/test/e2e/click-everything.e2e.ts b/test/e2e/click-everything.e2e.ts
index e2ead76cc8..46fe610d49 100644
--- a/test/e2e/click-everything.e2e.ts
+++ b/test/e2e/click-everything.e2e.ts
@@ -24,7 +24,6 @@ test('Click through instance page', async ({ page }) => {
     'role=cell[name="disk-2"]',
     // buttons disabled while instance is running
     'role=button[name="Create disk"][disabled]',
-    'role=button[name="Attach existing disk"][disabled]',
     // TODO: assert minitable contents
   ])
   await expectNotVisible(page, ['role=cell[name="disk-3"]'])