refactor: improve course outline page

feat: lms live link chore: update outline link fix: course outline link refactor: remove unnecessary css and rename test file refactor: remove unnecessary css from outlineSidebar test: make use of message variable instead of hardcoded text refactor: remove unnecessary h5 class test: use test id for detecting component refactor: update course outline url and some default messages
openedx · Dec 6, 2023 · 80cc5e3 · 80cc5e3
1 parent 2d8e1d6
commit 80cc5e3
Show file tree

Hide file tree

Showing 21 changed files with 115 additions and 77 deletions.
diff --git a/src/CourseAuthoringRoutes.jsx b/src/CourseAuthoringRoutes.jsx
@@ -42,7 +42,7 @@ const CourseAuthoringRoutes = () => {
     <CourseAuthoringPage courseId={courseId}>
       <Routes>
         <Route
-          path="outline"
+          path="/"
           element={<PageWrap><CourseOutline courseId={courseId} /></PageWrap>}
         />
         <Route

diff --git a/src/course-outline/CourseOutline.jsx b/src/course-outline/CourseOutline.jsx
@@ -50,7 +50,7 @@ const CourseOutline = ({ courseId }) => {
 
   return (
     <>
-      <Container size="xl" className="m-4">
+      <Container size="xl" className="px-4">
         <section className="course-outline-container mb-4 mt-5">
           <TransitionReplace>
             {showSuccessAlert ? (
@@ -71,7 +71,6 @@ const CourseOutline = ({ courseId }) => {
             className="mt-5"
             title={intl.formatMessage(messages.headingTitle)}
             subtitle={intl.formatMessage(messages.headingSubtitle)}
-            withSubHeaderContent={false}
             headerActions={(
               <HeaderNavigations
                 isReIndexShow={isReIndexShow}
@@ -84,8 +83,8 @@ const CourseOutline = ({ courseId }) => {
           <Layout
             lg={[{ span: 9 }, { span: 3 }]}
             md={[{ span: 9 }, { span: 3 }]}
-            sm={[{ span: 9 }, { span: 3 }]}
-            xs={[{ span: 9 }, { span: 3 }]}
+            sm={[{ span: 12 }, { span: 12 }]}
+            xs={[{ span: 12 }, { span: 12 }]}
             xl={[{ span: 9 }, { span: 3 }]}
           >
             <Layout.Element>
@@ -94,7 +93,7 @@ const CourseOutline = ({ courseId }) => {
                   <section className="course-outline-section">
                     <StatusBar
                       courseId={courseId}
-                      isLaoding={isLoading}
+                      isLoading={isLoading}
                       statusBarData={statusBarData}
                       openEnableHighlightsModal={openEnableHighlightsModal}
                     />

diff --git a/src/course-outline/CourseOutline.scss b/src/course-outline/CourseOutline.scss
@@ -1,3 +1,2 @@
 @import "./header-navigations/HeaderNavigations";
-@import "./outline-sidebar/OulineSidebar";
 @import "./status-bar/StatusBar";
diff --git a/src/course-outline/CourseOutline.test.jsx b/src/course-outline/CourseOutline.test.jsx
@@ -1,7 +1,5 @@
 import React from 'react';
-import {
-  render, waitFor, cleanup,
-} from '@testing-library/react';
+import { render, waitFor } from '@testing-library/react';
 import { IntlProvider } from '@edx/frontend-platform/i18n';
 import { AppProvider } from '@edx/frontend-platform/react';
 import { initializeMockApp } from '@edx/frontend-platform';
@@ -127,9 +125,8 @@ describe('<CourseOutline />', () => {
     expect(getByText('4/9 completed')).toBeInTheDocument();
   });
 
-  it('check enable highlights when enable highlights query is successfully', async () => {
-    cleanup();
-    const { getByText } = render(<RootWrapper />);
+  it('check highlights are enabled after enable highlights query is successful', async () => {
+    const { findByTestId } = render(<RootWrapper />);
 
     axiosMock
       .onGet(getCourseOutlineIndexApiUrl(courseId))
@@ -148,6 +145,6 @@ describe('<CourseOutline />', () => {
       .reply(200);
 
     await executeThunk(enableCourseHighlightsEmailsQuery(courseId), store.dispatch);
-    expect(getByText('Enabled')).toBeInTheDocument();
+    expect(await findByTestId('highlights-enabled-span')).toBeInTheDocument();
   });
 });
diff --git a/src/course-outline/data/api.js b/src/course-outline/data/api.js
@@ -2,22 +2,27 @@ import { camelCaseObject, getConfig } from '@edx/frontend-platform';
 import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
 
 const getApiBaseUrl = () => getConfig().STUDIO_BASE_URL;
+
 export const getCourseOutlineIndexApiUrl = (courseId) => `${getApiBaseUrl()}/api/contentstore/v1/course_index/${courseId}`;
+
 export const getCourseBestPracticesApiUrl = ({
   courseId,
   excludeGraded,
   all,
 }) => `${getApiBaseUrl()}/api/courses/v1/quality/${courseId}/?exclude_graded=${excludeGraded}&all=${all}`;
+
 export const getCourseLaunchApiUrl = ({
   courseId,
   gradedOnly,
   validateOras,
   all,
 }) => `${getApiBaseUrl()}/api/courses/v1/validation/${courseId}/?graded_only=${gradedOnly}&validate_oras=${validateOras}&all=${all}`;
+
 export const getEnableHighlightsEmailsApiUrl = (courseId) => {
   const formattedCourseId = courseId.split('course-v1:')[1];
   return `${getApiBaseUrl()}/xblock/block-v1:${formattedCourseId}+type@course+block@course`;
 };
+
 export const getCourseReindexApiUrl = (reindexLink) => `${getApiBaseUrl()}${reindexLink}`;
 
 /**

diff --git a/src/course-outline/enable-highlights-modal/messages.js b/src/course-outline/enable-highlights-modal/messages.js
@@ -19,7 +19,7 @@ const messages = defineMessages({
   },
   cancelButton: {
     id: 'course-authoring.course-outline.status-bar.modal.cancelButton',
-    defaultMessage: 'Not yet',
+    defaultMessage: 'Cancel',
   },
   submitButton: {
     id: 'course-authoring.course-outline.status-bar.modal.submitButton',

diff --git a/src/course-outline/header-navigations/HeaderNavigations.jsx b/src/course-outline/header-navigations/HeaderNavigations.jsx
@@ -5,6 +5,7 @@ import { Button, OverlayTrigger, Tooltip } from '@edx/paragon';
 import {
   Add as IconAdd,
   ArrowDropDown as ArrowDownIcon,
+  ArrowDropUp as ArrowUpIcon,
 } from '@edx/paragon/icons';
 
 import messages from './messages';
@@ -17,7 +18,7 @@ const HeaderNavigations = ({
 }) => {
   const intl = useIntl();
   const {
-    handleNewSection, handleReIndex, handleExpandAll, handleViewLive,
+    handleNewSection, handleReIndex, handleExpandAll, lmsLink,
   } = headerNavigationsActions;
 
   return (
@@ -57,8 +58,7 @@ const HeaderNavigations = ({
       )}
       <Button
         variant="outline-primary"
-        className={isSectionsExpanded ? 'expand-button-active' : ''}
-        iconBefore={ArrowDownIcon}
+        iconBefore={isSectionsExpanded ? ArrowUpIcon : ArrowDownIcon}
         onClick={handleExpandAll}
       >
         {isSectionsExpanded
@@ -74,7 +74,8 @@ const HeaderNavigations = ({
         )}
       >
         <Button
-          onClick={handleViewLive}
+          href={lmsLink}
+          target="_blank"
           variant="outline-primary"
         >
           {intl.formatMessage(messages.viewLiveButton)}
@@ -92,7 +93,7 @@ HeaderNavigations.propTypes = {
     handleNewSection: PropTypes.func.isRequired,
     handleReIndex: PropTypes.func.isRequired,
     handleExpandAll: PropTypes.func.isRequired,
-    handleViewLive: PropTypes.func.isRequired,
+    lmsLink: PropTypes.string.isRequired,
   }).isRequired,
 };
 

diff --git a/src/course-outline/header-navigations/HeaderNavigations.scss b/src/course-outline/header-navigations/HeaderNavigations.scss
@@ -1,10 +1,4 @@
 .header-navigations {
   display: flex;
   gap: .75rem;
-
-  .expand-button-active {
-    .pgn__icon {
-      transform: rotate(180deg);
-    }
-  }
 }
diff --git a/...der-navigations/HeaderNavigation.test.jsx → ...er-navigations/HeaderNavigations.test.jsx b/...der-navigations/HeaderNavigation.test.jsx → ...er-navigations/HeaderNavigations.test.jsx
@@ -8,20 +8,20 @@ import messages from './messages';
 const handleNewSectionMock = jest.fn();
 const handleReIndexMock = jest.fn();
 const handleExpandAllMock = jest.fn();
-const handleViewLiveMock = jest.fn();
 
 const headerNavigationsActions = {
   handleNewSection: handleNewSectionMock,
   handleReIndex: handleReIndexMock,
   handleExpandAll: handleExpandAllMock,
-  handleViewLive: handleViewLiveMock,
+  lmsLink: '',
 };
 
 const renderComponent = (props) => render(
   <IntlProvider locale="en">
     <HeaderNavigations
       headerNavigationsActions={headerNavigationsActions}
       isSectionsExpanded={false}
+      isDisabledReindexButton={false}
       isReIndexShow
       {...props}
     />
@@ -61,10 +61,6 @@ describe('<HeaderNavigations />', () => {
     const expandAllButton = getByRole('button', { name: messages.expandAllButton.defaultMessage });
     fireEvent.click(expandAllButton);
     expect(handleExpandAllMock).toHaveBeenCalledTimes(1);
-
-    const viewLiveButton = getByRole('button', { name: messages.viewLiveButton.defaultMessage });
-    fireEvent.click(viewLiveButton);
-    expect(handleViewLiveMock).toHaveBeenCalledTimes(1);
   });
 
   it('render collapse button correctly', () => {

diff --git a/src/course-outline/hooks.jsx b/src/course-outline/hooks.jsx
@@ -21,7 +21,7 @@ import {
 const useCourseOutline = ({ courseId }) => {
   const dispatch = useDispatch();
 
-  const { reindexLink } = useSelector(getOutlineIndexData);
+  const { reindexLink, lmsLink } = useSelector(getOutlineIndexData);
   const { outlineIndexLoadingStatus, reIndexLoadingStatus } = useSelector(getLoadingStatus);
   const statusBarData = useSelector(getStatusBarData);
   const savingStatus = useSelector(getSavingStatus);
@@ -48,13 +48,10 @@ const useCourseOutline = ({ courseId }) => {
     handleExpandAll: () => {
       setSectionsExpanded((prevState) => !prevState);
     },
-    handleViewLive: () => {
-      // TODO add handler
-    },
+    lmsLink,
   };
 
   const handleEnableHighlightsSubmit = () => {
-    dispatch(updateSavingStatus({ status: RequestStatus.PENDING }));
     dispatch(enableCourseHighlightsEmailsQuery(courseId));
     closeEnableHighlightsModal();
   };

diff --git a/src/course-outline/outline-sidebar/OulineSidebar.scss b/src/course-outline/outline-sidebar/OulineSidebar.scss
diff --git a/src/course-outline/outline-sidebar/OutlineSidebar.jsx b/src/course-outline/outline-sidebar/OutlineSidebar.jsx
@@ -3,7 +3,7 @@ import PropTypes from 'prop-types';
 import { Hyperlink } from '@edx/paragon';
 import { useIntl } from '@edx/frontend-platform/i18n';
 
-import HelpSidebar from '../../generic/help-sidebar';
+import { HelpSidebar } from '../../generic/help-sidebar';
 import { useHelpUrls } from '../../help-urls/hooks';
 import { getFormattedSidebarMessages } from './utils';
 
@@ -41,7 +41,7 @@ const OutlineSideBar = ({ courseId }) => {
             {descriptions.map((description) => (
               <p className="help-sidebar-about-descriptions" key={description}>{description}</p>
             ))}
-            {Boolean(link) && (
+            {Boolean(link) && Boolean(link.href) && (
               <Hyperlink
                 className="small"
                 destination={link.href}

diff --git a/src/course-outline/outline-sidebar/OutlineSidebar.test.jsx b/src/course-outline/outline-sidebar/OutlineSidebar.test.jsx
@@ -1,13 +1,18 @@
 import React from 'react';
-import { render } from '@testing-library/react';
+import { render, waitFor } from '@testing-library/react';
+import MockAdapter from 'axios-mock-adapter';
+import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
 import { IntlProvider } from '@edx/frontend-platform/i18n';
 import { initializeMockApp } from '@edx/frontend-platform';
 import { AppProvider } from '@edx/frontend-platform/react';
 
+import { helpUrls } from '../../help-urls/__mocks__';
+import { getHelpUrlsApiUrl } from '../../help-urls/data/api';
 import initializeStore from '../../store';
 import OutlineSidebar from './OutlineSidebar';
 import messages from './messages';
 
+let axiosMock;
 let store;
 const mockPathname = '/foo-bar';
 const courseId = '123';
@@ -19,6 +24,13 @@ jest.mock('react-router-dom', () => ({
   }),
 }));
 
+jest.mock('@edx/frontend-platform/i18n', () => ({
+  ...jest.requireActual('@edx/frontend-platform/i18n'),
+  useIntl: () => ({
+    formatMessage: (message) => message.defaultMessage,
+  }),
+}));
+
 const renderComponent = (props) => render(
   <AppProvider store={store} messages={{}}>
     <IntlProvider locale="en">
@@ -38,32 +50,35 @@ describe('<OutlineSidebar />', () => {
       },
     });
     store = initializeStore();
+    axiosMock = new MockAdapter(getAuthenticatedHttpClient());
+    axiosMock
+      .onGet(getHelpUrlsApiUrl())
+      .reply(200, helpUrls);
   });
 
-  it('render OutlineSidebar component correctly', () => {
+  it('render OutlineSidebar component correctly', async () => {
     const { getByText } = renderComponent();
 
-    expect(getByText(messages.section_1_title.defaultMessage)).toBeInTheDocument();
-    expect(getByText(messages.section_1_descriptions_1.defaultMessage)).toBeInTheDocument();
-    expect(getByText(messages.section_1_descriptions_2.defaultMessage)).toBeInTheDocument();
+    await waitFor(() => {
+      expect(getByText(messages.section_1_title.defaultMessage)).toBeInTheDocument();
+      expect(getByText(messages.section_1_descriptions_1.defaultMessage)).toBeInTheDocument();
+      expect(getByText(messages.section_1_descriptions_2.defaultMessage)).toBeInTheDocument();
 
-    expect(getByText(messages.section_2_title.defaultMessage)).toBeInTheDocument();
-    expect(getByText(messages.section_2_descriptions_1.defaultMessage)).toBeInTheDocument();
-    expect(getByText(messages.section_2_link.defaultMessage)).toBeInTheDocument();
+      expect(getByText(messages.section_2_title.defaultMessage)).toBeInTheDocument();
+      expect(getByText(messages.section_2_descriptions_1.defaultMessage)).toBeInTheDocument();
+      expect(getByText(messages.section_2_link.defaultMessage)).toBeInTheDocument();
 
-    expect(getByText(messages.section_3_title.defaultMessage)).toBeInTheDocument();
-    expect(getByText(messages.section_3_descriptions_1.defaultMessage)).toBeInTheDocument();
-    expect(getByText(messages.section_3_link.defaultMessage)).toBeInTheDocument();
+      expect(getByText(messages.section_3_title.defaultMessage)).toBeInTheDocument();
+      expect(getByText(messages.section_3_descriptions_1.defaultMessage)).toBeInTheDocument();
+      expect(getByText(messages.section_3_link.defaultMessage)).toBeInTheDocument();
 
-    expect(getByText(messages.section_4_title.defaultMessage)).toBeInTheDocument();
-    expect(getByText(messages.section_4_descriptions_1.defaultMessage)).toBeInTheDocument();
+      expect(getByText(messages.section_4_title.defaultMessage)).toBeInTheDocument();
+      expect(getByText(messages.section_4_descriptions_1.defaultMessage)).toBeInTheDocument();
 
-    expect(getByText(/To make a section, subsection, or unit unavailable to learners, select the Configure icon for that level, then select the appropriate/i)).toBeInTheDocument();
-    expect(getByText(/option. Grades for hidden sections, subsections, and units are not included in grade calculations./i)).toBeInTheDocument();
+      expect(getByText(messages.section_4_descriptions_2.defaultMessage)).toBeInTheDocument();
+      expect(getByText(messages.section_4_descriptions_3.defaultMessage)).toBeInTheDocument();
 
-    expect(getByText(/To hide the content of a subsection from learners after the subsection due date has passed, select the Configure icon for a subsection, then select/i)).toBeInTheDocument();
-    expect(getByText(/Grades for the subsection remain included in grade calculations./i)).toBeInTheDocument();
-
-    expect(getByText(messages.section_4_link.defaultMessage)).toBeInTheDocument();
+      expect(getByText(messages.section_4_link.defaultMessage)).toBeInTheDocument();
+    });
   });
 });
diff --git a/src/course-outline/status-bar/StatusBar.jsx b/src/course-outline/status-bar/StatusBar.jsx
@@ -42,7 +42,7 @@ const StatusBar = ({
   return (
     <Stack direction="horizontal" gap={3.5} className="outline-status-bar" data-testid="outline-status-bar">
       <div className="outline-status-bar__item">
-        <h5 className="h5">{intl.formatMessage(messages.startDateTitle)}</h5>
+        <h5>{intl.formatMessage(messages.startDateTitle)}</h5>
         <Hyperlink
           className="small"
           destination={scheduleDestination}
@@ -52,15 +52,15 @@ const StatusBar = ({
         </Hyperlink>
       </div>
       <div className="outline-status-bar__item">
-        <h5 className="h5">{intl.formatMessage(messages.pacingTypeTitle)}</h5>
+        <h5>{intl.formatMessage(messages.pacingTypeTitle)}</h5>
         <span className="small">
           {isSelfPaced
             ? intl.formatMessage(messages.pacingTypeSelfPaced)
             : intl.formatMessage(messages.pacingTypeInstructorPaced)}
         </span>
       </div>
-      <div className="outline-status-bar__item">
-        <h5 className="h5">{intl.formatMessage(messages.checklistTitle)}</h5>
+      <div className="outline-status-bar__item mr-4">
+        <h5>{intl.formatMessage(messages.checklistTitle)}</h5>
         <Hyperlink
           className="small"
           destination={checklistDestination}
@@ -69,11 +69,13 @@ const StatusBar = ({
           {checkListTitle} {intl.formatMessage(messages.checklistCompleted)}
         </Hyperlink>
       </div>
-      <div className="outline-status-bar__item">
-        <h5 className="h5">{intl.formatMessage(messages.highlightEmailsTitle)}</h5>
+      <div className="outline-status-bar__item ml-4">
+        <h5>{intl.formatMessage(messages.highlightEmailsTitle)}</h5>
         <div className="d-flex align-items-end">
           {highlightsEnabledForMessaging ? (
-            <span className="small">{intl.formatMessage(messages.highlightEmailsEnabled)}</span>
+            <span data-testid="highlights-enabled-span" className="small">
+              {intl.formatMessage(messages.highlightEmailsEnabled)}
+            </span>
           ) : (
             <Button size="sm" onClick={openEnableHighlightsModal}>
               {intl.formatMessage(messages.highlightEmailsButton)}

diff --git a/src/course-outline/status-bar/StatusBar.scss b/src/course-outline/status-bar/StatusBar.scss
@@ -2,7 +2,7 @@
   .outline-status-bar__item {
     display: flex;
     flex-direction: column;
-    justify-content: space-between;
+    justify-content: space-evenly;
     min-height: 3.75rem;
 
     & h5 {