Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(react): remove focus-trap-react replacing with internally managed focus traps #1730

Merged
merged 26 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/pages/components/Scrim.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { Scrim } from '@deque/cauldron-react'
A scrim is a component that is a semi-transparent overlay over the viewport to provide emphasis, focus, or draw attention to certain information. This is most commonly used as a backdrop for dialogs, modals, and other elements that need primary focus on the page.

<Note>
It is recommended that a focus trap is used to contain keyboard focus within the element that has primary focus. [Alert](./Alert) and [Modal](./Modal) both use [react-focus-trap](https://www.npmjs.com/package/focus-trap-react) to manage focus. If `Scrim` is being used with other components, a focus trap may be needed to prevent certain keyboard accessibility issues.
It is recommended that a focus trap is used to contain keyboard focus within the element that has primary focus. If `Scrim` is being used with other components, a focus trap (among other AT considerations) may be needed to prevent certain keyboard accessibility issues.
</Note>

## Example
Expand Down
1 change: 0 additions & 1 deletion packages/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
"dependencies": {
"@floating-ui/react-dom": "^2.1.2",
"classnames": "^2.2.6",
"focus-trap-react": "^10.2.3",
"focusable": "^2.3.0",
"keyname": "^0.1.0",
"react-id-generator": "^3.0.1",
Expand Down
290 changes: 126 additions & 164 deletions packages/react/src/components/Dialog/index.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import React from 'react';
import React, { useRef, useEffect, useCallback, forwardRef } from 'react';
import { createPortal } from 'react-dom';
import classNames from 'classnames';
import FocusTrap from 'focus-trap-react';
import Offscreen from '../Offscreen';
import Icon from '../Icon';
import ClickOutsideListener from '../ClickOutsideListener';
import AriaIsolate from '../../utils/aria-isolate';
import setRef from '../../utils/setRef';
import nextId from 'react-id-generator';
import { useId } from 'react-id-generator';
import { isBrowser } from '../../utils/is-browser';
import useSharedRef from '../../utils/useSharedRef';
import useFocusTrap from '../../utils/useFocusTrap';

export interface DialogProps extends React.HTMLAttributes<HTMLDivElement> {
children: React.ReactNode;
Expand All @@ -28,196 +28,157 @@ export interface DialogProps extends React.HTMLAttributes<HTMLDivElement> {
portal?: React.RefObject<HTMLElement> | HTMLElement;
}

interface DialogState {
isolator?: AriaIsolate;
}

const isEscape = (event: KeyboardEvent) =>
event.key === 'Escape' || event.key === 'Esc' || event.keyCode === 27;

const noop = () => {
//not empty
};

export default class Dialog extends React.Component<DialogProps, DialogState> {
static defaultProps = {
onClose: noop,
forceAction: false,
closeButtonText: 'Close'
};

private element: HTMLDivElement | null;
private heading: HTMLHeadingElement | null;
private headingId: string = nextId('dialog-title-');

constructor(props: DialogProps) {
super(props);

this.close = this.close.bind(this);
this.focusHeading = this.focusHeading.bind(this);
this.handleClickOutside = this.handleClickOutside.bind(this);
this.handleEscape = this.handleEscape.bind(this);
this.state = {};
}

componentDidMount() {
if (this.props.show) {
this.attachEventListeners();
this.attachIsolator(() => setTimeout(this.focusHeading));
}
}

componentWillUnmount() {
const { isolator } = this.state;
isolator?.deactivate();
this.removeEventListeners();
}

componentDidUpdate(prevProps: DialogProps) {
if (!prevProps.show && this.props.show) {
this.attachIsolator(this.focusHeading);
this.attachEventListeners();
} else if (prevProps.show && !this.props.show) {
this.removeEventListeners();
this.close();
}
}

private attachIsolator(done: () => void) {
this.setState(
{
isolator: new AriaIsolate(this.element as HTMLElement)
},
done
);
}

render() {
const {
dialogRef,
forceAction,
const Dialog = forwardRef<HTMLDivElement, DialogProps>(
(
{
dialogRef: dialogRefProp,
forceAction = false,
className,
children,
closeButtonText,
closeButtonText = 'Close',
heading,
show,
show = false,
portal,
onClose = () => null,
...other
} = this.props;
},
ref
): React.ReactPortal | null => {
const dialogRef = useSharedRef(dialogRefProp || ref);
const [headingId] = useId(1, 'dialog-title-');
const headingRef = useRef<HTMLHeadingElement>(null);
const isolatorRef = useRef<AriaIsolate>();

const handleClose = useCallback(() => {
isolatorRef.current?.deactivate();
if (show) {
onClose();
}
}, [show, onClose]);

const handleClickOutside = useCallback(() => {
if (show && !forceAction) {
handleClose();
}
}, [show, forceAction, handleClose]);

const focusHeading = useCallback(() => {
headingRef.current?.focus();
isolatorRef.current?.activate();
}, []);

const handleEscape = useCallback(
(keyboardEvent: KeyboardEvent) => {
if (!keyboardEvent.defaultPrevented && isEscape(keyboardEvent)) {
handleClose();
}
},
[handleClose]
);

useEffect(() => {
if (!show || !dialogRef.current) return;

isolatorRef.current = new AriaIsolate(dialogRef.current);
setTimeout(focusHeading);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit - this use of setTimeout could probably benefit from a comment (I assume this is to wait for the dialog to be rendered, if so there may be a way to avoid setTimeout use)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but I wish I remembered why as it's been like this since it's initial implementation:

this.attachEventListeners();
this.attachIsolator(() => setTimeout(this.focusHeading));

I'm taking an educated guess that it's queuing the focus to be on the next tick to prevent focus conflicts. We can add that as a comment if you feel that's useful?

return () => {
isolatorRef.current?.deactivate();
};
}, [show, focusHeading]);

useEffect(() => {
if (!forceAction) {
const portalElement = portal
? 'current' in portal
? portal.current
: portal
: document.body;

if (show) {
portalElement?.addEventListener('keyup', handleEscape);
}

return () => {
portalElement?.removeEventListener('keyup', handleEscape);
};
}
}, [show, forceAction, portal, handleEscape]);

useFocusTrap(dialogRef, {
disabled: !show,
initialFocusElement: headingRef
});

if (!show || !isBrowser()) {
return null;
}

const portal = this.props.portal || document.body;
const portalElement = portal
? 'current' in portal
? portal.current
: portal
: // eslint-disable-next-line ssr-friendly/no-dom-globals-in-react-fc
document.body;

const close = !forceAction ? (
<button className="Dialog__close" type="button" onClick={this.close}>
const closeButton = !forceAction ? (
<button className="Dialog__close" type="button" onClick={handleClose}>
<Icon type="close" aria-hidden="true" />
<Offscreen>{closeButtonText}</Offscreen>
</button>
) : null;

const Heading = `h${
typeof heading === 'object' && 'level' in heading && !!heading.level
const HeadingLevel = `h${
typeof heading === 'object' && 'level' in heading && heading.level
? heading.level
: 2
}` as 'h1';

const Dialog = (
<FocusTrap
focusTrapOptions={{
allowOutsideClick: true,
escapeDeactivates: false,
fallbackFocus: '.Dialog__heading'
}}
>
<ClickOutsideListener onClickOutside={this.handleClickOutside}>
<div
role="dialog"
className={classNames('Dialog', className, {
'Dialog--show': show
})}
ref={(el) => {
this.element = el;
if (!dialogRef) {
return;
}
setRef(dialogRef, el);
}}
aria-labelledby={this.headingId}
{...other}
>
<div className="Dialog__inner">
<div className="Dialog__header">
<Heading
className="Dialog__heading"
ref={(el: HTMLHeadingElement) => (this.heading = el)}
tabIndex={-1}
id={this.headingId}
>
{typeof heading === 'object' && 'text' in heading
? heading.text
: heading}
</Heading>
{close}
</div>
{children}
const dialog = (
<ClickOutsideListener onClickOutside={handleClickOutside}>
<div
role="dialog"
className={classNames('Dialog', className, {
'Dialog--show': show
})}
ref={dialogRef}
aria-labelledby={headingId}
{...other}
>
<div className="Dialog__inner">
<div className="Dialog__header">
<HeadingLevel
className="Dialog__heading"
ref={headingRef}
tabIndex={-1}
id={headingId}
>
{typeof heading === 'object' && 'text' in heading
? heading.text
: heading}
</HeadingLevel>
{closeButton}
</div>
{children}
</div>
</ClickOutsideListener>
</FocusTrap>
</div>
</ClickOutsideListener>
);

return createPortal(
Dialog,
('current' in portal ? portal.current : portal) || document.body
) as React.JSX.Element;
}

close() {
this.state.isolator?.deactivate();
if (this.props.show) {
this.props.onClose?.();
}
}

handleClickOutside() {
const { show, forceAction } = this.props;
if (show && !forceAction) {
this.close();
}
}

focusHeading() {
if (this.heading) {
this.heading.focus();
}
this.state.isolator?.activate();
dialog,
// eslint-disable-next-line ssr-friendly/no-dom-globals-in-react-fc
portalElement || document.body
) as React.ReactPortal;
}
);

private handleEscape(keyboardEvent: KeyboardEvent) {
if (!keyboardEvent.defaultPrevented && isEscape(keyboardEvent)) {
this.close();
}
}
Dialog.displayName = 'Dialog';

private attachEventListeners() {
const { forceAction } = this.props;
if (!forceAction) {
const portal = this.props.portal || document.body;
const targetElement =
portal instanceof HTMLElement ? portal : portal.current;
targetElement?.addEventListener('keyup', this.handleEscape);
}
}

private removeEventListeners() {
const portal = this.props.portal || document.body;
const targetElement =
portal instanceof HTMLElement ? portal : portal.current;
targetElement?.removeEventListener('keyup', this.handleEscape);
}
}
export default Dialog;

interface DialogAlignmentProps {
align?: 'left' | 'center' | 'right';
Expand Down Expand Up @@ -266,4 +227,5 @@ const DialogFooter = ({
</div>
);
DialogFooter.displayName = 'DialogFooter';

export { Dialog, DialogContent, DialogFooter };
3 changes: 1 addition & 2 deletions packages/react/src/components/Drawer/Drawer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -305,8 +305,7 @@ test('should not trap focus when behavior is non-modal', async () => {
'aria-hidden',
'true'
);
await user.keyboard('{Tab}');
expect(document.body).toHaveFocus();
document.body.focus();
await user.keyboard('{Tab}');
expect(screen.getByRole('button', { name: 'outside' })).toHaveFocus();
await user.keyboard('{Tab}');
Expand Down
Loading
Loading