Skip to content

fix: accessibility issues on outline and unit pages #1580

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

Merged
merged 10 commits into from
Jan 31, 2025
Merged
19 changes: 9 additions & 10 deletions src/course-home/outline-tab/OutlineTab.jsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import React, { useEffect, useState } from 'react';
import { useEffect, useRef, useState } from 'react';
import { useLocation, useNavigate } from 'react-router-dom';
import { useSelector } from 'react-redux';
import { sendTrackEvent } from '@edx/frontend-platform/analytics';
import { getAuthenticatedUser } from '@edx/frontend-platform/auth';
import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
import { useIntl } from '@edx/frontend-platform/i18n';
import { Button } from '@openedx/paragon';
import { PluginSlot } from '@openedx/frontend-plugin-framework';
import { AlertList } from '../../generic/user-messages';
Expand All @@ -29,7 +29,8 @@
import ProctoringInfoPanel from './widgets/ProctoringInfoPanel';
import AccountActivationAlert from '../../alerts/logistration-alert/AccountActivationAlert';

const OutlineTab = ({ intl }) => {
const OutlineTab = () => {
const intl = useIntl();
const {
courseId,
proctoringPanelStatus,
Expand All @@ -42,6 +43,8 @@
userTimezone,
} = useModel('courseHomeMeta', courseId);

const expandButtonRef = useRef();

const {
accessExpiration,
courseBlocks: {
Expand Down Expand Up @@ -159,12 +162,12 @@
</>
)}
<StartOrResumeCourseCard />
<WelcomeMessage courseId={courseId} />
<WelcomeMessage courseId={courseId} nextElementRef={expandButtonRef} />
{rootCourseId && (
<>
<div className="row w-100 m-0 mb-3 justify-content-end">
<div className="col-12 col-md-auto p-0">
<Button variant="outline-primary" block onClick={() => { setExpandAll(!expandAll); }}>
<Button ref={expandButtonRef} variant="outline-primary" block onClick={() => { setExpandAll(!expandAll); }}>

Check warning on line 170 in src/course-home/outline-tab/OutlineTab.jsx

View check run for this annotation

Codecov / codecov/patch

src/course-home/outline-tab/OutlineTab.jsx#L170

Added line #L170 was not covered by tests
{expandAll ? intl.formatMessage(messages.collapseAll) : intl.formatMessage(messages.expandAll)}
</Button>
</div>
Expand Down Expand Up @@ -225,8 +228,4 @@
);
};

OutlineTab.propTypes = {
intl: intlShape.isRequired,
};

export default injectIntl(OutlineTab);
export default OutlineTab;
12 changes: 12 additions & 0 deletions src/course-home/outline-tab/OutlineTab.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,18 @@ describe('Outline Tab', () => {
showMoreButton = screen.getByRole('button', { name: 'Show More' });
expect(showMoreButton).toBeInTheDocument();
});

fit('dismisses message', async () => {
expect(screen.getByTestId('alert-container-welcome')).toBeInTheDocument();
const dismissButton = screen.queryByRole('button', { name: 'Dismiss' });
const expandButton = screen.queryByRole('button', { name: 'Expand all' });

fireEvent.click(dismissButton);

expect(expandButton).toHaveFocus();

expect(screen.queryByText('Welcome Message')).toBeNull();
});
});

