diff --git a/src/courseware/CoursewareContainer.jsx b/src/courseware/CoursewareContainer.jsx
index 34fcc092..0bc792b5 100644
--- a/src/courseware/CoursewareContainer.jsx
+++ b/src/courseware/CoursewareContainer.jsx
@@ -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 {
diff --git a/src/courseware/CoursewareContainer.test.jsx b/src/courseware/CoursewareContainer.test.jsx
index 72f2fe80..5df0deeb 100644
--- a/src/courseware/CoursewareContainer.test.jsx
+++ b/src/courseware/CoursewareContainer.test.jsx
@@ -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}`);
});
diff --git a/src/courseware/course/Course.test.jsx b/src/courseware/course/Course.test.jsx
index 312a7862..6362c0dc 100644
--- a/src/courseware/course/Course.test.jsx
+++ b/src/courseware/course/Course.test.jsx
@@ -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();
diff --git a/src/courseware/course/sequence/Sequence.jsx b/src/courseware/course/sequence/Sequence.jsx
index faa58c90..529c09b8 100644
--- a/src/courseware/course/sequence/Sequence.jsx
+++ b/src/courseware/course/sequence/Sequence.jsx
@@ -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 = (
@@ -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 && }
@@ -186,7 +181,6 @@ const Sequence = ({
logEvent('edx.ui.lms.sequence.next_selected', 'bottom');
handleNext();
}}
- goToCourseExitPage={() => goToCourseExitPage()}
/>
)}
diff --git a/src/courseware/course/sequence/Sequence.test.jsx b/src/courseware/course/sequence/Sequence.test.jsx
index 8c07f989..716de869 100644
--- a/src/courseware/course/sequence/Sequence.test.jsx
+++ b/src/courseware/course/sequence/Sequence.test.jsx
@@ -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();
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(, { 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(, { 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(, { 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(, { 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,
diff --git a/src/courseware/course/sequence/sequence-navigation/SequenceNavigation.jsx b/src/courseware/course/sequence/sequence-navigation/SequenceNavigation.jsx
index 76bbaf3f..df45753b 100644
--- a/src/courseware/course/sequence/sequence-navigation/SequenceNavigation.jsx
+++ b/src/courseware/course/sequence/sequence-navigation/SequenceNavigation.jsx
@@ -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 (
+
+ );
+ };
+
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 (
-