Skip to content

Commit

Permalink
✨ Show the name of objects to be deleted in the confirm delete modals (
Browse files Browse the repository at this point in the history
…#1347)

Applies the pattern from
#1333 to the rest of our
entity delete confirmation modals.

Note that in order to do this, I needed to refactor away an inconsistent
pattern: Some of our delete modals were kept track of with
`thingIdToDelete` as a number in state and `isConfirmDialogOpen` as a
boolean. In order to show the name in the confirm modal, this PR
refactors these incidences to store a reference to the whole object
`thingToDelete` instead of just the id. The `isConfirmDialogOpen` state
was also redundant because the confirm modal is always closed when
`thingToDelete` is falsy and open when it's truthy, so this PR also
removes those booleans from state and uses `thingToDelete` as the source
of truth.

---------

Signed-off-by: Mike Turley <mike.turley@alum.cs.umass.edu>
Co-authored-by: Ian Bolton <ibolton@redhat.com>
  • Loading branch information
mturley and ibolton336 committed Sep 11, 2023
1 parent 3e7ede2 commit ccafe8d
Show file tree
Hide file tree
Showing 14 changed files with 156 additions and 171 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
TextInput,
} from "@patternfly/react-core";

import "./ConfirmDeleteCatalog.css";
import "./ConfirmDeleteDialog.css";

type ConfirmDeleteDialogProps = {
cancelBtnLabel?: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -648,17 +648,26 @@ export const ApplicationsTableAnalyze: React.FC = () => {
)}
</Modal>
<ConfirmDialog
title={t("dialog.title.delete", {
what:
applicationsToDelete.length > 1
? t("terms.application(s)").toLowerCase()
: t("terms.application").toLowerCase(),
})}
title={t(
applicationsToDelete.length > 1
? "dialog.title.delete"
: "dialog.title.deleteWithName",
{
what:
applicationsToDelete.length > 1
? t("terms.application(s)").toLowerCase()
: t("terms.application").toLowerCase(),
name:
applicationsToDelete.length === 1 && applicationsToDelete[0].name,
}
)}
titleIconVariant={"warning"}
isOpen={applicationsToDelete.length > 0}
message={`${t("dialog.message.applicationsBulkDelete")} ${t(
"dialog.message.delete"
)}`}
message={`${
applicationsToDelete.length > 1
? t("dialog.message.applicationsBulkDelete")
: ""
} ${t("dialog.message.delete")}`}
aria-label="Applications bulk delete"
confirmBtnVariant={ButtonVariant.danger}
confirmBtnLabel={t("actions.delete")}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import * as React from "react";
import { useState } from "react";
import { AxiosError } from "axios";
import { useHistory, useLocation } from "react-router-dom";
import { useHistory } from "react-router-dom";
import { Trans, useTranslation } from "react-i18next";

// @patternfly
Expand All @@ -19,30 +19,21 @@ import {
MenuToggleElement,
Modal,
} from "@patternfly/react-core";
import {
PencilAltIcon,
TagIcon,
EllipsisVIcon,
CubesIcon,
} from "@patternfly/react-icons";
import { PencilAltIcon, TagIcon, EllipsisVIcon } from "@patternfly/react-icons";
import {
Table,
Thead,
Tr,
Th,
Tbody,
Td,
TableText,
ActionsColumn,
CustomActionsToggleProps,
} from "@patternfly/react-table";