it('ignores comments and misformatted HTML', async () => {
Expand Down
61 changes: 36 additions & 25 deletions src/course-home/outline-tab/widgets/WelcomeMessage.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { useState, useMemo } from 'react';
import { useState, useMemo, useRef } from 'react';
import PropTypes from 'prop-types';

import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
import { useIntl } from '@edx/frontend-platform/i18n';
import { Alert, Button, TransitionReplace } from '@openedx/paragon';
import truncate from 'truncate-html';

Expand All @@ -11,11 +11,13 @@
import { useModel } from '../../../generic/model-store';
import { dismissWelcomeMessage } from '../../data/thunks';

const WelcomeMessage = ({ courseId, intl }) => {
const WelcomeMessage = ({ courseId, nextElementRef }) => {
const intl = useIntl();
const {
welcomeMessageHtml,
} = useModel('outline', courseId);

const messageBodyRef = useRef();
const [display, setDisplay] = useState(true);

// welcomeMessageHtml can contain comments or malformatted HTML which can impact the length that determines
Expand Down Expand Up @@ -49,46 +51,55 @@
dismissible
show={display}
onClose={() => {
nextElementRef.current?.focus();
setDisplay(false);
dispatch(dismissWelcomeMessage(courseId));
}}
className="raised-card"
actions={messageCanBeShortened ? [
<Button
onClick={() => setShowShortMessage(!showShortMessage)}
onClick={() => {

Check warning on line 61 in src/course-home/outline-tab/widgets/WelcomeMessage.jsx

View check run for this annotation

Codecov / codecov/patch

src/course-home/outline-tab/widgets/WelcomeMessage.jsx#L61

Added line #L61 was not covered by tests
if (showShortMessage && messageBodyRef.current) {
messageBodyRef.current.focus();

Check warning on line 63 in src/course-home/outline-tab/widgets/WelcomeMessage.jsx

View check run for this annotation

Codecov / codecov/patch

src/course-home/outline-tab/widgets/WelcomeMessage.jsx#L63

Added line #L63 was not covered by tests
}

setShowShortMessage(!showShortMessage);

Check warning on line 66 in src/course-home/outline-tab/widgets/WelcomeMessage.jsx

View check run for this annotation

Codecov / codecov/patch

src/course-home/outline-tab/widgets/WelcomeMessage.jsx#L66

Added line #L66 was not covered by tests
}}
variant="outline-primary"
>
{showShortMessage ? intl.formatMessage(messages.welcomeMessageShowMoreButton)
: intl.formatMessage(messages.welcomeMessageShowLessButton)}
</Button>,
] : []}
>
<TransitionReplace className="mb-3" enterDuration={400} exitDuration={200}>
{showShortMessage ? (
<LmsHtmlFragment
className="inline-link"
data-testid="short-welcome-message-iframe"
key="short-html"
html={shortWelcomeMessageHtml}
title={intl.formatMessage(messages.welcomeMessage)}
/>
) : (
<LmsHtmlFragment
className="inline-link"
data-testid="long-welcome-message-iframe"
key="full-html"
html={cleanedWelcomeMessageHtml}
title={intl.formatMessage(messages.welcomeMessage)}
/>
)}
</TransitionReplace>
<div ref={messageBodyRef} tabIndex="-1">
<TransitionReplace className="mb-3" enterDuration={400} exitDuration={200}>
{showShortMessage ? (
<LmsHtmlFragment
className="inline-link"
data-testid="short-welcome-message-iframe"
key="short-html"
html={shortWelcomeMessageHtml}
title={intl.formatMessage(messages.welcomeMessage)}
/>
) : (
<LmsHtmlFragment
className="inline-link"
data-testid="long-welcome-message-iframe"
key="full-html"
html={cleanedWelcomeMessageHtml}
title={intl.formatMessage(messages.welcomeMessage)}
/>
)}
</TransitionReplace>
</div>
</Alert>
);
};

WelcomeMessage.propTypes = {
courseId: PropTypes.string.isRequired,
intl: intlShape.isRequired,
nextElementRef: PropTypes.shape({ current: PropTypes.instanceOf(HTMLInputElement) }),
};

export default injectIntl(WelcomeMessage);
export default WelcomeMessage;
20 changes: 10 additions & 10 deletions src/courseware/course/bookmark/BookmarkButton.jsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import React, { useCallback } from 'react';
import { useCallback } from 'react';
import PropTypes from 'prop-types';
import { StatefulButton } from '@openedx/paragon';
import { Icon, StatefulButton } from '@openedx/paragon';
import { FormattedMessage } from '@edx/frontend-platform/i18n';
import { useDispatch } from 'react-redux';
import BookmarkOutlineIcon from './BookmarkOutlineIcon';
import BookmarkFilledIcon from './BookmarkFilledIcon';
import { Bookmark, BookmarkBorder } from '@openedx/paragon/icons';
import { removeBookmark, addBookmark } from './data/thunks';

const addBookmarkLabel = (
Expand Down Expand Up @@ -42,21 +41,22 @@ const BookmarkButton = ({
return (
<StatefulButton
variant="link"
className="px-1 ml-n1 btn-sm text-primary-500"
className={`px-1 ml-n1 btn-sm text-primary-500 ${isProcessing && 'disabled'}`}
onClick={toggleBookmark}
state={state}
disabledStates={['defaultProcessing', 'bookmarkedProcessing']}
aria-busy={isProcessing}
disabled={isProcessing}
labels={{
default: addBookmarkLabel,
defaultProcessing: addBookmarkLabel,
bookmarked: hasBookmarkLabel,
bookmarkedProcessing: hasBookmarkLabel,
}}
icons={{
default: <BookmarkOutlineIcon className="text-primary" />,
defaultProcessing: <BookmarkOutlineIcon className="text-primary" />,
bookmarked: <BookmarkFilledIcon className="text-primary" />,
bookmarkedProcessing: <BookmarkFilledIcon className="text-primary" />,
default: <Icon src={BookmarkBorder} className="text-primary" />,
defaultProcessing: <Icon src={BookmarkBorder} className="text-primary" />,
bookmarked: <Icon src={Bookmark} className="text-primary" />,
bookmarkedProcessing: <Icon src={Bookmark} className="text-primary" />,
}}
/>
);
Expand Down
7 changes: 0 additions & 7 deletions src/courseware/course/bookmark/BookmarkFilledIcon.jsx

This file was deleted.

7 changes: 0 additions & 7 deletions src/courseware/course/bookmark/BookmarkOutlineIcon.jsx

This file was deleted.

2 changes: 0 additions & 2 deletions src/courseware/course/bookmark/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
export { default as BookmarkButton } from './BookmarkButton';
export { default as BookmarkFilledIcon } from './BookmarkFilledIcon';
export { default as BookmarkOutlineIcon } from './BookmarkFilledIcon';
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ exports[`Unit component output snapshot: not bookmarked, do not show content 1`]
unitTitle="unit-title"
/>
</div>
<h2
<p
className="sr-only"
>
Level 2 headings may be created by course providers in the future.
</h2>
</p>
<BookmarkButton
isBookmarked={false}
isProcessing={false}
Expand Down
2 changes: 1 addition & 1 deletion src/courseware/course/sequence/Unit/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const Unit = ({
<h3 className="h3">{unit.title}</h3>
<UnitTitleSlot courseId={courseId} unitId={id} unitTitle={unit.title} />
</div>
<h2 className="sr-only">{formatMessage(messages.headerPlaceholder)}</h2>
<p className="sr-only">{formatMessage(messages.headerPlaceholder)}</p>
<BookmarkButton
unitId={unit.id}
isBookmarked={unit.bookmarked}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import React, { useCallback } from 'react';
import { useCallback } from 'react';
import { Link, useLocation } from 'react-router-dom';
import PropTypes from 'prop-types';
import { connect, useSelector } from 'react-redux';
import classNames from 'classnames';
import { Button } from '@openedx/paragon';
import { Button, Icon } from '@openedx/paragon';
import { Bookmark } from '@openedx/paragon/icons';

import UnitIcon from './UnitIcon';
import CompleteIcon from './CompleteIcon';
import BookmarkFilledIcon from '../../bookmark/BookmarkFilledIcon';

const UnitButton = ({
onClick,
Expand Down Expand Up @@ -46,7 +46,9 @@ const UnitButton = ({
{showTitle && <span className="unit-title">{title}</span>}
{showCompletion && complete ? <CompleteIcon size="sm" className="text-success ml-2" /> : null}
{bookmarked ? (
<BookmarkFilledIcon
<Icon
data-testid="bookmark-icon"
src={Bookmark}
className="text-primary small position-absolute"
style={{ top: '-3px', right: '5px' }}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,17 +63,17 @@ describe('Unit Button', () => {
});

it('does not show bookmark', () => {
const { container } = render(<UnitButton {...mockData} />);
container.querySelectorAll('svg').forEach(icon => {
expect(icon).not.toHaveClass('fa-bookmark');
});
const { queryByTestId } = render(<UnitButton {...mockData} />);
expect(queryByTestId('bookmark-icon')).toBeNull();
});

it('shows bookmark', () => {
const { container } = render(<UnitButton {...mockData} unitId={bookmarkedUnit.id} />, { wrapWithRouter: true });
const buttonIcons = container.querySelectorAll('svg');
expect(buttonIcons).toHaveLength(3);
expect(buttonIcons[2]).toHaveClass('fa-bookmark');

const bookmarkIcon = buttonIcons[2].closest('span');
expect(bookmarkIcon.getAttribute('data-testid')).toBe('bookmark-icon');
});

it('handles the click', () => {
Expand Down