Skip to content

Commit 38bb854

Browse files
ParkreinerParkreiner
authored
fix: update ErrorDialog logic and tests (#10111)
* fix: make error text less naggy * fix: make input colors sync with confirmation text state * fix: more color sync fixes * fix: remove flaky warning messages in test * fix: remove needless braces * refactor: clean up code * refactor: clean up code more
1 parent ae11317 commit 38bb854

File tree

2 files changed

+93
-61
lines changed

2 files changed

+93
-61
lines changed

‎site/src/components/Dialogs/DeleteDialog/DeleteDialog.test.tsx

+26-4
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,23 @@ import { screen } from "@testing-library/react";
22
import userEvent from "@testing-library/user-event";
33
import { render } from "testHelpers/renderHelpers";
44
import { DeleteDialog } from "./DeleteDialog";
5+
import { act } from "react-dom/test-utils";
6+
7+
const inputTestId = "delete-dialog-name-confirmation";
8+
9+
async function fillInputField(inputElement: HTMLElement, text: string) {
10+
// 2023-10-06 - There's something wonky with MUI's ConfirmDialog that causes
11+
// its state to update after a typing event gets fired, and React Testing
12+
// Library isn't able to catch it, making React DOM freak out because an
13+
// "unexpected" state change happened. It won't fail the test, but it makes
14+
// the console look really scary because it'll spit out a big warning message.
15+
// Tried everything under the sun to catch the state changes the proper way,
16+
// but the only way to get around it for now might be to manually make React
17+
// DOM aware of the changes
18+
19+
// eslint-disable-next-line testing-library/no-unnecessary-act -- have to make sure state updates don't slip through cracks
20+
return act(() => userEvent.type(inputElement, text));
21+
}
522

623
describe("DeleteDialog", () => {
724
it("disables confirm button when the text field is empty", () => {
@@ -14,6 +31,7 @@ describe("DeleteDialog", () => {
1431
name="MyTemplate"
1532
/>,
1633
);
34+
1735
const confirmButton = screen.getByRole("button", { name: "Delete" });
1836
expect(confirmButton).toBeDisabled();
1937
});
@@ -28,8 +46,10 @@ describe("DeleteDialog", () => {
2846
name="MyTemplate"
2947
/>,
3048
);
31-
const textField = screen.getByTestId("delete-dialog-name-confirmation");
32-
await userEvent.type(textField, "MyTemplateWrong");
49+
50+
const textField = screen.getByTestId(inputTestId);
51+
await fillInputField(textField, "MyTemplateButWrong");
52+
3353
const confirmButton = screen.getByRole("button", { name: "Delete" });
3454
expect(confirmButton).toBeDisabled();
3555
});
@@ -44,8 +64,10 @@ describe("DeleteDialog", () => {
4464
name="MyTemplate"
4565
/>,
4666
);
47-
const textField = screen.getByTestId("delete-dialog-name-confirmation");
48-
await userEvent.type(textField, "MyTemplate");
67+
68+
const textField = screen.getByTestId(inputTestId);
69+
await fillInputField(textField, "MyTemplate");
70+
4971
const confirmButton = screen.getByRole("button", { name: "Delete" });
5072
expect(confirmButton).not.toBeDisabled();
5173
});

‎site/src/components/Dialogs/DeleteDialog/DeleteDialog.tsx

+67-57
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
1-
import makeStyles from "@mui/styles/makeStyles";
1+
import {
2+
type FC,
3+
type FormEvent,
4+
type PropsWithChildren,
5+
useId,
6+
useState,
7+
} from "react";
8+
9+
import { useTheme } from "@emotion/react";
210
import TextField from "@mui/material/TextField";
3-
import { ChangeEvent, useState, PropsWithChildren, FC } from "react";
411
import { ConfirmDialog } from "../ConfirmDialog/ConfirmDialog";
512

613
export interface DeleteDialogProps {
@@ -22,51 +29,23 @@ export const DeleteDialog: FC<PropsWithChildren<DeleteDialogProps>> = ({
2229
name,
2330
confirmLoading,
2431
}) => {
25-
const styles = useStyles();
26-
const [nameValue, setNameValue] = useState("");
27-
const confirmed = name === nameValue;
28-
const handleChange = (event: ChangeEvent<HTMLInputElement>) => {
29-
setNameValue(event.target.value);
30-
};
31-
const hasError = nameValue.length > 0 && !confirmed;
32+
const hookId = useId();
33+
const theme = useTheme();
3234

33-
const content = (
34-
<>
35-
<p>Deleting this {entity} is irreversible!</p>
36-
{Boolean(info) && <p className={styles.warning}>{info}</p>}
37-
<p>Are you sure you want to proceed?</p>
38-
<p>
39-
Type &ldquo;<strong>{name}</strong>&rdquo; below to confirm.
40-
</p>
35+
const [userConfirmationText, setUserConfirmationText] = useState("");
36+
const [isFocused, setIsFocused] = useState(false);
4137

42-
<form
43-
onSubmit={(e) => {
44-
e.preventDefault();
45-
if (confirmed) {
46-
onConfirm();
47-
}
48-
}}
49-
>
50-
<TextField
51-
fullWidth
52-
autoFocus
53-
className={styles.textField}
54-
name="confirmation"
55-
autoComplete="off"
56-
id="confirmation"
57-
placeholder={name}
58-
value={nameValue}
59-
onChange={handleChange}
60-
label={`Name of the ${entity} to delete`}
61-
error={hasError}
62-
helperText={
63-
hasError && `${nameValue} does not match the name of this ${entity}`
64-
}
65-
inputProps={{ ["data-testid"]: "delete-dialog-name-confirmation" }}
66-
/>
67-
</form>
68-
</>
69-
);
38+
const deletionConfirmed = name === userConfirmationText;
39+
const onSubmit = (event: FormEvent) => {
40+
event.preventDefault();
41+
if (deletionConfirmed) {
42+
onConfirm();
43+
}
44+
};
45+
46+
const hasError = !deletionConfirmed && userConfirmationText.length > 0;
47+
const displayErrorMessage = hasError && !isFocused;
48+
const inputColor = hasError ? "error" : "primary";
7049

7150
return (
7251
<ConfirmDialog
@@ -76,19 +55,50 @@ export const DeleteDialog: FC<PropsWithChildren<DeleteDialogProps>> = ({
7655
title={`Delete ${entity}`}
7756
onConfirm={onConfirm}
7857
onClose={onCancel}
79-
description={content}
8058
confirmLoading={confirmLoading}
81-
disabled={!confirmed}
59+
disabled={!deletionConfirmed}
60+
description={
61+
<>
62+
<p>Deleting this {entity} is irreversible!</p>
63+
64+
{Boolean(info) && (
65+
<p css={{ color: theme.palette.warning.light }}>{info}</p>
66+
)}
67+
68+
<p>Are you sure you want to proceed?</p>
69+
70+
<p>
71+
Type &ldquo;<strong>{name}</strong>&rdquo; below to confirm.
72+
</p>
73+
74+
<form onSubmit={onSubmit}>
75+
<TextField
76+
fullWidth
77+
autoFocus
78+
sx={{ marginTop: theme.spacing(3) }}
79+
name="confirmation"
80+
autoComplete="off"
81+
id={`${hookId}-confirm`}
82+
placeholder={name}
83+
value={userConfirmationText}
84+
onChange={(event) => setUserConfirmationText(event.target.value)}
85+
onFocus={() => setIsFocused(true)}
86+
onBlur={() => setIsFocused(false)}
87+
label={`Name of the ${entity} to delete`}
88+
color={inputColor}
89+
error={displayErrorMessage}
90+
helperText={
91+
displayErrorMessage &&
92+
`${userConfirmationText} does not match the name of this ${entity}`
93+
}
94+
InputProps={{ color: inputColor }}
95+
inputProps={{
96+
"data-testid": "delete-dialog-name-confirmation",
97+
}}
98+
/>
99+
</form>
100+
</>
101+
}
82102
/>
83103
);
84104
};
85-
86-
const useStyles = makeStyles((theme) => ({
87-
warning: {
88-
color: theme.palette.warning.light,
89-
},
90-
91-
textField: {
92-
marginTop: theme.spacing(3),
93-
},
94-
}));

0 commit comments

Comments
 (0)