From 4bfadc023b6a299062f79707dfe9287c945e813e Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 31 Jan 2024 00:21:34 -0600 Subject: [PATCH] Abstract confirm-delete into more general confirm-action (#1927) * abstract confirm-delete into more general confirm-action * fix double error toast on instance delete --- ...DeleteModal.tsx => ConfirmActionModal.tsx} | 32 ++++++-------- app/main.tsx | 4 +- app/pages/SiloAccessPage.tsx | 3 +- .../project/access/ProjectAccessPage.tsx | 3 +- app/pages/project/instances/actions.tsx | 6 --- .../{confirm-delete.ts => confirm-action.ts} | 33 +++++++-------- app/stores/confirm-delete.tsx | 42 +++++++++++++++++++ 7 files changed, 73 insertions(+), 50 deletions(-) rename app/components/{ConfirmDeleteModal.tsx => ConfirmActionModal.tsx} (58%) rename app/stores/{confirm-delete.ts => confirm-action.ts} (57%) create mode 100644 app/stores/confirm-delete.tsx diff --git a/app/components/ConfirmDeleteModal.tsx b/app/components/ConfirmActionModal.tsx similarity index 58% rename from app/components/ConfirmDeleteModal.tsx rename to app/components/ConfirmActionModal.tsx index 23670dfaa..f1fb56b90 100644 --- a/app/components/ConfirmDeleteModal.tsx +++ b/app/components/ConfirmActionModal.tsx @@ -8,16 +8,13 @@ import { useState } from 'react' import { type ApiError } from '@oxide/api' -import { Message, Modal } from '@oxide/ui' -import { classed } from '@oxide/util' +import { Modal } from '@oxide/ui' -import { clearConfirmDelete, useConfirmDelete } from 'app/stores/confirm-delete' +import { clearConfirmAction, useConfirmAction } from 'app/stores/confirm-action' import { addToast } from 'app/stores/toast' -export const HL = classed.span`text-sans-semi-md text-default` - -export function ConfirmDeleteModal() { - const deleteConfig = useConfirmDelete((state) => state.deleteConfig) +export function ConfirmActionModal() { + const actionConfig = useConfirmAction((state) => state.actionConfig) // 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 @@ -25,28 +22,23 @@ export function ConfirmDeleteModal() { // loading state changes const [loading, setLoading] = useState(false) - if (!deleteConfig) return null - - const { doDelete, warning, label } = deleteConfig + if (!actionConfig) return null - const displayLabel = typeof label === 'string' ? {label} : label + const { doAction, modalContent, errorTitle, modalTitle } = actionConfig return ( - - -

Are you sure you want to delete {displayLabel}?

- {warning && } - + + {modalContent} { setLoading(true) try { - await doDelete() + await doAction() } catch (error) { addToast({ variant: 'error', - title: 'Could not delete resource', + title: errorTitle, content: (error as ApiError).message, }) } @@ -54,7 +46,7 @@ export function ConfirmDeleteModal() { setLoading(false) // do this regardless of success or error // TODO: generic success toast? - clearConfirmDelete() + clearConfirmAction() }} cancelText="Cancel" actionText="Confirm" diff --git a/app/main.tsx b/app/main.tsx index e93384135..4cc71bf5c 100644 --- a/app/main.tsx +++ b/app/main.tsx @@ -14,7 +14,7 @@ import { createBrowserRouter, RouterProvider } from 'react-router-dom' import { queryClient } from '@oxide/api' import { SkipLink } from '@oxide/ui' -import { ConfirmDeleteModal } from './components/ConfirmDeleteModal' +import { ConfirmActionModal } from './components/ConfirmActionModal' import { ErrorBoundary } from './components/ErrorBoundary' import { ReduceMotion } from './hooks' // stripped out by rollup in production @@ -45,7 +45,7 @@ function render() { - + diff --git a/app/pages/SiloAccessPage.tsx b/app/pages/SiloAccessPage.tsx index 5ae95a1d3..7e6c83081 100644 --- a/app/pages/SiloAccessPage.tsx +++ b/app/pages/SiloAccessPage.tsx @@ -32,13 +32,12 @@ import { import { groupBy, isTruthy } from '@oxide/util' import { AccessNameCell } from 'app/components/AccessNameCell' -import { HL } from 'app/components/ConfirmDeleteModal' import { RoleBadgeCell } from 'app/components/RoleBadgeCell' import { SiloAccessAddUserSideModal, SiloAccessEditUserSideModal, } from 'app/forms/silo-access' -import { confirmDelete } from 'app/stores/confirm-delete' +import { confirmDelete, HL } from 'app/stores/confirm-delete' const EmptyState = ({ onClick }: { onClick: () => void }) => ( diff --git a/app/pages/project/access/ProjectAccessPage.tsx b/app/pages/project/access/ProjectAccessPage.tsx index 07a9d9929..1026a5bdd 100644 --- a/app/pages/project/access/ProjectAccessPage.tsx +++ b/app/pages/project/access/ProjectAccessPage.tsx @@ -35,14 +35,13 @@ import { import { groupBy, isTruthy } from '@oxide/util' import { AccessNameCell } from 'app/components/AccessNameCell' -import { HL } from 'app/components/ConfirmDeleteModal' import { RoleBadgeCell } from 'app/components/RoleBadgeCell' import { ProjectAccessAddUserSideModal, ProjectAccessEditUserSideModal, } from 'app/forms/project-access' import { getProjectSelector, useProjectSelector } from 'app/hooks' -import { confirmDelete } from 'app/stores/confirm-delete' +import { confirmDelete, HL } from 'app/stores/confirm-delete' const EmptyState = ({ onClick }: { onClick: () => void }) => ( diff --git a/app/pages/project/instances/actions.tsx b/app/pages/project/instances/actions.tsx index c2ee1d3ab..a98eb4324 100644 --- a/app/pages/project/instances/actions.tsx +++ b/app/pages/project/instances/actions.tsx @@ -111,12 +111,6 @@ export const useMakeInstanceActions = ( options.onDelete?.() addToast({ title: `Deleting instance '${instance.name}'` }) }, - onError: (error) => - addToast({ - variant: 'error', - title: `Error deleting instance '${instance.name}'`, - content: error.message, - }), }), label: instance.name, }), diff --git a/app/stores/confirm-delete.ts b/app/stores/confirm-action.ts similarity index 57% rename from app/stores/confirm-delete.ts rename to app/stores/confirm-action.ts index 06d0c3cad..642a2d87c 100644 --- a/app/stores/confirm-delete.ts +++ b/app/stores/confirm-action.ts @@ -8,25 +8,22 @@ import type { ReactNode } from 'react' import { create } from 'zustand' -type DeleteConfig = { +type ActionConfig = { /** Must be `mutateAsync`, otherwise we can't catch the error generically */ - doDelete: () => Promise - warning?: string - /** - * Label identifying the resource. Could be a name or something more elaborate - * "the Admin role for user Harry Styles". If a string, the modal will - * automatically give it a highlighted style. Otherwise it will be rendered - * directly. - */ - label: ReactNode + doAction: () => Promise + /** e.g., Confirm delete, Confirm unlink */ + modalTitle: string + modalContent: ReactNode + /** Title of error toast */ + errorTitle: string } -type ConfirmDeleteStore = { - deleteConfig: DeleteConfig | null +type ConfirmActionStore = { + actionConfig: ActionConfig | null } -export const useConfirmDelete = create(() => ({ - deleteConfig: null, +export const useConfirmAction = create(() => ({ + actionConfig: null, })) // zustand docs say this pattern is equivalent to putting the actions on the @@ -39,10 +36,10 @@ export const useConfirmDelete = create(() => ({ /** * Note that this returns a function so we can save a line in the calling code. */ -export const confirmDelete = (deleteConfig: DeleteConfig) => () => { - useConfirmDelete.setState({ deleteConfig }) +export const confirmAction = (actionConfig: ActionConfig) => () => { + useConfirmAction.setState({ actionConfig }) } -export function clearConfirmDelete() { - useConfirmDelete.setState({ deleteConfig: null }) +export function clearConfirmAction() { + useConfirmAction.setState({ actionConfig: null }) } diff --git a/app/stores/confirm-delete.tsx b/app/stores/confirm-delete.tsx new file mode 100644 index 000000000..fd2327157 --- /dev/null +++ b/app/stores/confirm-delete.tsx @@ -0,0 +1,42 @@ +/* + * 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 { classed } from '@oxide/util' + +import { useConfirmAction } from './confirm-action' + +// confirmAction was originally abstracted from confirmDelete. this preserves +// the existing confirmDelete API by constructing a confirmAction from it + +type DeleteConfig = { + /** Must be `mutateAsync`, otherwise we can't catch the error generically */ + doDelete: () => Promise + /** + * Label identifying the resource. Could be a name or something more elaborate + * "the Admin role for user Harry Styles". If a string, the modal will + * automatically give it a highlighted style. Otherwise it will be rendered + * directly. + */ + label: React.ReactNode +} + +export const HL = classed.span`text-sans-semi-md text-default` + +export const confirmDelete = + ({ doDelete, label }: DeleteConfig) => + () => { + const displayLabel = typeof label === 'string' ? {label} : label + useConfirmAction.setState({ + actionConfig: { + doAction: doDelete, + modalContent:

Are you sure you want to delete {displayLabel}?

, + errorTitle: 'Could not delete resource', + modalTitle: 'Confirm delete', + }, + }) + }