Skip to content
Draft
Show file tree
Hide file tree
Changes from 2 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
2 changes: 2 additions & 0 deletions src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,5 @@ export const LOADED = 'loaded';
export const FAILED = 'failed';
export const DENIED = 'denied';
export type StatusValue = typeof LOADING | typeof LOADED | typeof FAILED | typeof DENIED;

export const MAIN_CONTENT_ID = 'main-content-heading';
12 changes: 11 additions & 1 deletion src/course-home/dates-tab/DatesTab.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { useSelector } from 'react-redux';
import { sendTrackEvent } from '@edx/frontend-platform/analytics';
import { useIntl } from '@edx/frontend-platform/i18n';

import { MAIN_CONTENT_ID } from '@src/constants';
import { useScrollToContent } from '@src/generic/hooks';
import messages from './messages';
import Timeline from './timeline/Timeline';

Expand All @@ -20,6 +22,8 @@ const DatesTab = () => {
courseId,
} = useSelector(state => state.courseHome);

useScrollToContent(MAIN_CONTENT_ID);

const {
isSelfPaced,
org,
Expand All @@ -44,7 +48,13 @@ const DatesTab = () => {

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
6 changes: 5 additions & 1 deletion src/course-home/progress-tab/ProgressHeader.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { useIntl } from '@edx/frontend-platform/i18n';
import { Button } from '@openedx/paragon';
import { useSelector } from 'react-redux';

import { useScrollToContent } from '@src/generic/hooks';
import { MAIN_CONTENT_ID } from '@src/constants';
import { useModel } from '../../generic/model-store';

import messages from './messages';
Expand All @@ -18,6 +20,8 @@ const ProgressHeader = () => {

const { studioUrl, username } = useModel('progress', courseId);

useScrollToContent(MAIN_CONTENT_ID);

const viewingOtherStudentsProgressPage = (targetUserId && targetUserId !== userId);

const pageTitle = viewingOtherStudentsProgressPage
Expand All @@ -26,7 +30,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
6 changes: 6 additions & 0 deletions src/courseware/course/bookmark/BookmarkFilledIcon.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { Icon } from '@openedx/paragon';
import { Bookmark } from '@openedx/paragon/icons';

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need translation for "Bookmark" text?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed


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}
unitIdx={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
38 changes: 32 additions & 6 deletions src/courseware/course/sequence/sequence-navigation/UnitButton.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ 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 { BookmarkFilledIcon } from '@src/courseware/course/bookmark';
import { useScrollToContent } from '@src/generic/hooks';
import UnitIcon from './UnitIcon';
import CompleteIcon from './CompleteIcon';

Expand All @@ -20,16 +21,36 @@ const UnitButton = ({
unitId,
className,
showTitle,
unitIdx,
}) => {
const { courseId, sequenceId } = useSelector(state => state.courseware);
const { pathname } = useLocation();
const basePath = `/course/${courseId}/${sequenceId}/${unitId}`;
const unitPath = pathname.startsWith('/preview') ? `/preview${basePath}` : basePath;

useScrollToContent(isActive ? `${title}-${unitIdx}` : null);

const handleClick = useCallback(() => {
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 @@ -41,16 +62,20 @@ const UnitButton = ({
title={title}
as={Link}
to={unitPath}
role="tabpanel"
tabIndex={isActive ? 0 : -1}
aria-controls={title}
id={`${title}-${unitIdx}`}
aria-labelledby={title}
onKeyDown={handleKeyDown}
>
<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' }}
/>
) : null}
</Button>
Expand All @@ -68,6 +93,7 @@ UnitButton.propTypes = {
showTitle: PropTypes.bool,
title: PropTypes.string.isRequired,
unitId: PropTypes.string.isRequired,
unitIdx: PropTypes.number.isRequired,
};

UnitButton.defaultProps = {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
.unit-filled-bookmark {
top: -3px;
right: 2px;
height: 20px;
width: 20px;
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
import React from 'react';
import { Factory } from 'rosie';
import {
fireEvent, initializeTestStore, render, screen,
act,
fireEvent,
initializeTestStore,
render,
screen,
waitFor,
} from '../../../../setupTest';
import UnitButton from './UnitButton';
import messages from './messages';

describe('Unit Button', () => {
let mockData;
Expand All @@ -28,17 +34,35 @@ describe('Unit Button', () => {
mockData = {
unitId: unit.id,
onClick: () => {},
unitIdx: 0,
};

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}-0`);
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 +103,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">{messages.bookmark.defaultMessage}</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">{messages.bookmark.defaultMessage}</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'));
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ const messages = defineMessages({
defaultMessage: 'Previous',
description: 'Button to return to the previous section',
},
bookmark: {
id: 'learning.generic.bookmark',
defaultMessage: 'Bookmark',
description: 'Button text for bookmarking',
},
});

export default messages;
Loading