From b8d3836e8406a40e4997ee1b1704d572a21bd0b1 Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Tue, 5 Sep 2023 14:26:15 -0500
Subject: [PATCH] add loading state to ok button in confirm delete modal

---
 app/components/ConfirmDeleteModal.tsx | 22 +++++++++++++++++++---
 app/test/e2e/images.e2e.ts            |  4 ++++
 app/test/e2e/snapshots.e2e.ts         |  4 ++++
 libs/ui/lib/modal/Modal.tsx           | 10 +++++++++-
 libs/ui/lib/spinner/Spinner.tsx       |  2 +-
 5 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/app/components/ConfirmDeleteModal.tsx b/app/components/ConfirmDeleteModal.tsx
index 5a5fa51600..23670dfaa3 100644
--- a/app/components/ConfirmDeleteModal.tsx
+++ b/app/components/ConfirmDeleteModal.tsx
@@ -5,6 +5,9 @@
  *
  * Copyright Oxide Computer Company
  */
+import { useState } from 'react'
+
+import { type ApiError } from '@oxide/api'
 import { Message, Modal } from '@oxide/ui'
 import { classed } from '@oxide/util'
 
@@ -16,6 +19,12 @@ export const HL = classed.span`text-sans-semi-md text-default`
 export function ConfirmDeleteModal() {
   const deleteConfig = useConfirmDelete((state) => state.deleteConfig)
 
+  // this is a bit sad -- ideally we would be able to use the loading state
+  // from the mutation directly, but that would require a lot of line changes
+  // and would require us to hook this up in a way that re-renders whenever the
+  // loading state changes
+  const [loading, setLoading] = useState(false)
+
   if (!deleteConfig) return null
 
   const { doDelete, warning, label } = deleteConfig
@@ -31,19 +40,26 @@ export function ConfirmDeleteModal() {
       <Modal.Footer
         onDismiss={clearConfirmDelete}
         onAction={async () => {
-          await doDelete().catch((error) =>
+          setLoading(true)
+          try {
+            await doDelete()
+          } catch (error) {
             addToast({
               variant: 'error',
               title: 'Could not delete resource',
-              content: error.message,
+              content: (error as ApiError).message,
             })
-          )
+          }
+
+          setLoading(false) // do this regardless of success or error
+
           // TODO: generic success toast?
           clearConfirmDelete()
         }}
         cancelText="Cancel"
         actionText="Confirm"
         actionType="danger"
+        actionLoading={loading}
       />
     </Modal>
   )
diff --git a/app/test/e2e/images.e2e.ts b/app/test/e2e/images.e2e.ts
index 79c3076e13..12baef4b8b 100644
--- a/app/test/e2e/images.e2e.ts
+++ b/app/test/e2e/images.e2e.ts
@@ -125,9 +125,13 @@ test('can delete an image from a project', async ({ page }) => {
   await page.goto('/projects/mock-project/images')
 
   await clickRowAction(page, 'image-3', 'Delete')
+  const spinner = page.getByRole('dialog').getByLabel('Spinner')
+  await expect(spinner).toBeHidden()
   await page.getByRole('button', { name: 'Confirm' }).click()
+  await expect(spinner).toBeVisible()
 
   // Check deletion was successful
   await expectVisible(page, ['text="image-3 has been deleted"'])
   await expectNotVisible(page, ['role=cell[name="image-3"]'])
+  await expect(spinner).toBeHidden()
 })
diff --git a/app/test/e2e/snapshots.e2e.ts b/app/test/e2e/snapshots.e2e.ts
index ac0dd82bac..0efcc96ca0 100644
--- a/app/test/e2e/snapshots.e2e.ts
+++ b/app/test/e2e/snapshots.e2e.ts
@@ -63,7 +63,11 @@ test('Error on delete snapshot', async ({ page }) => {
   const modal = page.getByRole('dialog', { name: 'Confirm delete' })
   await expect(modal).toBeVisible()
 
+  const spinner = page.getByRole('dialog').getByLabel('Spinner')
+  await expect(spinner).toBeHidden()
+
   await page.getByRole('button', { name: 'Confirm' }).click()
+  await expect(spinner).toBeVisible()
 
   // modal closes, but row is not gone and error toast is visible
   await expect(modal).toBeHidden()
diff --git a/libs/ui/lib/modal/Modal.tsx b/libs/ui/lib/modal/Modal.tsx
index 285d436235..657561e6ad 100644
--- a/libs/ui/lib/modal/Modal.tsx
+++ b/libs/ui/lib/modal/Modal.tsx
@@ -118,6 +118,7 @@ Modal.Footer = ({
   onAction,
   actionType = 'primary',
   actionText,
+  actionLoading,
   cancelText,
   disabled = false,
 }: {
@@ -126,6 +127,7 @@ Modal.Footer = ({
   onAction: () => void
   actionType?: 'primary' | 'danger'
   actionText: React.ReactNode
+  actionLoading?: boolean
   cancelText?: string
   disabled?: boolean
 }) => (
@@ -135,7 +137,13 @@ Modal.Footer = ({
       <Button variant="secondary" size="sm" onClick={onDismiss}>
         {cancelText || 'Cancel'}
       </Button>
-      <Button size="sm" variant={actionType} onClick={onAction} disabled={disabled}>
+      <Button
+        size="sm"
+        variant={actionType}
+        onClick={onAction}
+        disabled={disabled}
+        loading={actionLoading}
+      >
         {actionText}
       </Button>
     </div>
diff --git a/libs/ui/lib/spinner/Spinner.tsx b/libs/ui/lib/spinner/Spinner.tsx
index 12a845f2c8..8d569c6103 100644
--- a/libs/ui/lib/spinner/Spinner.tsx
+++ b/libs/ui/lib/spinner/Spinner.tsx
@@ -38,7 +38,7 @@ export const Spinner = ({
       viewBox={`0 0 ${frameSize + ' ' + frameSize}`}
       fill="none"
       xmlns="http://www.w3.org/2000/svg"
-      aria-labelledby="Spinner"
+      aria-label="Spinner"
       className={cn('spinner', `spinner-${variant}`, `spinner-${size}`, className)}
     >
       <circle