Skip to content

Commit

Permalink
Merge pull request #837 from w3c/accessible-name-test-plan-report-sta…
Browse files Browse the repository at this point in the history
…tus-dialog

Fix aria-labeledby for BasicModal/BasicThemedModal
  • Loading branch information
stalgiag authored Nov 21, 2023
2 parents 878fe80 + 50cd6c8 commit c101ecc
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 6 deletions.
9 changes: 5 additions & 4 deletions client/components/common/BasicModal/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,21 @@ const BasicModal = ({
}, [show]);

const id = useMemo(() => {
return uniqueId('focus-trapped-modal-');
return uniqueId('modal-');
}, []);

return (
<FocusTrapper isActive={show} trappedElId={id}>
<FocusTrapper isActive={show} trappedElId={`focus-trapped-${id}`}>
<Modal
show={show}
id={id}
id={`focus-trapped-${id}`}
centered={centered}
animation={animation}
onHide={useOnHide ? handleHide || handleClose : null}
onExit={!useOnHide ? handleHide || handleClose : null}
/* Disabled due to buggy implementation which jumps the page */
autoFocus={false}
aria-labelledby="basic-modal"
aria-labelledby={`title-${id}`}
dialogClassName={dialogClassName}
backdrop={staticBackdrop ? 'static' : true}
>
Expand All @@ -70,6 +70,7 @@ const BasicModal = ({
as={ModalTitleStyle}
tabIndex="-1"
ref={headerRef}
id={`title-${id}`}
>
{title}
</Modal.Title>
Expand Down
10 changes: 8 additions & 2 deletions client/components/common/BasicThemedModal/index.jsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import React, { useEffect, useRef } from 'react';
import React, { useEffect, useMemo, useRef } from 'react';
import PropTypes from 'prop-types';
import { Button, Modal } from 'react-bootstrap';
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
import { faExclamationTriangle } from '@fortawesome/free-solid-svg-icons';
import styled from '@emotion/styled';
import { uniqueId } from 'lodash';

const ModalTitleStyle = styled.h1`
border: 0;
Expand Down Expand Up @@ -50,6 +51,10 @@ const BasicThemedModal = ({
headerRef.current.focus();
}, [show]);

const id = useMemo(() => {
return uniqueId('modal-');
}, []);

return (
<>
<Modal
Expand All @@ -59,7 +64,7 @@ const BasicThemedModal = ({
onExit={handleClose}
/* Disabled due to buggy implementation which jumps the page */
autoFocus={false}
aria-labelledby="basic-modal"
aria-labelledby={`title-${id}`}
dialogClassName={dialogClassName}
>
<ColorStrip theme={theme} />
Expand All @@ -68,6 +73,7 @@ const BasicThemedModal = ({
as={ModalTitleStyle}
tabIndex="-1"
ref={headerRef}
id={`title-${id}`}
>
<ModalInnerSectionContainer>
<FontAwesomeIcon
Expand Down
21 changes: 21 additions & 0 deletions client/tests/BasicModal.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* @jest-environment jsdom
*/

import React from 'react';
import { render } from '@testing-library/react';
import BasicModal from '../components/common/BasicModal/';
import '@testing-library/jest-dom/extend-expect';

describe('<BasicModal />', () => {
it('has aria-labelledby matching the modal title id', () => {
const { getByRole, getByText } = render(
<BasicModal show title="Test Title" content="Test Content" />
);

const modal = getByRole('dialog');
const title = getByText('Test Title');

expect(modal).toHaveAttribute('aria-labelledby', title.id);
});
});

0 comments on commit c101ecc

Please sign in to comment.