Skip to content

Commit

Permalink
feat: legacy course navigation
Browse files Browse the repository at this point in the history
Add an option to enable the legacy course navigation where clicking a
breadcrumb leads to the course index page highlighting the selected section.
  • Loading branch information
ArturGaspar committed Nov 27, 2023
1 parent 0137868 commit 79d9cbc
Show file tree
Hide file tree
Showing 13 changed files with 175 additions and 23 deletions.
1 change: 1 addition & 0 deletions .env
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ DISCOVERY_API_BASE_URL=''
DISCUSSIONS_MFE_BASE_URL=''
ECOMMERCE_BASE_URL=''
ENABLE_JUMPNAV='true'
ENABLE_LEGACY_NAV=''
ENABLE_NOTICES=''
ENTERPRISE_LEARNER_PORTAL_HOSTNAME=''
EXAMS_BASE_URL=''
Expand Down
1 change: 1 addition & 0 deletions .env.development
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ DISCOVERY_API_BASE_URL='http://localhost:18381'
DISCUSSIONS_MFE_BASE_URL='http://localhost:2002'
ECOMMERCE_BASE_URL='http://localhost:18130'
ENABLE_JUMPNAV='true'
ENABLE_LEGACY_NAV=''
ENABLE_NOTICES=''
ENTERPRISE_LEARNER_PORTAL_HOSTNAME='localhost:8734'
EXAMS_BASE_URL='http://localhost:18740'
Expand Down
4 changes: 4 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ ENABLE_JUMPNAV
This feature flag is slated to be removed as jumpnav becomes default. Follow the progress of this ticket here:
https://openedx.atlassian.net/browse/TNL-8678

ENABLE_LEGACY_NAV
Enables the legacy behaviour in the course breadcrumbs, where links lead to
the course index highlighting the selected course section or subsection.

SOCIAL_UTM_MILESTONE_CAMPAIGN
This value is passed as the ``utm_campaign`` parameter for social-share
links when celebrating learning milestones in the course. Optional.
Expand Down
15 changes: 14 additions & 1 deletion src/course-home/outline-tab/OutlineTab.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,15 @@ const OutlineTab = ({ intl }) => {
}
}, [location.search]);

// A section or subsection is selected by its id being the location hash part.
// location.hash will contain an initial # sign, so remove it here.
const hashValue = location.hash.substring(1);
// Represents whether section is either selected or contains selected
// subsection and thus should be expanded by default.
const selectedSectionId = rootCourseId && courses[rootCourseId].sectionIds.find((sectionId) => (
(hashValue === sectionId) || sections[sectionId].sequenceIds.includes(hashValue)
));

