fix: breadcrumb jump nav styling

openedx · Jun 8, 2023 · 5a99ca5 · 5a99ca5
1 parent 9943df4
commit 5a99ca5
Show file tree

Hide file tree

Showing 4 changed files with 89 additions and 51 deletions.
diff --git a/src/courseware/course/CourseBreadcrumbs.jsx b/src/courseware/course/CourseBreadcrumbs.jsx
@@ -1,57 +1,80 @@
-import React, { useMemo } from 'react';
+import React, { useMemo, useState } from 'react';
 import PropTypes from 'prop-types';
 import { getConfig } from '@edx/frontend-platform';
 import { FormattedMessage } from '@edx/frontend-platform/i18n';
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
 import { faHome } from '@fortawesome/free-solid-svg-icons';
 import { useSelector } from 'react-redux';
-import { SelectMenu } from '@edx/paragon';
+import { useToggle, ModalPopup, Menu } from '@edx/paragon';
 import { Link } from 'react-router-dom';
 import { useModel, useModels } from '../../generic/model-store';
 import JumpNavMenuItem from './JumpNavMenuItem';
 
 const CourseBreadcrumb = ({
-  content, withSeparator, courseId, sequenceId, unitId, isStaff,
+  content,
+  withSeparator,
+  courseId,
+  sequenceId,
+  unitId,
+  isStaff,
 }) => {
-  const defaultContent = content.filter(destination => destination.default)[0] || { id: courseId, label: '', sequences: [] };
+  const defaultContent = content.filter(
+    (destination) => destination.default,
+  )[0] || { id: courseId, label: '', sequences: [] };
+
+  const showRegularLink = getConfig().ENABLE_JUMPNAV !== 'true' || content.length < 2 || !isStaff;
+  const [isOpen, open, close] = useToggle(false);
+  const [target, setTarget] = useState(null);
   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',
-      }}
+      <li
+        style={{
+          overflow: 'hidden',
+          textOverflow: 'ellipsis',
+          whiteSpace: 'nowrap',
+        }}
+        data-testid="breadcrumb-item"
       >
-        { getConfig().ENABLE_JUMPNAV !== 'true' || content.length < 2 || !isStaff
-          ? (
-            <Link
-              className="text-primary-500"
-              to={defaultContent.sequences.length
+        {showRegularLink ? (
+          <Link
+            className="text-primary-500"
+            to={
+              defaultContent.sequences.length
                 ? `/course/${courseId}/${defaultContent.sequences[0].id}`
-                : `/course/${courseId}/${defaultContent.id}`}
-            >
-              {defaultContent.label}
-            </Link>
-          )
-          : (
-            <SelectMenu isLink defaultMessage={defaultContent.label}>
-              {content.map(item => (
-                <JumpNavMenuItem
-                  isDefault={item.default}
-                  sequences={item.sequences}
-                  courseId={courseId}
-                  title={item.label}
-                  currentSequence={sequenceId}
-                  currentUnit={unitId}
-                />
-              ))}
-            </SelectMenu>
-          )}
-
+                : `/course/${courseId}/${defaultContent.id}`
+            }
+          >
+            {defaultContent.label}
+          </Link>
+        ) : (
+          <>
+            {
+              // eslint-disable-next-line
+              <a className="text-primary-500" onClick={open} ref={setTarget}>
+                {defaultContent.label}
+              </a>
+            }
+            <ModalPopup positionRef={target} isOpen={isOpen} onClose={close}>
+              <Menu>
+                {content.map((item) => (
+                  <JumpNavMenuItem
+                    isDefault={item.default}
+                    sequences={item.sequences}
+                    courseId={courseId}
+                    title={item.label}
+                    currentSequence={sequenceId}
+                    currentUnit={unitId}
+                    onClick={close}
+                  />
+                ))}
+              </Menu>
+            </ModalPopup>
+          </>
+        )}
       </li>
     </>
   );
