Andal.LND/frontend-app-learning
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: make nav buttons use links for accessibility (#1137)

Browse Source
This commit is contained in:
Mohamed Akram
2023-08-14 21:18:55 +04:00
committed by GitHub
parent 6232b0cb98
commit 5ee61904d5
15 changed files with 172 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
src/courseware/CoursewareContainer.jsx
View File

@@ -243,7 +243,7 @@ class CoursewareContainer extends Component {
checkSequenceUnitMarkerToSequenceUnitRedirect(courseId, sequenceStatus, sequence, routeUnitId);
}
handleUnitNavigationClick = (nextUnitId) => {
handleUnitNavigationClick = () => {
const {
courseId, sequenceId,
match: {
@@ -254,21 +254,17 @@ class CoursewareContainer extends Component {
} = this.props;
this.props.checkBlockCompletion(courseId, sequenceId, routeUnitId);
history.push(`/course/${courseId}/${sequenceId}/${nextUnitId}`);
};
handleNextSequenceClick = () => {
const {
course,
courseId,
nextSequence,
sequence,
sequenceId,
} = this.props;
if (nextSequence !== null) {
history.push(`/course/${courseId}/${nextSequence.id}/first`);
const celebrateFirstSection = course && course.celebrations && course.celebrations.firstSection;
if (celebrateFirstSection && sequence.sectionId !== nextSequence.sectionId) {
handleNextSectionCelebration(sequenceId, nextSequence.id);
@@ -276,12 +272,7 @@ class CoursewareContainer extends Component {
}
};
handlePreviousSequenceClick = () => {
const { previousSequence, courseId } = this.props;
if (previousSequence !== null) {
history.push(`/course/${courseId}/${previousSequence.id}/last`);
}
};
handlePreviousSequenceClick = () => {};
render() {
const {

6
src/courseware/CoursewareContainer.test.jsx
View File

@@ -185,7 +185,7 @@ describe('CoursewareContainer', () => {
function assertSequenceNavigation(container, expectedUnitCount = 3) {
// Ensure we had appropriate sequence navigation buttons. We should only have one unit.
const sequenceNavButtons = container.querySelectorAll('nav.sequence-navigation button');
const sequenceNavButtons = container.querySelectorAll('nav.sequence-navigation a, nav.sequence-navigation button');
expect(sequenceNavButtons).toHaveLength(expectedUnitCount + 2);
expect(sequenceNavButtons[0]).toHaveTextContent('Previous');
@@ -413,10 +413,10 @@ describe('CoursewareContainer', () => {
history.push(`/course/${courseId}/${sequenceBlock.id}/${unitBlocks[0].id}`);
const container = await waitFor(() => loadContainer());
const sequenceNavButtons = container.querySelectorAll('nav.sequence-navigation button');
const sequenceNavButtons = container.querySelectorAll('nav.sequence-navigation a, nav.sequence-navigation button');
const sequenceNextButton = sequenceNavButtons[4];
expect(sequenceNextButton).toHaveTextContent('Next');
fireEvent.click(sequenceNavButtons[4]);
fireEvent.click(sequenceNextButton);
expect(global.location.href).toEqual(`http://localhost/course/${courseId}/${sequenceBlock.id}/${unitBlocks[1].id}`);
});

4
src/courseware/course/Course.test.jsx
View File

@@ -280,8 +280,8 @@ describe('Course', () => {
loadUnit();
await waitFor(() => expect(screen.queryByText('Loading learning sequence...')).not.toBeInTheDocument());
screen.getAllByRole('button', { name: /previous/i }).forEach(button => fireEvent.click(button));
screen.getAllByRole('button', { name: /next/i }).forEach(button => fireEvent.click(button));
screen.getAllByRole('link', { name: /previous/i }).forEach(link => fireEvent.click(link));
screen.getAllByRole('link', { name: /next/i }).forEach(link => fireEvent.click(link));
// We are in the middle of the sequence, so no
expect(previousSequenceHandler).not.toHaveBeenCalled();

10
src/courseware/course/sequence/Sequence.jsx
View File

@@ -10,7 +10,6 @@ import {
} from '@edx/frontend-platform/analytics';
import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
import { useSelector } from 'react-redux';
import { history } from '@edx/frontend-platform';
import SequenceExamWrapper from '@edx/frontend-lib-special-exams';
import { breakpoints, useWindowSize } from '@edx/paragon';
@@ -139,9 +138,6 @@ const Sequence = ({
}
const gated = sequence && sequence.gatedContent !== undefined && sequence.gatedContent.gated;
const goToCourseExitPage = () => {
history.push(`/course/${courseId}/course-end`);
};
const defaultContent = (
<div className="sequence-container d-inline-flex flex-row">
@@ -150,7 +146,7 @@ const Sequence = ({
sequenceId={sequenceId}
unitId={unitId}
className="mb-4"
nextSequenceHandler={() => {
nextHandler={() => {
logEvent('edx.ui.lms.sequence.next_selected', 'top');
handleNext();
}}
@@ -158,11 +154,10 @@ const Sequence = ({
logEvent('edx.ui.lms.sequence.tab_selected', 'top', destinationUnitId);
handleNavigate(destinationUnitId);
}}
previousSequenceHandler={() => {
previousHandler={() => {
logEvent('edx.ui.lms.sequence.previous_selected', 'top');
handlePrevious();
}}
goToCourseExitPage={() => goToCourseExitPage()}
/>
{shouldDisplayNotificationTriggerInSequence && <SidebarTriggers />}
@@ -186,7 +181,6 @@ const Sequence = ({
logEvent('edx.ui.lms.sequence.next_selected', 'bottom');
handleNext();
}}
goToCourseExitPage={() => goToCourseExitPage()}
/>
)}
</div>

35
src/courseware/course/sequence/Sequence.test.jsx
View File

@@ -46,6 +46,7 @@ describe('Sequence', () => {
expect(screen.getByText('There is no content here.')).toBeInTheDocument();
expect(screen.queryByRole('button')).not.toBeInTheDocument();
expect(screen.queryByRole('link')).not.toBeInTheDocument();
});
it('renders correctly for gated content', async () => {
@@ -74,8 +75,10 @@ describe('Sequence', () => {
);
await waitFor(() => expect(screen.queryByText('Loading locked content messaging...')).toBeInTheDocument());
// `Previous`, `Active`, `Next`, `Prerequisite` and `Close Tray` buttons.
expect(screen.getAllByRole('button').length).toEqual(5);
// `Previous`, `Prerequisite` and `Close Tray` buttons.
expect(screen.getAllByRole('button').length).toEqual(3);
// `Active` and `Next` buttons.
expect(screen.getAllByRole('link').length).toEqual(2);
expect(screen.getByText('Content Locked')).toBeInTheDocument();
const unitContainer = container.querySelector('.unit-container');
@@ -112,6 +115,7 @@ describe('Sequence', () => {
// No normal content or navigation should be rendered. Just the above alert.
expect(screen.queryAllByRole('button').length).toEqual(0);
expect(screen.queryAllByRole('link').length).toEqual(1);
});
it('displays error message on sequence load failure', async () => {
@@ -125,13 +129,16 @@ describe('Sequence', () => {
it('handles loading unit', async () => {
render(<Sequence {...mockData} />);
expect(await screen.findByText('Loading learning sequence...')).toBeInTheDocument();
// Renders navigation buttons plus one button for each unit.
expect(screen.getAllByRole('button')).toHaveLength(4 + unitBlocks.length);
// `Previous`, `Bookmark` and `Close Tray` buttons
expect(screen.getAllByRole('button')).toHaveLength(3);
// Renders `Next` button plus one button for each unit.
expect(screen.getAllByRole('link')).toHaveLength(1 + unitBlocks.length);
loadUnit();
await waitFor(() => expect(screen.queryByText('Loading learning sequence...')).not.toBeInTheDocument());
// At this point there will be 2 `Previous` and 2 `Next` buttons.
expect(screen.getAllByRole('button', { name: /previous|next/i }).length).toEqual(4);
expect(screen.getAllByRole('button', { name: /previous/i }).length).toEqual(2);
expect(screen.getAllByRole('link', { name: /next/i }).length).toEqual(2);
});
describe('sequence and unit navigation buttons', () => {
@@ -163,7 +170,7 @@ describe('Sequence', () => {
render(<Sequence {...testData} />, { store: testStore });
expect(await screen.findByText('Loading learning sequence...')).toBeInTheDocument();
const sequencePreviousButton = screen.getByRole('button', { name: /previous/i });
const sequencePreviousButton = screen.getByRole('link', { name: /previous/i });
fireEvent.click(sequencePreviousButton);
expect(testData.previousSequenceHandler).toHaveBeenCalledTimes(1);
expect(sendTrackEvent).toHaveBeenCalledTimes(1);
@@ -176,7 +183,7 @@ describe('Sequence', () => {
loadUnit();
await waitFor(() => expect(screen.queryByText('Loading learning sequence...')).not.toBeInTheDocument());
const unitPreviousButton = screen.getAllByRole('button', { name: /previous/i })
const unitPreviousButton = screen.getAllByRole('link', { name: /previous/i })
.filter(button => button !== sequencePreviousButton)[0];
fireEvent.click(unitPreviousButton);
expect(testData.previousSequenceHandler).toHaveBeenCalledTimes(2);
@@ -199,7 +206,7 @@ describe('Sequence', () => {
render(<Sequence {...testData} />, { store: testStore });
expect(await screen.findByText('Loading learning sequence...')).toBeInTheDocument();
const sequenceNextButton = screen.getByRole('button', { name: /next/i });
const sequenceNextButton = screen.getByRole('link', { name: /next/i });
fireEvent.click(sequenceNextButton);
expect(testData.nextSequenceHandler).toHaveBeenCalledTimes(1);
expect(sendTrackEvent).toHaveBeenCalledWith('edx.ui.lms.sequence.next_selected', {
@@ -211,7 +218,7 @@ describe('Sequence', () => {
loadUnit();
await waitFor(() => expect(screen.queryByText('Loading learning sequence...')).not.toBeInTheDocument());
const unitNextButton = screen.getAllByRole('button', { name: /next/i })
const unitNextButton = screen.getAllByRole('link', { name: /next/i })
.filter(button => button !== sequenceNextButton)[0];
fireEvent.click(unitNextButton);
expect(testData.nextSequenceHandler).toHaveBeenCalledTimes(2);
@@ -237,11 +244,11 @@ describe('Sequence', () => {
render(<Sequence {...testData} />, { store: testStore });
await waitFor(() => expect(screen.queryByText('Loading learning sequence...')).toBeInTheDocument());
fireEvent.click(screen.getByRole('button', { name: /previous/i }));
fireEvent.click(screen.getByRole('link', { name: /previous/i }));
expect(testData.previousSequenceHandler).not.toHaveBeenCalled();
expect(testData.unitNavigationHandler).toHaveBeenCalledWith(unitBlocks[unitNumber - 1].id);
fireEvent.click(screen.getByRole('button', { name: /next/i }));
fireEvent.click(screen.getByRole('link', { name: /next/i }));
expect(testData.nextSequenceHandler).not.toHaveBeenCalled();
// As `previousSequenceHandler` and `nextSequenceHandler` are mocked, we aren't really changing the position here.
// Therefore the next unit will still be `the initial one + 1`.
@@ -323,11 +330,11 @@ describe('Sequence', () => {
loadUnit();
await waitFor(() => expect(screen.queryByText('Loading learning sequence...')).not.toBeInTheDocument());
screen.getAllByRole('button', { name: /previous/i }).forEach(button => fireEvent.click(button));
screen.getAllByRole('link', { name: /previous/i }).forEach(button => fireEvent.click(button));
expect(testData.previousSequenceHandler).toHaveBeenCalledTimes(2);
expect(testData.unitNavigationHandler).not.toHaveBeenCalled();
screen.getAllByRole('button', { name: /next/i }).forEach(button => fireEvent.click(button));
screen.getAllByRole('link', { name: /next/i }).forEach(button => fireEvent.click(button));
expect(testData.nextSequenceHandler).toHaveBeenCalledTimes(2);
expect(testData.unitNavigationHandler).not.toHaveBeenCalled();
@@ -370,7 +377,7 @@ describe('Sequence', () => {
render(<Sequence {...testData} />, { store: testStore });
await waitFor(() => expect(screen.queryByText('Loading learning sequence...')).toBeInTheDocument());
fireEvent.click(screen.getByRole('button', { name: targetUnit.display_name }));
fireEvent.click(screen.getByRole('link', { name: targetUnit.display_name }));
expect(testData.unitNavigationHandler).toHaveBeenCalledWith(targetUnit.id);
expect(sendTrackEvent).toHaveBeenCalledWith('edx.ui.lms.sequence.tab_selected', {
current_tab: currentTabNumber,

51
src/courseware/course/sequence/sequence-navigation/SequenceNavigation.jsx
View File

@@ -1,4 +1,5 @@
import React from 'react';
import { Link } from 'react-router-dom';
import PropTypes from 'prop-types';
import { breakpoints, Button, useWindowSize } from '@edx/paragon';
import { ChevronLeft, ChevronRight } from '@edx/paragon/icons';
@@ -26,12 +27,13 @@ const SequenceNavigation = ({
sequenceId,
className,
onNavigate,
nextSequenceHandler,
previousSequenceHandler,
goToCourseExitPage,
nextHandler,
previousHandler,
}) => {
const sequence = useModel('sequences', sequenceId);
const { isFirstUnit, isLastUnit } = useSequenceNavigationMetadata(sequenceId, unitId);
const {
isFirstUnit, isLastUnit, nextLink, previousLink,
} = useSequenceNavigationMetadata(sequenceId, unitId);
const {
courseId,
sequenceStatus,
@@ -63,27 +65,49 @@ const SequenceNavigation = ({
);
};
const renderPreviousButton = () => {
const disabled = isFirstUnit;
const prevArrow = isRtl(getLocale()) ? ChevronRight : ChevronLeft;
return (
<Button
variant="link"
className="previous-btn"
onClick={previousHandler}
disabled={disabled}
iconBefore={prevArrow}
as={disabled ? undefined : Link}
to={disabled ? undefined : previousLink}
>
{shouldDisplayNotificationTriggerInSequence ? null : intl.formatMessage(messages.previousButton)}
</Button>
);
};
const renderNextButton = () => {
const { exitActive, exitText } = GetCourseExitNavigation(courseId, intl);
const buttonOnClick = isLastUnit ? goToCourseExitPage : nextSequenceHandler;
const buttonText = (isLastUnit && exitText) ? exitText : intl.formatMessage(messages.nextButton);
const disabled = isLastUnit && !exitActive;
const nextArrow = isRtl(getLocale()) ? ChevronLeft : ChevronRight;
return (
<Button variant="link" className="next-btn" onClick={buttonOnClick} disabled={disabled} iconAfter={nextArrow}>
<Button
variant="link"
className="next-btn"
onClick={nextHandler}
disabled={disabled}
iconAfter={nextArrow}
as={disabled ? undefined : Link}
to={disabled ? undefined : nextLink}
>
{shouldDisplayNotificationTriggerInSequence ? null : buttonText}
</Button>
);
};
const prevArrow = isRtl(getLocale()) ? ChevronRight : ChevronLeft;
return sequenceStatus === LOADED && (
<nav id="courseware-sequenceNavigation" className={classNames('sequence-navigation', className)} style={{ width: shouldDisplayNotificationTriggerInSequence ? '90%' : null }}>
<Button variant="link" className="previous-btn" onClick={previousSequenceHandler} disabled={isFirstUnit} iconBefore={prevArrow}>
{shouldDisplayNotificationTriggerInSequence ? null : intl.formatMessage(messages.previousButton)}
</Button>
{renderPreviousButton()}
{renderUnitButtons()}
{renderNextButton()}
@@ -97,9 +121,8 @@ SequenceNavigation.propTypes = {
unitId: PropTypes.string,
className: PropTypes.string,
onNavigate: PropTypes.func.isRequired,
nextSequenceHandler: PropTypes.func.isRequired,
previousSequenceHandler: PropTypes.func.isRequired,
goToCourseExitPage: PropTypes.func.isRequired,
nextHandler: PropTypes.func.isRequired,
previousHandler: PropTypes.func.isRequired,
};
SequenceNavigation.defaultProps = {

35
src/courseware/course/sequence/sequence-navigation/SequenceNavigation.test.jsx
View File

@@ -25,10 +25,9 @@ describe('Sequence Navigation', () => {
mockData = {
unitId: unitBlocks[1].id,
sequenceId: courseware.sequenceId,
previousSequenceHandler: () => {},
previousHandler: () => {},
onNavigate: () => {},
nextSequenceHandler: () => {},
goToCourseExitPage: () => {},
nextHandler: () => {},
};
});
@@ -77,7 +76,7 @@ describe('Sequence Navigation', () => {
const onNavigate = jest.fn();
render(<SequenceNavigation {...mockData} {...{ onNavigate }} />);
const unitButtons = screen.getAllByRole('button', { name: /\d+/ });
const unitButtons = screen.getAllByRole('link', { name: /\d+/ });
expect(unitButtons).toHaveLength(unitButtons.length);
unitButtons.forEach(button => fireEvent.click(button));
expect(onNavigate).toHaveBeenCalledTimes(unitButtons.length);
@@ -86,7 +85,7 @@ describe('Sequence Navigation', () => {
it('has both navigation buttons enabled for a non-corner unit of the sequence', () => {
render(<SequenceNavigation {...mockData} />);
screen.getAllByRole('button', { name: /previous|next/i }).forEach(button => {
screen.getAllByRole('link', { name: /previous|next/i }).forEach(button => {
expect(button).toBeEnabled();
});
});
@@ -95,7 +94,7 @@ describe('Sequence Navigation', () => {
render(<SequenceNavigation {...mockData} unitId={unitBlocks[0].id} />);
expect(screen.getByRole('button', { name: /previous/i })).toBeDisabled();
expect(screen.getByRole('button', { name: /next/i })).toBeEnabled();
expect(screen.getByRole('link', { name: /next/i })).toBeEnabled();
});
it('has the "Next" button disabled for the last unit of the sequence if there is no Exit page', async () => {
@@ -110,7 +109,7 @@ describe('Sequence Navigation', () => {
{ store: testStore },
);
expect(screen.getByRole('button', { name: /previous/i })).toBeEnabled();
expect(screen.getByRole('link', { name: /previous/i })).toBeEnabled();
expect(screen.getByRole('button', { name: /next/i })).toBeDisabled();
});
@@ -126,8 +125,8 @@ describe('Sequence Navigation', () => {
{ store: testStore },
);
expect(screen.getByRole('button', { name: /previous/i })).toBeEnabled();
expect(screen.getByRole('button', { name: /next \(end of course\)/i })).toBeEnabled();
expect(screen.getByRole('link', { name: /previous/i })).toBeEnabled();
expect(screen.getByRole('link', { name: /next \(end of course\)/i })).toBeEnabled();
});
it('displays complete course message instead of the "Next" button as needed', async () => {
@@ -147,19 +146,19 @@ describe('Sequence Navigation', () => {
{ store: testStore },
);
expect(screen.getByRole('button', { name: /previous/i })).toBeEnabled();
expect(screen.getByRole('button', { name: /Complete the course/i })).toBeEnabled();
expect(screen.getByRole('link', { name: /previous/i })).toBeEnabled();
expect(screen.getByRole('link', { name: /Complete the course/i })).toBeEnabled();
});
it('handles "Previous" and "Next" click', () => {
const previousSequenceHandler = jest.fn();
const nextSequenceHandler = jest.fn();
render(<SequenceNavigation {...mockData} {...{ previousSequenceHandler, nextSequenceHandler }} />);
const previousHandler = jest.fn();
const nextHandler = jest.fn();
render(<SequenceNavigation {...mockData} {...{ previousHandler, nextHandler }} />);
fireEvent.click(screen.getByRole('button', { name: /previous/i }));
expect(previousSequenceHandler).toHaveBeenCalledTimes(1);
fireEvent.click(screen.getByRole('link', { name: /previous/i }));
expect(previousHandler).toHaveBeenCalledTimes(1);
fireEvent.click(screen.getByRole('button', { name: /next/i }));
expect(nextSequenceHandler).toHaveBeenCalledTimes(1);
fireEvent.click(screen.getByRole('link', { name: /next/i }));
expect(nextHandler).toHaveBeenCalledTimes(1);
});
});

4
src/courseware/course/sequence/sequence-navigation/SequenceNavigationDropdown.test.jsx
View File

@@ -47,7 +47,7 @@ describe('Sequence Navigation Dropdown', () => {
});
const dropdownMenu = container.querySelector('.dropdown-menu');
// Only the current unit should be marked as active.
getAllByRole(dropdownMenu, 'button', { hidden: true }).forEach(button => {
getAllByRole(dropdownMenu, 'link', { hidden: true }).forEach(button => {
if (button.textContent === unit.display_name) {
expect(button).toHaveClass('active');
} else {
@@ -66,7 +66,7 @@ describe('Sequence Navigation Dropdown', () => {
fireEvent.click(dropdownToggle);
});
const dropdownMenu = container.querySelector('.dropdown-menu');
getAllByRole(dropdownMenu, 'button', { hidden: true }).forEach(button => fireEvent.click(button));
getAllByRole(dropdownMenu, 'link', { hidden: true }).forEach(button => fireEvent.click(button));
expect(onNavigate).toHaveBeenCalledTimes(unitBlocks.length);
unitBlocks.forEach((unit, index) => {
expect(onNavigate).toHaveBeenNthCalledWith(index + 1, unit.id);

6
src/courseware/course/sequence/sequence-navigation/SequenceNavigationTabs.test.jsx
View File

@@ -43,7 +43,7 @@ describe('Sequence Navigation Tabs', () => {
useIndexOfLastVisibleChild.mockReturnValue([0, null, null]);
render(<SequenceNavigationTabs {...mockData} />);
expect(screen.getAllByRole('button')).toHaveLength(unitBlocks.length);
expect(screen.getAllByRole('link')).toHaveLength(unitBlocks.length);
});
it('renders unit buttons and dropdown button', async () => {
@@ -60,8 +60,8 @@ describe('Sequence Navigation Tabs', () => {
await fireEvent.click(dropdownToggle);
});
const dropdownMenu = container.querySelector('.dropdown');
const dropdownButtons = getAllByRole(dropdownMenu, 'button');
expect(dropdownButtons).toHaveLength(unitBlocks.length + 1);
const dropdownButtons = getAllByRole(dropdownMenu, 'link');
expect(dropdownButtons).toHaveLength(unitBlocks.length);
expect(screen.getByRole('button', { name: `${activeBlockNumber} of ${unitBlocks.length}` }))
.toHaveClass('dropdown-toggle');
});

7
src/courseware/course/sequence/sequence-navigation/UnitButton.jsx
View File

@@ -1,6 +1,7 @@
import React, { useCallback } from 'react';
import { Link } from 'react-router-dom';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import { connect, useSelector } from 'react-redux';
import classNames from 'classnames';
import { Button } from '@edx/paragon';
@@ -20,6 +21,8 @@ const UnitButton = ({
className,
showTitle,
}) => {
const { courseId, sequenceId } = useSelector(state => state.courseware);
const handleClick = useCallback(() => {
onClick(unitId);
}, [onClick, unitId]);
@@ -33,6 +36,8 @@ const UnitButton = ({
variant="link"
onClick={handleClick}
title={title}
as={Link}
to={`/course/${courseId}/${sequenceId}/${unitId}`}
>
<UnitIcon type={contentType} />
{showTitle && <span className="unit-title">{title}</span>}

6
src/courseware/course/sequence/sequence-navigation/UnitButton.test.jsx
View File

@@ -33,12 +33,12 @@ describe('Unit Button', () => {
it('hides title by default', () => {
render(<UnitButton {...mockData} />);
expect(screen.getByRole('button')).not.toHaveTextContent(unit.display_name);
expect(screen.getByRole('link')).not.toHaveTextContent(unit.display_name);
});
it('shows title', () => {
render(<UnitButton {...mockData} showTitle />);
expect(screen.getByRole('button')).toHaveTextContent(unit.display_name);
expect(screen.getByRole('link')).toHaveTextContent(unit.display_name);
});
it('does not show completion for non-completed unit', () => {
@@ -79,7 +79,7 @@ describe('Unit Button', () => {
it('handles the click', () => {
const onClick = jest.fn();
render(<UnitButton {...mockData} onClick={onClick} />);
fireEvent.click(screen.getByRole('button'));
fireEvent.click(screen.getByRole('link'));
expect(onClick).toHaveBeenCalledTimes(1);
});
});

41
src/courseware/course/sequence/sequence-navigation/UnitNavigation.jsx
View File

@@ -1,4 +1,5 @@
import React from 'react';
import { Link } from 'react-router-dom';
import PropTypes from 'prop-types';
import { Button } from '@edx/paragon';
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
@@ -20,14 +21,32 @@ const UnitNavigation = ({
unitId,
onClickPrevious,
onClickNext,
goToCourseExitPage,
}) => {
const { isFirstUnit, isLastUnit } = useSequenceNavigationMetadata(sequenceId, unitId);
const {
isFirstUnit, isLastUnit, nextLink, previousLink,
} = useSequenceNavigationMetadata(sequenceId, unitId);
const { courseId } = useSelector(state => state.courseware);
const renderPreviousButton = () => {
const disabled = isFirstUnit;
const prevArrow = isRtl(getLocale()) ? faChevronRight : faChevronLeft;
return (
<Button
variant="outline-secondary"
className="previous-button mr-2 d-flex align-items-center justify-content-center"
disabled={disabled}
onClick={onClickPrevious}
as={disabled ? undefined : Link}
to={disabled ? undefined : previousLink}
>
<FontAwesomeIcon icon={prevArrow} className="mr-2" size="sm" />
{intl.formatMessage(messages.previousButton)}
</Button>
);
};
const renderNextButton = () => {
const { exitActive, exitText } = GetCourseExitNavigation(courseId, intl);
const buttonOnClick = isLastUnit ? goToCourseExitPage : onClickNext;
const buttonText = (isLastUnit && exitText) ? exitText : intl.formatMessage(messages.nextButton);
const disabled = isLastUnit && !exitActive;
const nextArrow = isRtl(getLocale()) ? faChevronLeft : faChevronRight;
@@ -35,8 +54,10 @@ const UnitNavigation = ({
<Button
variant="outline-primary"
className="next-button d-flex align-items-center justify-content-center"
onClick={buttonOnClick}
onClick={onClickNext}
disabled={disabled}
as={disabled ? undefined : Link}
to={disabled ? undefined : nextLink}
>
<UnitNavigationEffortEstimate sequenceId={sequenceId} unitId={unitId}>
{buttonText}
@@ -46,18 +67,9 @@ const UnitNavigation = ({
);
};
const prevArrow = isRtl(getLocale()) ? faChevronRight : faChevronLeft;
return (
<div className="unit-navigation d-flex">
<Button
variant="outline-secondary"
className="previous-button mr-2 d-flex align-items-center justify-content-center"
disabled={isFirstUnit}
onClick={onClickPrevious}
>
<FontAwesomeIcon icon={prevArrow} className="mr-2" size="sm" />
{intl.formatMessage(messages.previousButton)}
</Button>
{renderPreviousButton()}
{renderNextButton()}
</div>
);
@@ -69,7 +81,6 @@ UnitNavigation.propTypes = {
unitId: PropTypes.string,
onClickPrevious: PropTypes.func.isRequired,
onClickNext: PropTypes.func.isRequired,
goToCourseExitPage: PropTypes.func.isRequired,
};
UnitNavigation.defaultProps = {

23
src/courseware/course/sequence/sequence-navigation/UnitNavigation.test.jsx
View File

@@ -22,7 +22,6 @@ describe('Unit Navigation', () => {
sequenceId: courseware.sequenceId,
onClickPrevious: () => {},
onClickNext: () => {},
goToCourseExitPage: () => {},
};
});
@@ -36,7 +35,7 @@ describe('Unit Navigation', () => {
/>);
// Only "Previous" and "Next" buttons should be rendered.
expect(screen.getAllByRole('button')).toHaveLength(2);
expect(screen.getAllByRole('link')).toHaveLength(2);
});
it('handles the clicks', () => {
@@ -45,23 +44,21 @@ describe('Unit Navigation', () => {
render(<UnitNavigation
{...mockData}
sequenceId=""
unitId=""
onClickPrevious={onClickPrevious}
onClickNext={onClickNext}
/>);
fireEvent.click(screen.getByRole('button', { name: /previous/i }));
fireEvent.click(screen.getByRole('link', { name: /previous/i }));
expect(onClickPrevious).toHaveBeenCalledTimes(1);
fireEvent.click(screen.getByRole('button', { name: /next/i }));
fireEvent.click(screen.getByRole('link', { name: /next/i }));
expect(onClickNext).toHaveBeenCalledTimes(1);
});
it('has the navigation buttons enabled for the non-corner unit in the sequence', () => {
render(<UnitNavigation {...mockData} />);
screen.getAllByRole('button').forEach(button => {
screen.getAllByRole('link').forEach(button => {
expect(button).toBeEnabled();
});
});
@@ -70,7 +67,7 @@ describe('Unit Navigation', () => {
render(<UnitNavigation {...mockData} unitId={unitBlocks[0].id} />);
expect(screen.getByRole('button', { name: /previous/i })).toBeDisabled();
expect(screen.getByRole('button', { name: /next/i })).toBeEnabled();
expect(screen.getByRole('link', { name: /next/i })).toBeEnabled();
});
it('has the "Next" button disabled for the last unit in the sequence if there is no Exit Page', async () => {
@@ -85,7 +82,7 @@ describe('Unit Navigation', () => {
{ store: testStore },
);
expect(screen.getByRole('button', { name: /previous/i })).toBeEnabled();
expect(screen.getByRole('link', { name: /previous/i })).toBeEnabled();
expect(screen.getByRole('button', { name: /next/i })).toBeDisabled();
});
@@ -101,8 +98,8 @@ describe('Unit Navigation', () => {
{ store: testStore },
);
expect(screen.getByRole('button', { name: /previous/i })).toBeEnabled();
expect(screen.getByRole('button', { name: /next \(end of course\)/i })).toBeEnabled();
expect(screen.getByRole('link', { name: /previous/i })).toBeEnabled();
expect(screen.getByRole('link', { name: /next \(end of course\)/i })).toBeEnabled();
});
it('displays complete course message instead of the "Next" button as needed', async () => {
@@ -122,7 +119,7 @@ describe('Unit Navigation', () => {
{ store: testStore },
);
expect(screen.getByRole('button', { name: /previous/i })).toBeEnabled();
expect(screen.getByRole('button', { name: /Complete the course/i })).toBeEnabled();
expect(screen.getByRole('link', { name: /previous/i })).toBeEnabled();
expect(screen.getByRole('link', { name: /Complete the course/i })).toBeEnabled();
});
});

42
src/courseware/course/sequence/sequence-navigation/hooks.js
View File

@@ -7,6 +7,7 @@ import { sequenceIdsSelector } from '../../../data';
export function useSequenceNavigationMetadata(currentSequenceId, currentUnitId) {
const sequenceIds = useSelector(sequenceIdsSelector);
const sequence = useModel('sequences', currentSequenceId);
const courseId = useSelector(state => state.courseware.courseId);
const courseStatus = useSelector(state => state.courseware.courseStatus);
const sequenceStatus = useSelector(state => state.courseware.sequenceStatus);
@@ -14,12 +15,43 @@ export function useSequenceNavigationMetadata(currentSequenceId, currentUnitId)
if (courseStatus !== 'loaded' || sequenceStatus !== 'loaded' || !currentSequenceId || !currentUnitId) {
return { isFirstUnit: false, isLastUnit: false };
}
const isFirstSequence = sequenceIds.indexOf(currentSequenceId) === 0;
const isFirstUnitInSequence = sequence.unitIds.indexOf(currentUnitId) === 0;
const sequenceIndex = sequenceIds.indexOf(currentSequenceId);
const unitIndex = sequence.unitIds.indexOf(currentUnitId);
const isFirstSequence = sequenceIndex === 0;
const isFirstUnitInSequence = unitIndex === 0;
const isFirstUnit = isFirstSequence && isFirstUnitInSequence;
const isLastSequence = sequenceIds.indexOf(currentSequenceId) === sequenceIds.length - 1;
const isLastUnitInSequence = sequence.unitIds.indexOf(currentUnitId) === sequence.unitIds.length - 1;
const isLastSequence = sequenceIndex === sequenceIds.length - 1;
const isLastUnitInSequence = unitIndex === sequence.unitIds.length - 1;
const isLastUnit = isLastSequence && isLastUnitInSequence;
return { isFirstUnit, isLastUnit };
const nextSequenceId = sequenceIndex < sequenceIds.length - 1 ? sequenceIds[sequenceIndex + 1] : null;
const previousSequenceId = sequenceIndex > 0 ? sequenceIds[sequenceIndex - 1] : null;
let nextLink;
if (isLastUnit) {
nextLink = `/course/${courseId}/course-end`;
} else {
const nextIndex = unitIndex + 1;
if (nextIndex < sequence.unitIds.length) {
const nextUnitId = sequence.unitIds[nextIndex];
nextLink = `/course/${courseId}/${currentSequenceId}/${nextUnitId}`;
} else if (nextSequenceId) {
nextLink = `/course/${courseId}/${nextSequenceId}/first`;
}
}
let previousLink;
const previousIndex = unitIndex - 1;
if (previousIndex >= 0) {
const previousUnitId = sequence.unitIds[previousIndex];
previousLink = `/course/${courseId}/${currentSequenceId}/${previousUnitId}`;
} else if (previousSequenceId) {
previousLink = `/course/${courseId}/${previousSequenceId}/last`;
}
return {
isFirstUnit, isLastUnit, nextLink, previousLink,
};
}

2
src/product-tours/ProductTours.test.jsx
View File

@@ -293,7 +293,7 @@ describe('Courseware Tour', () => {
});
const container = await loadContainer();
const sequenceNavButtons = container.querySelectorAll('nav.sequence-navigation button');
const sequenceNavButtons = container.querySelectorAll('nav.sequence-navigation a, nav.sequence-navigation button');
const sequenceNextButton = sequenceNavButtons[4];
expect(sequenceNextButton).toHaveTextContent('Next');
fireEvent.click(sequenceNextButton);