Skip to content

Commit 49a4e15

Browse files
authored
fix(Modal): modal no longer sets focus when close prop changes (#1253)
1 parent 494b927 commit 49a4e15

File tree

2 files changed

+39
-4
lines changed

2 files changed

+39
-4
lines changed

src/components/Modal/Modal.test.tsx

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import userEvent from "@testing-library/user-event";
22
import { render, screen } from "@testing-library/react";
3-
import React, { useEffect } from "react";
3+
import React, { FC, useEffect, useState } from "react";
44

5+
import Button from "components/Button";
6+
import Input from "components/Input";
57
import Modal from "./Modal";
68

79
describe("Modal ", () => {
@@ -189,4 +191,35 @@ describe("Modal ", () => {
189191
await userEvent.click(container);
190192
expect(handleCloseModal).toHaveBeenCalledTimes(0);
191193
});
194+
195+
it("doesn't lose focus when close prop changes", async () => {
196+
const TestComponent: FC = () => {
197+
const [inputText, setInputText] = useState("");
198+
199+
const handleCloseModal = () => {
200+
setInputText("");
201+
};
202+
203+
return (
204+
<div>
205+
<Modal title="Delete item1" close={handleCloseModal}>
206+
<Button>Show help</Button>
207+
<p>Type "delete item1" to confirm.</p>
208+
<Input
209+
type="text"
210+
value={inputText}
211+
onChange={(event) => setInputText(event.target.value)}
212+
/>
213+
</Modal>
214+
</div>
215+
);
216+
};
217+
218+
const { container } = render(<TestComponent />);
219+
220+
const input = container.querySelector("input");
221+
await userEvent.type(container.querySelector("input"), "delete item1");
222+
expect(input).toHaveFocus();
223+
expect(input).toHaveValue("delete item1");
224+
});
192225
});

src/components/Modal/Modal.tsx

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,17 +106,19 @@ export const Modal = ({
106106
modalRef.current.focus();
107107
}, [modalRef]);
108108

109+
const hasCloseButton = !!close;
110+
109111
useEffect(() => {
110112
focusableModalElements.current = modalRef.current.querySelectorAll(
111113
focusableElementSelectors,
112114
);
113115
let focusIndex = 0;
114116
// when the close button is rendered, focus on the 2nd content element and not the close btn.
115-
if (close && focusableModalElements.current.length > 1) {
117+
if (hasCloseButton && focusableModalElements.current.length > 1) {
116118
focusIndex = 1;
117119
}
118120
focusableModalElements.current[focusIndex]?.focus({ preventScroll: true });
119-
}, [close]);
121+
}, [hasCloseButton]);
120122

121123
useEffect(() => {
122124
const keyDown = (event: KeyboardEvent) => {
@@ -183,7 +185,7 @@ export const Modal = ({
183185
<h2 className="p-modal__title" id={titleId}>
184186
{title}
185187
</h2>
186-
{!!close && (
188+
{hasCloseButton && (
187189
<button
188190
type="button"
189191
className="p-modal__close"

0 commit comments

Comments
 (0)