Merge pull request #7 from open-craft/0x29a/bb8366/quince-shared-branch

feat: port code drift for open-craft/frontend-app-learning [BB-8367]

.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=''

.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=''

README.rst

@@ -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.

src/course-home/outline-tab/LmsHtmlFragment.jsx

@@ -29,7 +29,7 @@ const LmsHtmlFragment = ({
   const iframe = useRef(null);
   function resetIframeHeight() {
     if (iframe?.current?.contentWindow?.document?.body) {
-      iframe.current.height = iframe.current.contentWindow.document.body.scrollHeight;
+      iframe.current.height = iframe.current.contentWindow.document.body.parentNode.scrollHeight;
     }
   }
 

src/course-home/outline-tab/OutlineTab.jsx

@@ -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">
@@ -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]}
                   />

src/course-home/outline-tab/OutlineTab.test.jsx

@@ -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();
@@ -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"
@@ -257,6 +289,22 @@ describe('Outline Tab', () => {
       });
     });
 
+    it('ignores comments and misformatted HTML', async () => {
+      setTabData({
+        welcome_message_html: '<p class="additional-spaces-in-tag"   >'
+      + '<!-- Even if the welcome_message_html length is above the limit because of comments, we hope it will not be shortened. -->'
+      + '<!-- Even if the welcome_message_html length is above the limit because of comments, we hope it will not be shortened. -->'
+      + 'Test welcome message that happens to be longer than one hundred words because of comments but displayed content is less.'
+      + 'It should not be shortened.'
+      + '<!-- Even if the welcome_message_html length is above the limit because of comments, we hope it will not be shortened. -->'
+      + '<!-- Even if the welcome_message_html length is above the limit because of comments, we hope it will not be shortened. -->'
+      + '</p>',
+      });
+      await fetchAndRender();
+      const showMoreButton = screen.queryByRole('button', { name: 'Show More' });
+      expect(showMoreButton).not.toBeInTheDocument();
+    });
+
     it('does not display if no update available', async () => {
       setTabData({ welcome_message_html: null });
       await fetchAndRender();

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';
@@ -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,
@@ -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);
 
@@ -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">
@@ -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}
@@ -104,6 +114,8 @@ const Section = ({
               courseId={courseId}
               sequence={sequences[sequenceId]}
               first={index === 0}
+              last={index === (sequenceIds.length - 1)}
+              selected={hashValue === sequenceId}
             />
           ))}
         </ol>

src/course-home/outline-tab/Section.scss

@@ -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;
+}

src/course-home/outline-tab/SequenceLink.jsx

@@ -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,
@@ -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"
@@ -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 ? (
@@ -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);

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;
+}

src/course-home/outline-tab/hooks.js

@@ -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,
+};

src/course-home/outline-tab/widgets/WelcomeMessage.jsx

@@ -1,4 +1,4 @@
-import React, { useState } from 'react';
+import React, { useState, useMemo } from 'react';
 import PropTypes from 'prop-types';
 
 import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
@@ -18,8 +18,22 @@ const WelcomeMessage = ({ courseId, intl }) => {
 
   const [display, setDisplay] = useState(true);
 
-  const shortWelcomeMessageHtml = truncate(welcomeMessageHtml, 100, { byWords: true, keepWhitespaces: true });
-  const messageCanBeShortened = shortWelcomeMessageHtml.length < welcomeMessageHtml.length;
+  // welcomeMessageHtml can contain comments or malformatted HTML which can impact the length that determines
+  // messageCanBeShortened. We clean it by calling truncate with a length of welcomeMessageHtml.length which
+  // will not result in a truncation but a formatting into 'truncate-html' canonical format.
+  const cleanedWelcomeMessageHtml = useMemo(
+    () => truncate(welcomeMessageHtml, welcomeMessageHtml.length, { keepWhitespaces: true }),
+    [welcomeMessageHtml],
+  );
+  const shortWelcomeMessageHtml = useMemo(
+    () => truncate(cleanedWelcomeMessageHtml, 100, { byWords: true, keepWhitespaces: true }),
+    [cleanedWelcomeMessageHtml],
+  );
+  const messageCanBeShortened = useMemo(
+    () => (shortWelcomeMessageHtml.length < cleanedWelcomeMessageHtml.length),
+    [cleanedWelcomeMessageHtml, shortWelcomeMessageHtml],
+  );
+
   const [showShortMessage, setShowShortMessage] = useState(messageCanBeShortened);
   const dispatch = useDispatch();
 
@@ -63,7 +77,7 @@ const WelcomeMessage = ({ courseId, intl }) => {
             className="inline-link"
             data-testid="long-welcome-message-iframe"
             key="full-html"
-            html={welcomeMessageHtml}
+            html={cleanedWelcomeMessageHtml}
             title={intl.formatMessage(messages.welcomeMessage)}
           />
         )}

src/courseware/course/CourseBreadcrumbs.jsx

@@ -25,29 +25,27 @@ 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 && (
         <li className="col-auto p-0 mx-2 text-primary-500 text-truncate text-nowrap" role="presentation" aria-hidden>/</li>
       )}
 
       <li
-        style={{
-          overflow: 'hidden',
-          textOverflow: 'ellipsis',
-          whiteSpace: 'nowrap',
-        }}
+        className="reactive-crumbs"
         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>
         ) : (