Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: legacy course navigation #1239

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
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
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=''
ArturGaspar marked this conversation as resolved.
Show resolved Hide resolved
ENABLE_NOTICES=''
ENTERPRISE_LEARNER_PORTAL_HOSTNAME='localhost:8734'
EXAMS_BASE_URL=''
Expand Down
4 changes: 4 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,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 @@ -123,6 +123,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 @@ -173,7 +182,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,
};
18 changes: 10 additions & 8 deletions src/courseware/course/CourseBreadcrumbs.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ const CourseBreadcrumb = ({
const showRegularLink = getConfig().ENABLE_JUMPNAV !== 'true' || content.length < 2 || !isStaff;
const [isOpen, open, close] = useToggle(false);
const [target, setTarget] = useState(null);

let regularLinkRoute;
if (getConfig().ENABLE_LEGACY_NAV) {
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 @@ -40,14 +49,7 @@ const CourseBreadcrumb = ({
data-testid="breadcrumb-item"
>
{showRegularLink ? (
<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 @@ -112,23 +112,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.queryAllByTestId('breadcrumb-item')).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 @@ -154,6 +154,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