@@ -87,14 +110,21 @@ const CourseBreadcrumbs = ({
   isStaff,
 }) => {
   const course = useModel('coursewareMeta', courseId);
-  const courseStatus = useSelector(state => state.courseware.courseStatus);
-  const sequenceStatus = useSelector(state => state.courseware.sequenceStatus);
+  const courseStatus = useSelector((state) => state.courseware.courseStatus);
+  const sequenceStatus = useSelector(
+    (state) => state.courseware.sequenceStatus,
+  );
 
-  const allSequencesInSections = Object.fromEntries(useModels('sections', course.sectionIds).map(section => [section.id, {
-    default: section.id === sectionId,
-    title: section.title,
-    sequences: useModels('sequences', section.sequenceIds),
-  }]));
+  const allSequencesInSections = Object.fromEntries(
+    useModels('sections', course.sectionIds).map((section) => [
+      section.id,
+      {
+        default: section.id === sectionId,
+        title: section.title,
+        sequences: useModels('sequences', section.sequenceIds),
+      },
+    ]),
+  );
 
   const links = useMemo(() => {
     const chapters = [];
@@ -108,7 +138,7 @@ const CourseBreadcrumbs = ({
           sequences: section.sequences,
         });
         if (section.default) {
-          section.sequences.forEach(sequence => {
+          section.sequences.forEach((sequence) => {
             sequentials.push({
               id: sequence.id,
               label: sequence.title,
@@ -124,7 +154,7 @@ const CourseBreadcrumbs = ({
 
   return (
     <nav aria-label="breadcrumb" className="my-4 d-inline-block col-sm-10">
-      <ol className="list-unstyled d-flex  flex-nowrap align-items-center m-0">
+      <ol className="list-unstyled d-flex flex-nowrap align-items-center m-0">
         <li className="list-unstyled col-auto m-0 p-0">
           <Link
             className="flex-shrink-0 text-primary"
@@ -138,7 +168,7 @@ const CourseBreadcrumbs = ({
             />
           </Link>
         </li>
-        {links.map(content => (
+        {links.map((content) => (
           <CourseBreadcrumb
             courseId={courseId}
             sequenceId={sequenceId}

diff --git a/src/courseware/course/CourseBreadcrumbs.test.jsx b/src/courseware/course/CourseBreadcrumbs.test.jsx
@@ -123,6 +123,6 @@ describe('CourseBreadcrumbs', () => {
     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);
+    expect(screen.queryAllByTestId('breadcrumb-item')).toHaveLength(2);
   });
 });
diff --git a/src/courseware/course/JumpNavMenuItem.jsx b/src/courseware/course/JumpNavMenuItem.jsx
@@ -1,7 +1,7 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import { history } from '@edx/frontend-platform';
-import { MenuItem } from '@edx/paragon';
+import { Dropdown } from '@edx/paragon';
 
 import {
   sendTrackingLogEvent,
@@ -15,6 +15,7 @@ const JumpNavMenuItem = ({
   currentUnit,
   sequences,
   isDefault,
+  onClick,
 }) => {
   function logEvent(targetUrl) {
     const eventName = 'edx.ui.lms.jump_nav.selected';
@@ -34,33 +35,39 @@ const JumpNavMenuItem = ({
     }
     return `/course/${courseId}/${sequences[0].id}`;
   }
-  function handleClick() {
+  function handleClick(e) {
     const url = destinationUrl();
     logEvent(url);
     history.push(url);
+    if (onClick) { onClick(e); }
   }
 
   return (
-    <MenuItem
-      defaultSelected={isDefault}
-      onClick={() => handleClick()}
+    <Dropdown.Item
+      active={isDefault}
+      onClick={e => handleClick(e)}
     >
       {title}
-    </MenuItem>
+    </Dropdown.Item>
   );
 };
 
 const sequenceShape = PropTypes.shape({
   id: PropTypes.string.isRequired,
 });
 
+JumpNavMenuItem.defaultProps = {
+  onClick: null,
+};
+
 JumpNavMenuItem.propTypes = {
   title: PropTypes.string.isRequired,
   sequences: PropTypes.arrayOf(sequenceShape).isRequired,
   isDefault: PropTypes.bool.isRequired,
   courseId: PropTypes.string.isRequired,
   currentSequence: PropTypes.string.isRequired,
   currentUnit: PropTypes.string.isRequired,
+  onClick: PropTypes.func,
 };
 
 export default JumpNavMenuItem;
diff --git a/src/courseware/course/JumpNavMenuItem.test.jsx b/src/courseware/course/JumpNavMenuItem.test.jsx
@@ -22,6 +22,7 @@ const mockData = {
     },
   ],
   isDefault: false,
+  onClick: jest.fn().mockName('onClick'),
 };
 describe('JumpNavMenuItem', () => {
   render(
-Original file line number
+Diff line change
@@ Expand Up / @@ -22,6 +22,7 @@ const mockData = { @@
         },
       ],
       isDefault: false,
+      onClick: jest.fn().mockName('onClick'),
     };
     describe('JumpNavMenuItem', () => {
       render(
@@ Expand Down @@