return (
<>
<div data-learner-type={learnerType} className="row w-100 mx-0 my-3 justify-content-between">
Expand Down Expand Up @@ -171,7 +180,11 @@ const OutlineTab = ({ intl }) => {
<Section
key={sectionId}
courseId={courseId}
defaultOpen={sections[sectionId].resumeBlock}
defaultOpen={
(selectedSectionId)
? sectionId === selectedSectionId
: sections[sectionId].resumeBlock
}
expand={expandAll}
section={sections[sectionId]}
/>
Expand Down
32 changes: 32 additions & 0 deletions src/course-home/outline-tab/OutlineTab.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,16 @@ describe('Outline Tab', () => {
});

describe('Course Outline', () => {
const { scrollIntoView } = window.HTMLElement.prototype;

beforeEach(() => {
window.HTMLElement.prototype.scrollIntoView = jest.fn();
});

afterEach(() => {
window.HTMLElement.prototype.scrollIntoView = scrollIntoView;
});

it('displays link to start course', async () => {
await fetchAndRender();
expect(screen.getByRole('link', { name: messages.start.defaultMessage })).toBeInTheDocument();
Expand All @@ -107,6 +117,28 @@ describe('Outline Tab', () => {
expect(expandedSectionNode).toHaveAttribute('aria-expanded', 'true');
});

it('expands and scrolls to selected section', async () => {
const { courseBlocks, sectionBlocks } = await buildMinimalCourseBlocks(courseId, 'Title');
setTabData({
course_blocks: { blocks: courseBlocks.blocks },
});
await fetchAndRender(`http://localhost/#${sectionBlocks[0].id}`);
const expandedSectionNode = screen.getByRole('button', { name: /Title of Section/ });
expect(expandedSectionNode).toHaveAttribute('aria-expanded', 'true');
expect(window.HTMLElement.prototype.scrollIntoView).toHaveBeenCalled();
});

it('expands and scrolls to section that contains selected subsection', async () => {
const { courseBlocks, sequenceBlocks } = await buildMinimalCourseBlocks(courseId, 'Title');
setTabData({
course_blocks: { blocks: courseBlocks.blocks },
});
await fetchAndRender(`http://localhost/#${sequenceBlocks[0].id}`);
const expandedSectionNode = screen.getByRole('button', { name: /Title of Section/ });
expect(expandedSectionNode).toHaveAttribute('aria-expanded', 'true');
expect(window.HTMLElement.prototype.scrollIntoView).toHaveBeenCalled();
});

it('handles expand/collapse all button click', async () => {
await fetchAndRender();
// Button renders as "Expand All"
Expand Down
16 changes: 14 additions & 2 deletions src/course-home/outline-tab/Section.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import React, { useEffect, useState } from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';
import { useLocation } from 'react-router-dom';
import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
import { Collapsible, IconButton } from '@edx/paragon';
import { faCheckCircle as fasCheckCircle, faMinus, faPlus } from '@fortawesome/free-solid-svg-icons';
Expand All @@ -10,7 +12,9 @@ import SequenceLink from './SequenceLink';
import { useModel } from '../../generic/model-store';

import genericMessages from '../../generic/messages';
import { useScrollTo } from './hooks';
import messages from './messages';
import './Section.scss';

const Section = ({
courseId,
Expand All @@ -29,6 +33,10 @@ const Section = ({
sequences,
},
} = useModel('outline', courseId);
// A section or subsection is selected by its id being the location hash part.
// location.hash will contain an initial # sign, so remove it here.
const hashValue = useLocation().hash.substring(1);
const selected = hashValue === section.id;

const [open, setOpen] = useState(defaultOpen);

Expand All @@ -41,6 +49,8 @@ const Section = ({
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

const sectionRef = useScrollTo(selected);

const sectionTitle = (
<div className="row w-100 m-0">
<div className="col-auto p-0">
Expand Down Expand Up @@ -72,9 +82,9 @@ const Section = ({
);

return (
<li>
<li ref={sectionRef}>
<Collapsible
className="mb-2"
className={classNames('mb-2 section', { 'section-selected': selected })}
styling="card-lg"
title={sectionTitle}
open={open}
Expand Down Expand Up @@ -104,6 +114,8 @@ const Section = ({
courseId={courseId}
sequence={sequences[sequenceId]}
first={index === 0}
last={index === (sequenceIds.length - 1)}
selected={hashValue === sequenceId}
/>
))}
</ol>
Expand Down
15 changes: 15 additions & 0 deletions src/course-home/outline-tab/Section.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
@import "~@edx/brand/paragon/variables";
@import "~@edx/paragon/scss/core/core";
@import "~@edx/brand/paragon/overrides";

.section .collapsible-body {
/* Internal SequenceLink components will have padding instead so when
* highlighted the highlighting reaches the top and/or bottom of the
* collapsible body. */
padding-top: 0;
padding-bottom: 0;
}

.section-selected > .collapsible-trigger {
background-color: $light-300;
}
22 changes: 20 additions & 2 deletions src/course-home/outline-tab/SequenceLink.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,18 @@ import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';

import EffortEstimate from '../../shared/effort-estimate';
import { useModel } from '../../generic/model-store';
import { useScrollTo } from './hooks';
import messages from './messages';
import './SequenceLink.scss';

const SequenceLink = ({
id,
intl,
courseId,
first,
last,
sequence,
selected,
}) => {
const {
complete,
Expand All @@ -39,6 +43,8 @@ const SequenceLink = ({
const coursewareUrl = <Link to={`/course/${courseId}/${id}`}>{title}</Link>;
const displayTitle = showLink ? coursewareUrl : title;

const sequenceLinkRef = useScrollTo(selected);

const dueDateMessage = (
<FormattedMessage
id="learning.outline.sequence-due-date-set"
Expand Down Expand Up @@ -84,8 +90,18 @@ const SequenceLink = ({
);

return (
<li>
<div className={classNames('', { 'mt-2 pt-2 border-top border-light': !first })}>
<li
ref={sequenceLinkRef}
className={classNames('', { 'sequence-link-selected': selected })}
>
<div
className={classNames('', {
'pt-2 border-top border-light': !first,
'pt-2.5': first,
'pb-2': !last,
'pb-2.5': last,
})}
>
<div className="row w-100 m-0">
<div className="col-auto p-0">
{complete ? (
Expand Down Expand Up @@ -129,7 +145,9 @@ SequenceLink.propTypes = {
intl: intlShape.isRequired,
courseId: PropTypes.string.isRequired,
first: PropTypes.bool.isRequired,
last: PropTypes.bool.isRequired,
sequence: PropTypes.shape().isRequired,
selected: PropTypes.bool.isRequired,
};

export default injectIntl(SequenceLink);
7 changes: 7 additions & 0 deletions src/course-home/outline-tab/SequenceLink.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
@import "~@edx/brand/paragon/variables";
@import "~@edx/paragon/scss/core/core";
@import "~@edx/brand/paragon/overrides";

.sequence-link-selected {
background-color: $light-300;
}
21 changes: 21 additions & 0 deletions src/course-home/outline-tab/hooks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/* eslint-disable import/prefer-default-export */

import { useEffect, useRef, useState } from 'react';

function useScrollTo(shouldScroll) {
const ref = useRef(null);
const [scrolled, setScrolled] = useState(false);

useEffect(() => {
if (shouldScroll && !scrolled) {
setScrolled(true);
ref.current.scrollIntoView({ behavior: 'smooth' });
}
}, [shouldScroll, scrolled]);

return ref;
}

export {
useScrollTo,
};
16 changes: 10 additions & 6 deletions src/courseware/course/CourseBreadcrumbs.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@ const CourseBreadcrumb = ({
content, withSeparator, courseId, sequenceId, unitId, isStaff,
}) => {
const defaultContent = content.filter(destination => destination.default)[0] || { id: courseId, label: '', sequences: [] };

let regularLinkRoute;
if (getConfig().ENABLE_LEGACY_NAV === 'true') {
regularLinkRoute = `/course/${courseId}/home#${defaultContent.id}`;
} else if (defaultContent.sequences.length) {
regularLinkRoute = `/course/${courseId}/${defaultContent.sequences[0].id}`;
} else {
regularLinkRoute = `/course/${courseId}/${defaultContent.id}`;
}
return (
<>
{withSeparator && (
Expand All @@ -23,12 +32,7 @@ const CourseBreadcrumb = ({
<li className="reactive-crumbs">
{ getConfig().ENABLE_JUMPNAV !== 'true' || content.length < 2 || !isStaff
? (
<Link
className="text-primary-500"
to={defaultContent.sequences.length
? `/course/${courseId}/${defaultContent.sequences[0].id}`
: `/course/${courseId}/${defaultContent.id}`}
>
<Link className="text-primary-500" to={regularLinkRoute}>
{defaultContent.label}
</Link>
)
Expand Down
47 changes: 35 additions & 12 deletions src/courseware/course/CourseBreadcrumbs.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,23 +106,46 @@ describe('CourseBreadcrumbs', () => {
},
],
]);
render(
<IntlProvider>
<BrowserRouter>
<CourseBreadcrumbs
courseId="course-v1:edX+DemoX+Demo_Course"
sectionId="block-v1:edX+DemoX+Demo_Course+type@chapter+block@interactive_demonstrations"
sequenceId="block-v1:edX+DemoX+Demo_Course+type@sequential+block@basic_questions"
isStaff
/>
</BrowserRouter>,
</IntlProvider>,
);
it('renders course breadcrumbs as expected', async () => {
await render(
<IntlProvider>
<BrowserRouter>
<CourseBreadcrumbs
courseId="course-v1:edX+DemoX+Demo_Course"
sectionId="block-v1:edX+DemoX+Demo_Course+type@chapter+block@interactive_demonstrations"
sequenceId="block-v1:edX+DemoX+Demo_Course+type@sequential+block@basic_questions"
isStaff
/>
</BrowserRouter>,
</IntlProvider>,
);
expect(screen.queryAllByRole('link')).toHaveLength(1);
const courseHomeButtonDestination = screen.getAllByRole('link')[0].href;
expect(courseHomeButtonDestination).toBe('http://localhost/course/course-v1:edX+DemoX+Demo_Course/home');
expect(screen.getByRole('navigation', { name: 'breadcrumb' })).toBeInTheDocument();
expect(screen.queryAllByRole('button')).toHaveLength(2);
});
it('renders legacy navigation links as expected', async () => {
getConfig.mockImplementation(() => ({
ENABLE_JUMPNAV: 'false',
ENABLE_LEGACY_NAV: 'true',
}));
await render(
<IntlProvider>
<BrowserRouter>
<CourseBreadcrumbs
courseId="course-v1:edX+DemoX+Demo_Course"
sectionId="block-v1:edX+DemoX+Demo_Course+type@chapter+block@interactive_demonstrations"
sequenceId="block-v1:edX+DemoX+Demo_Course+type@sequential+block@basic_questions"
/>
</BrowserRouter>,
</IntlProvider>,
);
expect(screen.queryAllByRole('link')).toHaveLength(3);
const sectionButtonDestination = screen.getAllByRole('link')[1].href;
expect(sectionButtonDestination).toBe('http://localhost/course/course-v1:edX+DemoX+Demo_Course/home#block-v1:edX+DemoX+Demo_Course+type@chapter+block@interactive_demonstrations');
const sequenceButtonDestination = screen.getAllByRole('link')[2].href;
expect(sequenceButtonDestination).toBe('http://localhost/course/course-v1:edX+DemoX+Demo_Course/home#block-v1:edX+DemoX+Demo_Course+type@sequential+block@basic_questions');
expect(screen.queryAllByRole('button')).toHaveLength(0);
});
});
1 change: 1 addition & 0 deletions src/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ initialize({
DISCUSSIONS_MFE_BASE_URL: process.env.DISCUSSIONS_MFE_BASE_URL || null,
ENTERPRISE_LEARNER_PORTAL_HOSTNAME: process.env.ENTERPRISE_LEARNER_PORTAL_HOSTNAME || null,
ENABLE_JUMPNAV: process.env.ENABLE_JUMPNAV || null,
ENABLE_LEGACY_NAV: process.env.ENABLE_LEGACY_NAV || null,
ENABLE_NOTICES: process.env.ENABLE_NOTICES || null,
INSIGHTS_BASE_URL: process.env.INSIGHTS_BASE_URL || null,
SEARCH_CATALOG_URL: process.env.SEARCH_CATALOG_URL || null,
Expand Down

0 comments on commit 79d9cbc

Please sign in to comment.