Skip to content

feat: improved accessibility skipping to main content #1599

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
7 changes: 6 additions & 1 deletion src/course-home/dates-tab/DatesTab.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,17 @@ import SuggestedScheduleHeader from '../suggested-schedule-messaging/SuggestedSc
import ShiftDatesAlert from '../suggested-schedule-messaging/ShiftDatesAlert';
import UpgradeToCompleteAlert from '../suggested-schedule-messaging/UpgradeToCompleteAlert';
import UpgradeToShiftDatesAlert from '../suggested-schedule-messaging/UpgradeToShiftDatesAlert';
import { useScrollToContent } from '../../generic/hooks';

const MAIN_CONTENT_ID = 'main-content-heading';

const DatesTab = ({ intl }) => {
const {
courseId,
} = useSelector(state => state.courseHome);

useScrollToContent(MAIN_CONTENT_ID);

const {
isSelfPaced,
org,
Expand All @@ -43,7 +48,7 @@ const DatesTab = ({ intl }) => {

return (
<>
<div role="heading" aria-level="1" className="h2 my-3">
<div id={MAIN_CONTENT_ID} tabIndex="-1" role="heading" aria-level="1" className="h2 my-3">
{intl.formatMessage(messages.title)}
</div>
{isSelfPaced && hasDeadlines && (
Expand Down
11 changes: 9 additions & 2 deletions src/course-home/progress-tab/ProgressHeader.jsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,26 @@
import React from 'react';

import { getAuthenticatedUser } from '@edx/frontend-platform/auth';
import { useIntl } from '@edx/frontend-platform/i18n';
import { Button } from '@openedx/paragon';
import { useIntl } from '@edx/frontend-platform/i18n';
import { useSelector } from 'react-redux';

import { useModel } from '../../generic/model-store';
import { useScrollToContent } from '../../generic/hooks';

import messages from './messages';

const MAIN_CONTENT_ID = 'main-content-heading';

const ProgressHeader = () => {
const intl = useIntl();
const {
courseId,
targetUserId,
} = useSelector(state => state.courseHome);

useScrollToContent(MAIN_CONTENT_ID);

const { administrator, userId } = getAuthenticatedUser();

const { studioUrl, username } = useModel('progress', courseId);
Expand All @@ -26,7 +33,7 @@ const ProgressHeader = () => {

return (
<div className="row w-100 m-0 mt-3 mb-4 justify-content-between">
<h1>{pageTitle}</h1>
<h1 id={MAIN_CONTENT_ID} tabIndex="-1">{pageTitle}</h1>
{administrator && studioUrl && (
<Button variant="outline-primary" size="sm" className="align-self-center" href={studioUrl}>
{intl.formatMessage(messages.studioLink)}
Expand Down
7 changes: 7 additions & 0 deletions src/courseware/course/bookmark/BookmarkFilledIcon.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import React from 'react';
import { Icon } from '@openedx/paragon';
import { Bookmark } from '@openedx/paragon/icons';

const BookmarkFilledIcon = (props) => <Icon src={Bookmark} screenReaderText="Bookmark" {...props} />;

export default BookmarkFilledIcon;
1 change: 1 addition & 0 deletions src/courseware/course/bookmark/index.js
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export { default as BookmarkButton } from './BookmarkButton';
export { default as BookmarkFilledIcon } from './BookmarkFilledIcon';
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ describe('Sequence Navigation', () => {
const onNavigate = jest.fn();
render(<SequenceNavigation {...mockData} {...{ onNavigate }} />, { wrapWithRouter: true });

const unitButtons = screen.getAllByRole('link', { name: /\d+/ });
const unitButtons = screen.getAllByRole('tabpanel', { name: /\d+/ });
expect(unitButtons).toHaveLength(unitButtons.length);
unitButtons.forEach(button => fireEvent.click(button));
expect(onNavigate).toHaveBeenCalledTimes(unitButtons.length);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('Sequence Navigation Dropdown', () => {
});
const dropdownMenu = container.querySelector('.dropdown-menu');
// Only the current unit should be marked as active.
getAllByRole(dropdownMenu, 'link', { hidden: true }).forEach(button => {
getAllByRole(dropdownMenu, 'tabpanel', { hidden: true }).forEach(button => {
if (button.textContent === unit.display_name) {
expect(button).toHaveClass('active');
} else {
Expand All @@ -72,7 +72,7 @@ describe('Sequence Navigation Dropdown', () => {
fireEvent.click(dropdownToggle);
});
const dropdownMenu = container.querySelector('.dropdown-menu');
getAllByRole(dropdownMenu, 'link', { hidden: true }).forEach(button => fireEvent.click(button));
getAllByRole(dropdownMenu, 'tabpanel', { hidden: true }).forEach(button => fireEvent.click(button));
expect(onNavigate).toHaveBeenCalledTimes(unitBlocks.length);
unitBlocks.forEach((unit, index) => {
expect(onNavigate).toHaveBeenNthCalledWith(index + 1, unit.id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,14 @@ const SequenceNavigationTabs = ({
style={shouldDisplayDropdown ? invisibleStyle : null}
ref={containerRef}
>
{unitIds.map(buttonUnitId => (
{unitIds.map((buttonUnitId, idx) => (
<UnitButton
key={buttonUnitId}
unitId={buttonUnitId}
isActive={unitId === buttonUnitId}
showCompletion={showCompletion}
onClick={onNavigate}
unitIndex={idx}
/>
))}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ describe('Sequence Navigation Tabs', () => {
useIndexOfLastVisibleChild.mockReturnValue([0, null, null]);
render(<SequenceNavigationTabs {...mockData} />, { wrapWithRouter: true });

expect(screen.getAllByRole('link')).toHaveLength(unitBlocks.length);
expect(screen.getAllByRole('tabpanel')).toHaveLength(unitBlocks.length);
});

it('renders unit buttons and dropdown button', async () => {
Expand All @@ -54,15 +54,15 @@ describe('Sequence Navigation Tabs', () => {
const booyah = render(<SequenceNavigationTabs {...mockData} />, { wrapWithRouter: true });

// wait for links to appear so we aren't testing an empty div
await screen.findAllByRole('link');
await screen.findAllByRole('tabpanel');

container = booyah.container;

const dropdownToggle = container.querySelector('.dropdown-toggle');
await userEvent.click(dropdownToggle);

const dropdownMenu = container.querySelector('.dropdown');
const dropdownButtons = getAllByRole(dropdownMenu, 'link');
const dropdownButtons = getAllByRole(dropdownMenu, 'tabpanel');
expect(dropdownButtons).toHaveLength(unitBlocks.length);
expect(screen.getByRole('button', { name: `${activeBlockNumber} of ${unitBlocks.length}` }))
.toHaveClass('dropdown-toggle');
Expand Down
41 changes: 35 additions & 6 deletions src/courseware/course/sequence/sequence-navigation/UnitButton.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ import { Link, useLocation } from 'react-router-dom';
import PropTypes from 'prop-types';
import { connect, useSelector } from 'react-redux';
import classNames from 'classnames';
import { Button, Icon } from '@openedx/paragon';
import { Bookmark } from '@openedx/paragon/icons';
import { Button } from '@openedx/paragon';

import UnitIcon from './UnitIcon';
import CompleteIcon from './CompleteIcon';
import BookmarkFilledIcon from '../../bookmark/BookmarkFilledIcon';
import { useScrollToContent } from '../../../../generic/hooks';

const UnitButton = ({
onClick,
Expand All @@ -20,7 +21,10 @@ const UnitButton = ({
unitId,
className,
showTitle,
unitIndex,
}) => {
useScrollToContent(isActive ? `${title}-${unitIndex}` : null);

const { courseId, sequenceId } = useSelector(state => state.courseware);
const { pathname } = useLocation();
const basePath = `/course/${courseId}/${sequenceId}/${unitId}`;
Expand All @@ -30,6 +34,23 @@ const UnitButton = ({
onClick(unitId);
}, [onClick, unitId]);

const handleKeyDown = (event) => {
if (event.key === 'Enter' || event.key === ' ') {
onClick(unitId);

const performFocus = () => {
const targetElement = document.getElementById('bookmark-button');
if (targetElement) {
targetElement.focus();
}
};

requestAnimationFrame(() => {
requestAnimationFrame(performFocus);
});
}
};

return (
<Button
className={classNames({
Expand All @@ -39,18 +60,25 @@ const UnitButton = ({
variant="link"
onClick={handleClick}
title={title}
role="tabpanel"
tabIndex={isActive ? 0 : -1}
aria-controls={title}
id={`${title}-${unitIndex}`}
aria-labelledby={title}
onKeyDown={handleKeyDown}
as={Link}
to={unitPath}
>
<UnitIcon type={contentType} />
{showTitle && <span className="unit-title">{title}</span>}
{showCompletion && complete ? <CompleteIcon size="sm" className="text-success ml-2" /> : null}
{bookmarked ? (
<Icon
<BookmarkFilledIcon
className="unit-filled-bookmark text-primary small position-absolute"
data-testid="bookmark-icon"
src={Bookmark}
className="text-primary small position-absolute"
style={{ top: '-3px', right: '5px' }}
style={{
top: '-3px', right: '2px', height: '20px', width: '20px',
}}
/>
) : null}
</Button>
Expand All @@ -68,6 +96,7 @@ UnitButton.propTypes = {
showTitle: PropTypes.bool,
title: PropTypes.string.isRequired,
unitId: PropTypes.string.isRequired,
unitIndex: PropTypes.number.isRequired,
};

UnitButton.defaultProps = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import { Factory } from 'rosie';
import { act, waitFor } from '@testing-library/react';
import {
fireEvent, initializeTestStore, render, screen,
} from '../../../../setupTest';
Expand Down Expand Up @@ -28,17 +29,35 @@ describe('Unit Button', () => {
mockData = {
unitId: unit.id,
onClick: () => {},
unitIndex: courseMetadata.id,
};

global.requestAnimationFrame = jest.fn((cb) => {
setImmediate(cb);
});
});

it('hides title by default', () => {
render(<UnitButton {...mockData} />, { wrapWithRouter: true });
expect(screen.getByRole('link')).not.toHaveTextContent(unit.display_name);
expect(screen.getByRole('tabpanel')).not.toHaveTextContent(unit.display_name);
});

it('shows title', () => {
render(<UnitButton {...mockData} showTitle />, { wrapWithRouter: true });
expect(screen.getByRole('link')).toHaveTextContent(unit.display_name);
expect(screen.getByRole('tabpanel')).toHaveTextContent(unit.display_name);
});

it('check button attributes', () => {
render(<UnitButton {...mockData} showTitle />, { wrapWithRouter: true });
expect(screen.getByRole('tabpanel')).toHaveAttribute('id', `${unit.display_name}-${courseMetadata.id}`);
expect(screen.getByRole('tabpanel')).toHaveAttribute('aria-controls', unit.display_name);
expect(screen.getByRole('tabpanel')).toHaveAttribute('aria-labelledby', unit.display_name);
expect(screen.getByRole('tabpanel')).toHaveAttribute('tabindex', '-1');
});

it('button with isActive prop has tabindex 0', () => {
render(<UnitButton {...mockData} isActive />, { wrapWithRouter: true });
expect(screen.getByRole('tabpanel')).toHaveAttribute('tabindex', '0');
});

it('does not show completion for non-completed unit', () => {
Expand Down Expand Up @@ -79,7 +98,65 @@ describe('Unit Button', () => {
it('handles the click', () => {
const onClick = jest.fn();
render(<UnitButton {...mockData} onClick={onClick} />, { wrapWithRouter: true });
fireEvent.click(screen.getByRole('link'));
fireEvent.click(screen.getByRole('tabpanel'));
expect(onClick).toHaveBeenCalledTimes(1);
});

it('focuses the bookmark button after key press', async () => {
jest.useFakeTimers();

const { container } = render(
<>
<UnitButton {...mockData} />
<button id="bookmark-button" type="button">Bookmark</button>
</>,
{ wrapWithRouter: true },
);
const unitButton = container.querySelector('[role="tabpanel"]');

fireEvent.keyDown(unitButton, { key: 'Enter' });

await act(async () => {
jest.runAllTimers();
});

await waitFor(() => {
expect(document.activeElement).toBe(document.getElementById('bookmark-button'));
});

jest.useRealTimers();
});

it('calls onClick and focuses bookmark button on Enter or Space key press', async () => {
const onClick = jest.fn();
const { container } = render(
<>
<UnitButton {...mockData} onClick={onClick} />
<button id="bookmark-button" type="button">Bookmark</button>
</>,
{ wrapWithRouter: true },
);

const unitButton = container.querySelector('[role="tabpanel"]');

await act(async () => {
fireEvent.keyDown(unitButton, { key: 'Enter' });
});

await waitFor(() => {
expect(requestAnimationFrame).toHaveBeenCalledTimes(2);
expect(onClick).toHaveBeenCalledTimes(1);
expect(document.activeElement).toBe(document.getElementById('bookmark-button'));
});

await act(async () => {
fireEvent.keyDown(unitButton, { key: ' ' });
});

await waitFor(() => {
expect(requestAnimationFrame).toHaveBeenCalledTimes(4);
expect(onClick).toHaveBeenCalledTimes(2);
expect(document.activeElement).toBe(document.getElementById('bookmark-button'));
});
});
});
Loading