Skip to content

Commit

Permalink
feat(Modal): keep focus inside opened modal, return focus to last ele…
Browse files Browse the repository at this point in the history 
…ment (#2434)
  • Loading branch information
@YossiSaadi
YossiSaadi authored Sep 26, 2024
1 parent c95b799 commit 5f66d89
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 19 deletions.
1 change: 1 addition & 0 deletions packages/core/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@
"monday-ui-style": "0.12.0",
"prop-types": "^15.8.1",
"react-dates": "21.8.0",
"react-focus-lock": "^2.13.2",
"react-inlinesvg": "^3.0.1",
"react-is": "^16.9.0",
"react-popper": "^2.3.0",
Expand Down
39 changes: 21 additions & 18 deletions packages/core/src/components/ModalNew/Modal/Modal.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React, { forwardRef, useCallback, useMemo, useState } from "react";
import cx from "classnames";
import { RemoveScroll } from "react-remove-scroll";
import FocusLock from "react-focus-lock";
import { getTestId } from "../../../tests/test-ids-utils";
import { ComponentDefaultTestId } from "../../../tests/constants";
import styles from "./Modal.module.scss";
Expand Down Expand Up @@ -79,24 +80,26 @@ const Modal = forwardRef(
onClick={onBackdropClick}
aria-hidden
/>
<div
ref={ref}
className={cx(styles.modal, getStyle(styles, camelCase("size-" + size)), className)}
id={id}
data-testid={dataTestId || getTestId(ComponentDefaultTestId.MODAL_NEXT, id)}
role="dialog"
aria-modal
aria-labelledby={titleId}
aria-describedby={descriptionId}
>
<ModalTopActions
renderAction={renderHeaderAction}
color={closeButtonTheme}
closeButtonAriaLabel={closeButtonAriaLabel}
onClose={onClose}
/>
{children}
</div>
<FocusLock returnFocus>
<div
ref={ref}
className={cx(styles.modal, getStyle(styles, camelCase("size-" + size)), className)}
id={id}
data-testid={dataTestId || getTestId(ComponentDefaultTestId.MODAL_NEXT, id)}
role="dialog"
aria-modal
aria-labelledby={titleId}
aria-describedby={descriptionId}
>
<ModalTopActions
renderAction={renderHeaderAction}
color={closeButtonTheme}
closeButtonAriaLabel={closeButtonAriaLabel}
onClose={onClose}
/>
{children}
</div>
</FocusLock>
</RemoveScroll>
</ModalProvider>
);
Expand Down
65 changes: 65 additions & 0 deletions packages/core/src/components/ModalNew/Modal/__tests__/Modal.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,71 @@ describe("Modal", () => {
expect(mockOnClose).toHaveBeenCalled();
});

it("traps focus inside the modal when opened", () => {
const { getByLabelText } = render(
<>
<button type="button">Focusable outside</button>
<Modal id={id} show closeButtonAriaLabel={closeButtonAriaLabel}>
{childrenContent}
</Modal>
</>
);
const closeButton = getByLabelText(closeButtonAriaLabel);
expect(closeButton).toHaveFocus();

userEvent.tab();
expect(closeButton).toHaveFocus();
});

it("releases focus lock inside the modal when closed", () => {
const { rerender, getByLabelText, getByText } = render(
<>
<button type="button">Focusable outside 1</button>
<button type="button">Focusable outside 2</button>
<Modal id={id} show closeButtonAriaLabel={closeButtonAriaLabel}>
{childrenContent}
</Modal>
</>
);
const closeButton = getByLabelText(closeButtonAriaLabel);
expect(closeButton).toHaveFocus();

rerender(
<>
<button type="button">Focusable outside 1</button>
<button type="button">Focusable outside 2</button>
<Modal id={id} show={false} closeButtonAriaLabel={closeButtonAriaLabel}>
{childrenContent}
</Modal>
</>
);

userEvent.tab();
expect(getByText("Focusable outside 1")).toHaveFocus();
userEvent.tab();
expect(getByText("Focusable outside 2")).toHaveFocus();
});

it("traps and moves focus between modal elements", () => {
const { getByLabelText, getByText } = render(
<Modal id={id} show closeButtonAriaLabel={closeButtonAriaLabel}>
<button type="button">Focusable 1</button>
<button type="button">Focusable 2</button>
</Modal>
);
const closeButton = getByLabelText(closeButtonAriaLabel);
expect(closeButton).toHaveFocus();

userEvent.tab();
expect(getByText("Focusable 1")).toHaveFocus();

userEvent.tab();
expect(getByText("Focusable 2")).toHaveFocus();

userEvent.tab();
expect(closeButton).toHaveFocus();
});

it.todo("renders the correct aria-labelledby");

it.todo("renders the correct aria-describedby");
Expand Down
35 changes: 34 additions & 1 deletion yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1187,6 +1187,13 @@
dependencies:
regenerator-runtime "^0.14.0"

"@babel/runtime@^7.12.13":
version "7.25.6"
resolved "https://registry.npmjs.org/@babel/runtime/-/runtime-7.25.6.tgz#9afc3289f7184d8d7f98b099884c26317b9264d2"
integrity sha512-VBj9MYyDb9tuLq7yzqjgzt6Q+IBQLrGZfdjOekyEirZPHxXWoTSGUTMrpsfi58Up73d13NfYLv8HT9vmznjzhQ==
dependencies:
regenerator-runtime "^0.14.0"

"@babel/runtime@^7.4.4", "@babel/runtime@^7.7.2":
version "7.25.0"
resolved "https://registry.npmjs.org/@babel/runtime/-/runtime-7.25.0.tgz#3af9a91c1b739c569d5d80cc917280919c544ecb"
Expand Down Expand Up @@ -10213,6 +10220,13 @@ flow-parser@0.*:
resolved "https://registry.npmjs.org/flow-parser/-/flow-parser-0.231.0.tgz#13daa172b3c06ffacbb31025592dc0db41fe28f3"
integrity sha512-WVzuqwq7ZnvBceCG0DGeTQebZE+iIU0mlk5PmJgYj9DDrt+0isGC2m1ezW9vxL4V+HERJJo9ExppOnwKH2op6Q==

focus-lock@^1.3.5:
version "1.3.5"
resolved "https://registry.npmjs.org/focus-lock/-/focus-lock-1.3.5.tgz#aa644576e5ec47d227b57eb14e1efb2abf33914c"
integrity sha512-QFaHbhv9WPUeLYBDe/PAuLKJ4Dd9OPvKs9xZBr3yLXnUrDNaVXKu2baDBXe3naPY30hgHYSsf2JW4jzas2mDEQ==
dependencies:
tslib "^2.0.3"

focusable-selectors@^0.4.0:
version "0.4.0"
resolved "https://registry.npmjs.org/focusable-selectors/-/focusable-selectors-0.4.0.tgz#c93092bfe65c7cf7ef52aed82fb6f8df31072459"
Expand Down Expand Up @@ -16681,6 +16695,13 @@ [email protected]:
iconv-lite "0.4.24"
unpipe "1.0.0"

react-clientside-effect@^1.2.6:
version "1.2.6"
resolved "https://registry.npmjs.org/react-clientside-effect/-/react-clientside-effect-1.2.6.tgz#29f9b14e944a376b03fb650eed2a754dd128ea3a"
integrity sha512-XGGGRQAKY+q25Lz9a/4EPqom7WRjz3z9R2k4jhVKA/puQFH/5Nt27vFZYql4m4NVNdUvX8PS3O7r/Zzm7cjUlg==
dependencies:
"@babel/runtime" "^7.12.13"

react-colorful@^5.1.2:
version "5.6.1"
resolved "https://registry.npmjs.org/react-colorful/-/react-colorful-5.6.1.tgz#7dc2aed2d7c72fac89694e834d179e32f3da563b"
Expand Down Expand Up @@ -16774,6 +16795,18 @@ react-fast-compare@^3.0.1:
resolved "https://registry.npmjs.org/react-fast-compare/-/react-fast-compare-3.2.2.tgz#929a97a532304ce9fee4bcae44234f1ce2c21d49"
integrity sha512-nsO+KSNgo1SbJqJEYRE9ERzo7YtYbou/OqjSQKxV7jcKox7+usiUVZOAC+XnDOABXggQTno0Y1CpVnuWEc1boQ==

react-focus-lock@^2.13.2:
version "2.13.2"
resolved "https://registry.npmjs.org/react-focus-lock/-/react-focus-lock-2.13.2.tgz#e1addac2f8b9550bc0581f3c416755ba0f81f5ef"
integrity sha512-T/7bsofxYqnod2xadvuwjGKHOoL5GH7/EIPI5UyEvaU/c2CcphvGI371opFtuY/SYdbMsNiuF4HsHQ50nA/TKQ==
dependencies:
"@babel/runtime" "^7.0.0"
focus-lock "^1.3.5"
prop-types "^15.6.2"
react-clientside-effect "^1.2.6"
use-callback-ref "^1.3.2"
use-sidecar "^1.1.2"

react-from-dom@^0.6.2:
version "0.6.2"
resolved "https://registry.npmjs.org/react-from-dom/-/react-from-dom-0.6.2.tgz#9da903a508c91c013b55afcd59348b8b0a39bdb4"
Expand Down Expand Up @@ -19871,7 +19904,7 @@ url@^0.11.0:
punycode "^1.4.1"
qs "^6.11.2"

use-callback-ref@^1.3.0:
use-callback-ref@^1.3.0, use-callback-ref@^1.3.2:
version "1.3.2"
resolved "https://registry.npmjs.org/use-callback-ref/-/use-callback-ref-1.3.2.tgz#6134c7f6ff76e2be0b56c809b17a650c942b1693"
integrity sha512-elOQwe6Q8gqZgDA8mrh44qRTQqpIHDcZ3hXTLjBe1i4ph8XpNJnO+aQf3NaG+lriLopI4HMx9VjQLfPQ6vhnoA==
Expand Down

0 comments on commit 5f66d89

Please sign in to comment.