Skip to content
Draft
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
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?


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