From a718c67f3647f0585d50188577c003ba531d5759 Mon Sep 17 00:00:00 2001 From: David Joy Date: Tue, 5 May 2020 09:46:18 -0400 Subject: [PATCH] Show message when there are no units in a sequence. (#60) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TNL-7191 - We didn’t fully protect against sequences with no units. The next/previous buttons now check whether there is a unit ID and construct a URL without if one doesn’t exist. When we load a sequence without units, we now show a message to the user so the page doesn’t look broken. --- src/courseware/CoursewareContainer.jsx | 18 +++-- src/courseware/course/InstructorToolbar.jsx | 1 - src/courseware/course/sequence/Sequence.jsx | 42 ++++-------- .../course/sequence/SequenceContent.jsx | 67 +++++++++++++++++++ src/courseware/course/sequence/messages.js | 5 ++ 5 files changed, 98 insertions(+), 35 deletions(-) create mode 100644 src/courseware/course/sequence/SequenceContent.jsx diff --git a/src/courseware/CoursewareContainer.jsx b/src/courseware/CoursewareContainer.jsx index 09afb13f..86e560a3 100644 --- a/src/courseware/CoursewareContainer.jsx +++ b/src/courseware/CoursewareContainer.jsx @@ -55,8 +55,13 @@ function useNextSequenceHandler(courseId, sequenceId) { const sequenceStatus = useSelector(state => state.courseware.sequenceStatus); return useCallback(() => { if (nextSequence !== null) { - const nextUnitId = nextSequence.unitIds[0]; - history.push(`/course/${courseId}/${nextSequence.id}/${nextUnitId}`); + if (nextSequence.unitIds.length > 0) { + const nextUnitId = nextSequence.unitIds[0]; + history.push(`/course/${courseId}/${nextSequence.id}/${nextUnitId}`); + } else { + // Some sequences have no units. This will show a blank page with prev/next buttons. + history.push(`/course/${courseId}/${nextSequence.id}`); + } } }, [courseStatus, sequenceStatus, sequenceId]); } @@ -67,8 +72,13 @@ function usePreviousSequenceHandler(courseId, sequenceId) { const sequenceStatus = useSelector(state => state.courseware.sequenceStatus); return useCallback(() => { if (previousSequence !== null) { - const previousUnitId = previousSequence.unitIds[previousSequence.unitIds.length - 1]; - history.push(`/course/${courseId}/${previousSequence.id}/${previousUnitId}`); + if (previousSequence.unitIds.length > 0) { + const previousUnitId = previousSequence.unitIds[previousSequence.unitIds.length - 1]; + history.push(`/course/${courseId}/${previousSequence.id}/${previousUnitId}`); + } else { + // Some sequences have no units. This will show a blank page with prev/next buttons. + history.push(`/course/${courseId}/${previousSequence.id}`); + } } }, [courseStatus, sequenceStatus, sequenceId]); } diff --git a/src/courseware/course/InstructorToolbar.jsx b/src/courseware/course/InstructorToolbar.jsx index 7b3ddb84..ac10c698 100644 --- a/src/courseware/course/InstructorToolbar.jsx +++ b/src/courseware/course/InstructorToolbar.jsx @@ -4,7 +4,6 @@ import { connect } from 'react-redux'; import { Collapsible } from '@edx/paragon'; function InstructorToolbar(props) { - // TODO: Only render this toolbar if the user is course staff if (!props.activeUnitLmsWebUrl) { return null; } diff --git a/src/courseware/course/sequence/Sequence.jsx b/src/courseware/course/sequence/Sequence.jsx index 5809d3eb..8ee537db 100644 --- a/src/courseware/course/sequence/Sequence.jsx +++ b/src/courseware/course/sequence/Sequence.jsx @@ -1,20 +1,19 @@ /* eslint-disable no-use-before-define */ import React, { - useEffect, useContext, Suspense, useState, + useEffect, useContext, useState, } from 'react'; import PropTypes from 'prop-types'; import { sendTrackEvent } from '@edx/frontend-platform/analytics'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; - import { useSelector } from 'react-redux'; -import Unit from './Unit'; -import { SequenceNavigation, UnitNavigation } from './sequence-navigation'; + import PageLoading from '../../../PageLoading'; -import messages from './messages'; import { UserMessagesContext, ALERT_TYPES } from '../../../user-messages'; import { useModel } from '../../../model-store'; -const ContentLock = React.lazy(() => import('./content-lock')); +import messages from './messages'; +import { SequenceNavigation, UnitNavigation } from './sequence-navigation'; +import SequenceContent from './SequenceContent'; function Sequence({ unitId, @@ -132,30 +131,13 @@ function Sequence({ }} />
- {gated && ( - - )} - > - - - )} - {!gated && unitId !== null && ( - - )} + {unitHasLoaded && ( import('./content-lock')); + +function SequenceContent({ + gated, intl, courseId, sequenceId, unitId, unitLoadedHandler, +}) { + const sequence = useModel('sequences', sequenceId); + + if (gated) { + return ( + + )} + > + + + ); + } + + if (unitId === null) { + return ( +
+ {intl.formatMessage(messages['learn.sequence.no.content'])} +
+ ); + } + + return ( + + ); +} + +SequenceContent.propTypes = { + gated: PropTypes.bool.isRequired, + courseId: PropTypes.string.isRequired, + sequenceId: PropTypes.string.isRequired, + unitId: PropTypes.string, + unitLoadedHandler: PropTypes.func.isRequired, + intl: intlShape.isRequired, +}; + +SequenceContent.defaultProps = { + unitId: null, +}; + +export default injectIntl(SequenceContent); diff --git a/src/courseware/course/sequence/messages.js b/src/courseware/course/sequence/messages.js index deebaf15..0cd447f8 100644 --- a/src/courseware/course/sequence/messages.js +++ b/src/courseware/course/sequence/messages.js @@ -16,6 +16,11 @@ const messages = defineMessages({ defaultMessage: 'There was an error loading this course.', description: 'Message when a course fails to load', }, + 'learn.sequence.no.content': { + id: 'learn.sequence.no.content', + defaultMessage: 'There is no content here.', + description: 'Message shown when there is no content to show a user inside a learning sequence.', + }, }); export default messages;