Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions app/components/ConfirmDeleteModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -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
Expand All @@ -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>
)
Expand Down
4 changes: 4 additions & 0 deletions app/test/e2e/images.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
4 changes: 4 additions & 0 deletions app/test/e2e/snapshots.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
10 changes: 9 additions & 1 deletion libs/ui/lib/modal/Modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ Modal.Footer = ({
onAction,
actionType = 'primary',
actionText,
actionLoading,
cancelText,
disabled = false,
}: {
Expand All @@ -126,6 +127,7 @@ Modal.Footer = ({
onAction: () => void
actionType?: 'primary' | 'danger'
actionText: React.ReactNode
actionLoading?: boolean
cancelText?: string
disabled?: boolean
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding even more props here to pass through to the children is a clear sign this should be refactored into a compositional API.

}) => (
Expand All @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion libs/ui/lib/spinner/Spinner.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down