fix(ARCH-662): fix design system modal accessibility (#4960)

* fix(ARCH-662): fix design system modal accessibility * improve accessibility and reopen test
Talend · Oct 30, 2023 · b8406b6 · b8406b6
1 parent 2177ddf
commit b8406b6
Show file tree

Hide file tree

Showing 5 changed files with 96 additions and 90 deletions.
diff --git a/.changeset/nasty-birds-cheat.md b/.changeset/nasty-birds-cheat.md
@@ -0,0 +1,5 @@
+---
+'@talend/design-system': patch
+---
+
+Fix Design System accessibility for Modal
diff --git a/packages/design-system/src/components/Modal/Modal.module.scss b/packages/design-system/src/components/Modal/Modal.module.scss
@@ -1,31 +1,29 @@
 @use '~@talend/design-tokens/lib/tokens';
 
 .modal-backdrop::before,
-.modal-container {
+.modal-backdrop {
 	position: fixed;
 	top: 0;
 	left: 0;
 	right: 0;
 	bottom: 0;
 }
 
-.modal-backdrop {
-	&::before {
-		z-index: tokens.$coral-elevation-layer-interactive-front;
-		content: '';
-		background-color: tokens.$coral-color-assistive-background;
-		opacity: tokens.$coral-opacity-l;
-	}
+.modal-backdrop::before {
+	content: '';
+	background-color: tokens.$coral-color-assistive-background;
+	opacity: tokens.$coral-opacity-l;
 }
 
-.modal-container {
-	z-index: calc(tokens.$coral-elevation-layer-interactive-front + 1);
+.modal-backdrop {
+	z-index: tokens.$coral-elevation-layer-interactive-front;
 	display: flex;
 	justify-content: center;
 	align-items: center;
 }
 
 .modal {
+	z-index: calc(tokens.$coral-elevation-layer-interactive-front + 1);
 	position: fixed;
 	min-width: 660px;
 	max-width: 95%;

diff --git a/packages/design-system/src/components/Modal/Modal.test.tsx b/packages/design-system/src/components/Modal/Modal.test.tsx
@@ -5,7 +5,7 @@ import { render } from '@testing-library/react';
 import { Modal } from './';
 
 describe('Message', () => {
-	xit('should render a11y html', async () => {
+	it('should render a11y html', async () => {
 		const { container } = render(
 			<main>
 				<Modal visible header={{ title: '(Default story title)' }} onClose={() => jest.fn()}>

diff --git a/packages/design-system/src/components/Modal/Modal.tsx b/packages/design-system/src/components/Modal/Modal.tsx
@@ -71,6 +71,7 @@ export function Modal(props: ModalPropsType): ReactElement {
 
 	const backdropRef = useRef<HTMLDivElement>(null);
 	const dialogRef = useRef<HTMLDivElement>(null);
+	const titleId = 'modal-header-text-title';
 
 	useEffect(() => {
 		dialogRef.current?.focus();
@@ -98,92 +99,89 @@ export function Modal(props: ModalPropsType): ReactElement {
 				className={styles['modal-backdrop']}
 				data-test="modal.backdrop"
 				data-testid="modal.backdrop"
+				onClick={onClickBackdropHandler}
+				ref={backdropRef}
 			>
-				<div
-					aria-hidden
-					className={styles['modal-container']}
-					onClick={onClickBackdropHandler}
-					ref={backdropRef}
+				<Dialog
+					{...rest}
+					visible={dialog.visible}
+					data-test="modal"
+					data-testid="modal"
+					className={styles.modal}
+					hide={preventEscaping ? () => undefined : () => onCloseHandler()}
+					aria-labelledby={titleId}
+					ref={dialogRef}
 				>
-					<Dialog
-						{...rest}
-						visible={dialog.visible}
-						data-test="modal"
-						data-testid="modal"
-						className={styles.modal}
-						hide={preventEscaping ? () => undefined : () => onCloseHandler()}
-						ref={dialogRef}
-					>
-						<StackVertical gap={0}>
-							<div className={styles.modal__header}>
-								{header.icon && (
-									<ModalIcon
-										icon={header.icon}
-										data-test="modal.header.icon"
-										data-testid="modal.header.icon"
-									/>
-								)}
-								<div className={styles['modal-header-text']}>
+					<StackVertical gap={0}>
+						<div className={styles.modal__header}>
+							{header.icon && (
+								<ModalIcon
+									icon={header.icon}
+									data-test="modal.header.icon"
+									data-testid="modal.header.icon"
+								/>
+							)}
+							<div className={styles['modal-header-text']}>
+								<span
+									id={titleId}
+									className={styles['modal-header-text__title']}
+									data-test="modal.header.title"
+								>
+									{header.title}
+								</span>
+								{header.description && (
 									<span
-										className={styles['modal-header-text__title']}
-										data-test="modal.header.title"
+										className={styles['modal-header-text__description']}
+										data-test="modal.header.description"
 									>
-										{header.title}
+										{header.description}
 									</span>
-									{header.description && (
-										<span
-											className={styles['modal-header-text__description']}
-											data-test="modal.header.description"
-										>
-											{header.description}
-										</span>
-									)}
-								</div>
+								)}
 							</div>
+						</div>
 
-							<div className={styles.modal__content} data-test="modal.content">
-								{children}
-							</div>
+						<div className={styles.modal__content} data-test="modal.content">
+							{children}
+						</div>
 
-							<div className={styles.modal__buttons} data-test="modal.buttons">
-								<StackHorizontal gap="XS" justify="end">
-									{!preventEscaping && (
-										<span className={styles['close-button']}>
-											<ButtonSecondary
-												onClick={() => onCloseHandler()}
-												data-test="modal.buttons.close"
-												data-testid="modal.buttons.close"
-												data-feature="modal.buttons.close"
-											>
-												{primaryAction || secondaryAction
-													? t('CANCEL', 'Cancel')
-													: t('CLOSE', 'Close')}
-											</ButtonSecondary>
-										</span>
-									)}
-
-									{secondaryAction && (
+						<div className={styles.modal__buttons} data-test="modal.buttons">
+							<StackHorizontal gap="XS" justify="end">
+								{!preventEscaping && (
+									<span className={styles['close-button']}>
 										<ButtonSecondary
-											data-test="modal.buttons.secondary"
-											data-testid="modal.buttons.secondary"
-											data-feature="modal.buttons.secondary"
-											{...secondaryAction}
-										/>
-									)}
-
-									{primaryAction && (
-										<PrimaryAction
-											data-testid="modal.buttons.primary"
-											data-test="modal.buttons.primary"
-											data-feature="modal.buttons.primary"
-											{...primaryAction}
-										/>
-									)}
-								</StackHorizontal>
-							</div>
-						</StackVertical>
-					</Dialog>
-				</div>
+											onClick={() => onCloseHandler()}
+											data-test="modal.buttons.close"
+											data-testid="modal.buttons.close"
+											data-feature="modal.buttons.close"
+										>
+											{primaryAction || secondaryAction
+												? t('CANCEL', 'Cancel')
+												: t('CLOSE', 'Close')}
+										</ButtonSecondary>
+									</span>
+								)}
+
+								{secondaryAction && (
+									<ButtonSecondary
+										data-test="modal.buttons.secondary"
+										data-testid="modal.buttons.secondary"
+										data-feature="modal.buttons.secondary"
+										{...secondaryAction}
+									/>
+								)}
+
+								{primaryAction && (
+									<PrimaryAction
+										data-testid="modal.buttons.primary"
+										data-test="modal.buttons.primary"
+										data-feature="modal.buttons.primary"
+										{...primaryAction}
+									/>
+								)}
+							</StackHorizontal>
+						</div>
+					</StackVertical>
+				</Dialog>
 			</DialogBackdrop>
 		</>
 	);

diff --git a/packages/design-system/src/components/Modal/Primitives/DialogBackdrop.tsx b/packages/design-system/src/components/Modal/Primitives/DialogBackdrop.tsx
@@ -1,16 +1,21 @@
+import { Ref, forwardRef } from 'react';
 import { DialogState } from './DialogState';
 import { Portal } from './Portal';
 
 export type DialogBackdropProps = React.HTMLAttributes<HTMLDivElement> & DialogState;
 
-export function DialogBackdrop(props: DialogBackdropProps) {
+function BaseDialogBackdrop(props: DialogBackdropProps, ref: Ref<HTMLDivElement>) {
 	const { children, visible, ...rest } = props;
 	if (!visible) {
 		return null;
 	}
 	return (
 		<Portal>
-			<div {...rest}>{children}</div>;
+			<div {...rest} ref={ref}>
+				{children}
+			</div>
 		</Portal>
 	);
 }
+
+export const DialogBackdrop = forwardRef(BaseDialogBackdrop);