UI: Improve modal a11y by setting role & using title as label (#45472)

* UI: Improve modal a11y by setting role & using title as label

* remove wrapping div for cutom title components

* Fix typo
This commit is contained in:
Giordano Ricci 2022-02-22 14:12:34 +00:00 committed by GitHub
parent c11cf7d470
commit 9307cc86f2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 43 additions and 51 deletions

View File

@ -44,9 +44,6 @@ exports[`no enzyme tests`] = {
"packages/grafana-ui/src/components/Logs/LogRows.test.tsx:2288254498": [
[3, 17, 13, "RegExp match", "2409514259"]
],
"packages/grafana-ui/src/components/Modal/Modal.test.tsx:4235780832": [
[1, 17, 13, "RegExp match", "2409514259"]
],
"packages/grafana-ui/src/components/QueryField/QueryField.test.tsx:1906163280": [
[1, 19, 13, "RegExp match", "2409514259"]
],

View File

@ -1,27 +1,24 @@
import React from 'react';
import { mount } from 'enzyme';
import { Modal } from './Modal';
import { render, screen } from '@testing-library/react';
describe('Modal', () => {
it('renders without error', () => {
mount(<Modal title={'Some Title'} isOpen={true} />);
});
it('renders nothing by default or when isOpen is false', () => {
const wrapper = mount(<Modal title={'Some Title'} />);
expect(wrapper.html()).toBe(null);
render(<Modal title="Some Title" />);
wrapper.setProps({ ...wrapper.props(), isOpen: false });
expect(wrapper.html()).toBe(null);
expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
});
it('renders correct contents', () => {
const wrapper = mount(
<Modal title={'Some Title'} isOpen={true}>
<div id={'modal-content'}>Content</div>
render(
<Modal title="Some Title" isOpen>
<div data-testid="modal-content">Content</div>
</Modal>
);
expect(wrapper.find('div#modal-content').length).toBe(1);
expect(wrapper.contains('Some Title')).toBeTruthy();
expect(screen.getByRole('dialog')).toBeInTheDocument();
expect(screen.getByLabelText('Some Title')).toBeInTheDocument();
expect(screen.getByTestId('modal-content')).toBeInTheDocument();
});
});

View File

@ -1,7 +1,9 @@
import { cx } from '@emotion/css';
import { FocusScope } from '@react-aria/focus';
import { OverlayContainer } from '@react-aria/overlays';
import React, { PropsWithChildren, useCallback, useEffect } from 'react';
import { useDialog } from '@react-aria/dialog';
import { OverlayContainer, useOverlay } from '@react-aria/overlays';
import React, { PropsWithChildren, useRef } from 'react';
import { useTheme2 } from '../../themes';
import { IconName } from '../../types';
@ -39,33 +41,24 @@ export function Modal(props: PropsWithChildren<Props>) {
closeOnBackdropClick = true,
className,
contentClassName,
onDismiss: propsOnDismiss,
onDismiss,
onClickBackdrop,
trapFocus = true,
} = props;
const theme = useTheme2();
const styles = getModalStyles(theme);
const onDismiss = useCallback(() => {
if (propsOnDismiss) {
propsOnDismiss();
}
}, [propsOnDismiss]);
useEffect(() => {
const onEscKey = (ev: KeyboardEvent) => {
if (ev.key === 'Esc' || ev.key === 'Escape') {
onDismiss();
}
};
if (isOpen && closeOnEscape) {
document.addEventListener('keydown', onEscKey, false);
} else {
document.removeEventListener('keydown', onEscKey, false);
}
return () => {
document.removeEventListener('keydown', onEscKey, false);
};
}, [closeOnEscape, isOpen, onDismiss]);
const ref = useRef<HTMLDivElement>(null);
// Handle interacting outside the dialog and pressing
// the Escape key to close the modal.
const { overlayProps, underlayProps } = useOverlay(
{ isKeyboardDismissDisabled: closeOnEscape, isOpen, onClose: onDismiss },
ref
);
// Get props for the dialog and its title
const { dialogProps, titleProps } = useDialog({}, ref);
if (!isOpen) {
return null;
@ -78,16 +71,17 @@ export function Modal(props: PropsWithChildren<Props>) {
<div
className={styles.modalBackdrop}
onClick={onClickBackdrop || (closeOnBackdropClick ? onDismiss : undefined)}
{...underlayProps}
/>
<FocusScope contain={trapFocus} autoFocus restoreFocus>
{/*
tabIndex=-1 is needed here to support highlighting text within the modal when using FocusScope
see https://github.com/adobe/react-spectrum/issues/1604#issuecomment-781574668
*/}
<div tabIndex={-1} className={cx(styles.modal, className)}>
<div className={cx(styles.modal, className)} ref={ref} {...overlayProps} {...dialogProps}>
<div className={headerClass}>
{typeof title === 'string' && <DefaultModalHeader {...props} title={title} />}
{typeof title !== 'string' && title}
{typeof title === 'string' && <DefaultModalHeader {...props} title={title} id={titleProps.id} />}
{
// FIXME: custom title components won't get an accessible title.
// Do we really want to support them or shall we just limit this ModalTabsHeader?
typeof title !== 'string' && title
}
<div className={styles.modalHeaderClose}>
<IconButton aria-label="Close dialogue" surface="header" name="times" size="xl" onClick={onDismiss} />
</div>
@ -130,11 +124,12 @@ function ModalButtonRow({ leftItems, children }: { leftItems?: React.ReactNode;
Modal.ButtonRow = ModalButtonRow;
interface DefaultModalHeaderProps {
id?: string;
title: string;
icon?: IconName;
iconTooltip?: string;
}
function DefaultModalHeader({ icon, iconTooltip, title }: DefaultModalHeaderProps): JSX.Element {
return <ModalHeader icon={icon} iconTooltip={iconTooltip} title={title} />;
function DefaultModalHeader({ icon, iconTooltip, title, id }: DefaultModalHeaderProps): JSX.Element {
return <ModalHeader icon={icon} iconTooltip={iconTooltip} title={title} id={id} />;
}

View File

@ -5,6 +5,7 @@ import { useStyles2 } from '../../themes';
interface Props {
title: string;
id?: string;
/** @deprecated */
icon?: IconName;
/** @deprecated */
@ -12,12 +13,14 @@ interface Props {
}
/** @internal */
export const ModalHeader: React.FC<Props> = ({ icon, iconTooltip, title, children }) => {
export const ModalHeader: React.FC<Props> = ({ icon, iconTooltip, title, children, id }) => {
const styles = useStyles2(getModalStyles);
return (
<>
<h2 className={styles.modalHeaderTitle}>{title}</h2>
<h2 className={styles.modalHeaderTitle} id={id}>
{title}
</h2>
{children}
</>
);