// @app components and utilities
import { AppPlaceholder } from "@app/components/AppPlaceholder";
import {
FilterType,
FilterToolbar,
FilterCategory,
} from "@app/components/FilterToolbar/FilterToolbar";
import { SimplePagination } from "@app/components/SimplePagination";
import {
Expand Down Expand Up @@ -72,10 +63,7 @@ import { checkAccess } from "@app/utils/rbac-utils";

// Hooks
import { useQueryClient } from "@tanstack/react-query";
import {
handlePropagatedRowClick,
useLocalTableControls,
} from "@app/hooks/table-controls";
import { useLocalTableControls } from "@app/hooks/table-controls";
import { useAssessApplication } from "@app/hooks";

// Queries
Expand Down Expand Up @@ -403,7 +391,7 @@ export const ApplicationsTable: React.FC = () => {
what: t("terms.tagName").toLowerCase(),
}) + "...",
getItemValue: (item) => {
let tagNames = item?.tags?.map((tag) => tag.name).join("");
const tagNames = item?.tags?.map((tag) => tag.name).join("");
return tagNames || "";
},
selectOptions: dedupeFunction(
Expand Down Expand Up @@ -857,17 +845,27 @@ export const ApplicationsTable: React.FC = () => {
/>
</Modal>
<ConfirmDialog
title={t("dialog.title.delete", {
what:
applicationsToDelete.length > 1
? t("terms.application(s)").toLowerCase()
: t("terms.application").toLowerCase(),
})}
title={t(
applicationsToDelete.length > 1
? "dialog.title.delete"
: "dialog.title.deleteWithName",
{
what:
applicationsToDelete.length > 1
? t("terms.application(s)").toLowerCase()
: t("terms.application").toLowerCase(),
name:
applicationsToDelete.length === 1 &&
applicationsToDelete[0].name,
}
)}
titleIconVariant={"warning"}
isOpen={applicationsToDelete.length > 0}
message={`${t("dialog.message.applicationsBulkDelete")} ${t(
"dialog.message.delete"
)}`}
message={`${
applicationsToDelete.length > 1
? t("dialog.message.applicationsBulkDelete")
: ""
} ${t("dialog.message.delete")}`}
aria-label="Applications bulk delete"
confirmBtnVariant={ButtonVariant.danger}
confirmBtnLabel={t("actions.delete")}
Expand Down
26 changes: 11 additions & 15 deletions client/src/app/pages/applications/manage-imports/manage-imports.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,8 @@ export const ManageImports: React.FC = () => {
// i18
const { t } = useTranslation();

const [isConfirmDialogOpen, setIsConfirmDialogOpen] =
React.useState<Boolean>(false);

const [importSummaryIdToDelete, setImportSummaryIdToDelete] =
React.useState<number>();
const [importSummaryToDelete, setImportSummaryToDelete] =
React.useState<ApplicationImportSummary>();

const { pushNotification } = React.useContext(NotificationsContext);

Expand Down Expand Up @@ -276,8 +273,7 @@ export const ManageImports: React.FC = () => {

// Row actions
const deleteRow = (row: ApplicationImportSummary) => {
setImportSummaryIdToDelete(row.id);
setIsConfirmDialogOpen(true);
setImportSummaryToDelete(row);
};

const viewRowDetails = (row: ApplicationImportSummary) => {
Expand Down Expand Up @@ -383,25 +379,25 @@ export const ManageImports: React.FC = () => {
}}
/>
</Modal>
{isConfirmDialogOpen && (
{!!importSummaryToDelete && (
<ConfirmDialog
title={t("dialog.title.delete", {
title={t("dialog.title.deleteWithName", {
what: "import summary",
name: importSummaryToDelete.filename,
})}
titleIconVariant={"warning"}
message={t("dialog.message.delete")}
isOpen={true}
confirmBtnVariant={ButtonVariant.danger}
confirmBtnLabel={t("actions.delete")}
cancelBtnLabel={t("actions.cancel")}
onCancel={() => setIsConfirmDialogOpen(false)}
onClose={() => setIsConfirmDialogOpen(false)}
onCancel={() => setImportSummaryToDelete(undefined)}
onClose={() => setImportSummaryToDelete(undefined)}
onConfirm={() => {
if (importSummaryIdToDelete) {
deleteImportSummary(importSummaryIdToDelete);
setImportSummaryIdToDelete(undefined);
if (importSummaryToDelete) {
deleteImportSummary(importSummaryToDelete.id);
setImportSummaryToDelete(undefined);
}
setIsConfirmDialogOpen(false);
}}
/>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ import { Questionnaire } from "@app/api/models";
import { useHistory } from "react-router-dom";
import { Paths } from "@app/Paths";
import { ImportQuestionnaireForm } from "@app/pages/assessment/import-questionnaire-form/import-questionnaire-form";
import ConfirmDeleteDialog from "@app/components/ConfirmDeleteCatalog/ConfirmDeleteCatalog";
import ConfirmDeleteDialog from "@app/components/ConfirmDeleteDialog/ConfirmDeleteDialog";

const AssessmentSettings: React.FC = () => {
const { t } = useTranslation();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ export const BusinessServices: React.FC = () => {
const { pushNotification } = React.useContext(NotificationsContext);

const [isConfirmDialogOpen, setIsConfirmDialogOpen] =
React.useState<Boolean>(false);
React.useState<boolean>(false);

const [businessServiceIdToDelete, setBusinessServiceIdToDelete] =
React.useState<number>();
const [businessServiceToDelete, setBusinessServiceToDelete] =
React.useState<BusinessService>();

const [createUpdateModalState, setCreateUpdateModalState] = React.useState<
"create" | BusinessService | null
Expand Down Expand Up @@ -193,7 +193,7 @@ export const BusinessServices: React.FC = () => {
});

const deleteRow = (row: BusinessService) => {
setBusinessServiceIdToDelete(row.id);
setBusinessServiceToDelete(row);
setIsConfirmDialogOpen(true);
};

Expand Down Expand Up @@ -289,8 +289,9 @@ export const BusinessServices: React.FC = () => {
</Modal>
{isConfirmDialogOpen && (
<ConfirmDialog
title={t("dialog.title.delete", {
title={t("dialog.title.deleteWithName", {
what: t("terms.businessService").toLowerCase(),
name: businessServiceToDelete?.name,
})}
titleIconVariant={"warning"}
message={t("dialog.message.delete")}
Expand All @@ -301,9 +302,9 @@ export const BusinessServices: React.FC = () => {
onCancel={() => setIsConfirmDialogOpen(false)}
onClose={() => setIsConfirmDialogOpen(false)}
onConfirm={() => {
if (businessServiceIdToDelete) {
deleteBusinessService(businessServiceIdToDelete);
setBusinessServiceIdToDelete(undefined);
if (businessServiceToDelete) {
deleteBusinessService(businessServiceToDelete.id);
setBusinessServiceToDelete(undefined);
}
setIsConfirmDialogOpen(false);
}}
Expand Down
28 changes: 11 additions & 17 deletions client/src/app/pages/controls/job-functions/job-functions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,8 @@ export const JobFunctions: React.FC = () => {
const { t } = useTranslation();
const { pushNotification } = React.useContext(NotificationsContext);

const [isConfirmDialogOpen, setIsConfirmDialogOpen] =
React.useState<Boolean>(false);

const [
jobFunctionstakeholderIdToDelete,
setJobFunctionStakeholderIdToDelete,
] = React.useState<number>();
const [jobFunctionToDelete, setJobFunctionToDelete] =
React.useState<JobFunction>();

const [createUpdateModalState, setCreateUpdateModalState] = React.useState<
"create" | JobFunction | null
Expand Down Expand Up @@ -154,8 +149,7 @@ export const JobFunctions: React.FC = () => {
// Rows

const deleteRow = (row: JobFunction) => {
setJobFunctionStakeholderIdToDelete(row.id);
setIsConfirmDialogOpen(true);
setJobFunctionToDelete(row);
};

// Advanced filters
Expand Down Expand Up @@ -247,25 +241,25 @@ export const JobFunctions: React.FC = () => {
/>
</Modal>

{isConfirmDialogOpen && (
{!!jobFunctionToDelete && (
<ConfirmDialog
title={t("dialog.title.delete", {
title={t("dialog.title.deleteWithName", {
what: t("terms.jobFunction").toLowerCase(),
name: jobFunctionToDelete.name,
})}
isOpen={true}
titleIconVariant={"warning"}
message={t("dialog.message.delete")}
confirmBtnVariant={ButtonVariant.danger}
confirmBtnLabel={t("actions.delete")}
cancelBtnLabel={t("actions.cancel")}
onCancel={() => setIsConfirmDialogOpen(false)}
onClose={() => setIsConfirmDialogOpen(false)}
onCancel={() => setJobFunctionToDelete(undefined)}
onClose={() => setJobFunctionToDelete(undefined)}
onConfirm={() => {
if (jobFunctionstakeholderIdToDelete) {
deleteJobFunction(jobFunctionstakeholderIdToDelete);
setJobFunctionStakeholderIdToDelete(undefined);
if (jobFunctionToDelete) {
deleteJobFunction(jobFunctionToDelete.id);
setJobFunctionToDelete(undefined);
}
setIsConfirmDialogOpen(false);
}}
/>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,8 @@ export const StakeholderGroups: React.FC = () => {
const { t } = useTranslation();
const { pushNotification } = React.useContext(NotificationsContext);

const [isConfirmDialogOpen, setIsConfirmDialogOpen] =
React.useState<Boolean>(false);

const [stakeholderGroupIdToDelete, setStakeholderGroupIdToDelete] =
React.useState<number>();
const [stakeholderGroupToDelete, setStakeholderGroupToDelete] =
React.useState<StakeholderGroup>();

const [createUpdateModalState, setCreateUpdateModalState] = React.useState<
"create" | StakeholderGroup | null
Expand Down Expand Up @@ -250,8 +247,7 @@ export const StakeholderGroups: React.FC = () => {
};

const deleteRow = (row: StakeholderGroup) => {
setStakeholderGroupIdToDelete(row.id);
setIsConfirmDialogOpen(true);
setStakeholderGroupToDelete(row);
};

// Advanced filters
Expand Down Expand Up @@ -350,25 +346,25 @@ export const StakeholderGroups: React.FC = () => {
/>
</Modal>

{isConfirmDialogOpen && (
{!!stakeholderGroupToDelete && (
<ConfirmDialog
title={t("dialog.title.delete", {
title={t("dialog.title.deleteWithName", {
what: t("terms.stakeholderGroup").toLowerCase(),
name: stakeholderGroupToDelete.name,
})}
isOpen={true}
titleIconVariant={"warning"}
message={t("dialog.message.delete")}
confirmBtnVariant={ButtonVariant.danger}
confirmBtnLabel={t("actions.delete")}
cancelBtnLabel={t("actions.cancel")}
onCancel={() => setIsConfirmDialogOpen(false)}
onClose={() => setIsConfirmDialogOpen(false)}
onCancel={() => setStakeholderGroupToDelete(undefined)}
onClose={() => setStakeholderGroupToDelete(undefined)}
onConfirm={() => {
if (stakeholderGroupIdToDelete) {
deleteStakeholderGroup(stakeholderGroupIdToDelete);
setStakeholderGroupIdToDelete(undefined);
if (stakeholderGroupToDelete) {
deleteStakeholderGroup(stakeholderGroupToDelete.id);
setStakeholderGroupToDelete(undefined);
}
setIsConfirmDialogOpen(false);
}}
/>
)}
Expand Down
Loading

0 comments on commit ccafe8d

Please sign in to comment.