feat: legacy course navigation

Add an option to enable the legacy course navigation where clicking a breadcrumb leads to the course index page highlighting the selected section.
openedx · Nov 24, 2023 · f158ea9 · f158ea9
1 parent bce25c4
commit f158ea9
Show file tree

Hide file tree

Showing 12 changed files with 129 additions and 24 deletions.
diff --git a/.env b/.env
@@ -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=''

diff --git a/.env.development b/.env.development
@@ -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=''

diff --git a/README.rst b/README.rst
@@ -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.

diff --git a/src/course-home/outline-tab/OutlineTab.jsx b/src/course-home/outline-tab/OutlineTab.jsx
@@ -123,6 +123,14 @@ 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);
+  const selectedSectionId = courses[rootCourseId].sectionIds.find((sectionId) => (
+    // Section is selected or contains selected subsection.
+    (hashValue === sectionId) || (sections[sectionId].sequenceIds.includes(hashValue))
+  ));
+
   return (
     <>
       <div data-learner-type={learnerType} className="row w-100 mx-0 my-3 justify-content-between">
@@ -173,7 +181,11 @@ const OutlineTab = ({ intl }) => {
                   <Section
                     key={sectionId}
                     courseId={courseId}
-                    defaultOpen={sections[sectionId].resumeBlock}
+                    defaultOpen={
+                      (selectedSectionId !== undefined)
+                        ? sectionId === selectedSectionId
+                        : sections[sectionId].resumeBlock
+                    }
                     expand={expandAll}
                     section={sections[sectionId]}
                   />

diff --git a/src/course-home/outline-tab/OutlineTab.test.jsx b/src/course-home/outline-tab/OutlineTab.test.jsx
@@ -107,6 +107,26 @@ describe('Outline Tab', () => {
       expect(expandedSectionNode).toHaveAttribute('aria-expanded', 'true');
     });
 
+    it('expands 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');
+    });
+
+    it('expands 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');
+    });
+
     it('handles expand/collapse all button click', async () => {
       await fetchAndRender();
       // Button renders as "Expand All"

diff --git a/src/course-home/outline-tab/Section.jsx b/src/course-home/outline-tab/Section.jsx
@@ -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';
@@ -11,6 +13,7 @@ import { useModel } from '../../generic/model-store';
 
 import genericMessages from '../../generic/messages';
 import messages from './messages';
+import './Section.scss';
 
 const Section = ({
   courseId,
@@ -29,6 +32,9 @@ const Section = ({
       sequences,
     },
   } = useModel('outline', courseId);
+  // Remove the initial # sign.
+  const hashValue = useLocation().hash.substring(1);
+  const selected = hashValue === section.id;
 
   const [open, setOpen] = useState(defaultOpen);
 
@@ -74,7 +80,7 @@ const Section = ({
   return (
     <li>
       <Collapsible
-        className="mb-2"
+        className={classNames('mb-2 section', { 'section-selected': selected })}
         styling="card-lg"
         title={sectionTitle}
         open={open}
@@ -104,6 +110,8 @@ const Section = ({
               courseId={courseId}
               sequence={sequences[sequenceId]}
               first={index === 0}
+              last={index === (sequenceIds.length - 1)}
+              selected={hashValue === sequenceId}
             />
           ))}
         </ol>

diff --git a/src/course-home/outline-tab/Section.scss b/src/course-home/outline-tab/Section.scss
@@ -0,0 +1,14 @@
+@import "~@edx/brand/paragon/variables";
+@import "~@edx/paragon/scss/core/core";
+@import "~@edx/brand/paragon/overrides";
+
+.section > div > div > .collapsible-body {
+  /* Internal SequenceLink components will have padding so the highlighting
+   * reaches the top and/or bottom of the collapsible. */
+  padding-top: 0;
+  padding-bottom: 0;
+}
+
+.section-selected > .collapsible-trigger {
+  background-color: $light-300;
+}
diff --git a/src/course-home/outline-tab/SequenceLink.jsx b/src/course-home/outline-tab/SequenceLink.jsx
@@ -15,13 +15,16 @@ import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
 import EffortEstimate from '../../shared/effort-estimate';
 import { useModel } from '../../generic/model-store';
 import messages from './messages';
+import './SequenceLink.scss';
 
 const SequenceLink = ({
   id,
   intl,
   courseId,
   first,
+  last,
   sequence,
+  selected,
 }) => {
   const {
     complete,
@@ -84,8 +87,15 @@ const SequenceLink = ({
   );
 
   return (
-    <li>
-      <div className={classNames('', { 'mt-2 pt-2 border-top border-light': !first })}>
+    <li 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 ? (
@@ -129,7 +139,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);
diff --git a/src/course-home/outline-tab/SequenceLink.scss b/src/course-home/outline-tab/SequenceLink.scss
@@ -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;
+}
diff --git a/src/courseware/course/CourseBreadcrumbs.jsx b/src/courseware/course/CourseBreadcrumbs.jsx
@@ -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 === '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 && (
@@ -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>
         ) : (

diff --git a/src/courseware/course/CourseBreadcrumbs.test.jsx b/src/courseware/course/CourseBreadcrumbs.test.jsx
@@ -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);
+  });
 });
diff --git a/src/index.jsx b/src/index.jsx
@@ -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,