From 79d9cbc7efed1c535d52abe62ce24a1004e533d6 Mon Sep 17 00:00:00 2001 From: Artur Gaspar Date: Fri, 17 Nov 2023 08:34:30 -0300 Subject: [PATCH] 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. --- .env | 1 + .env.development | 1 + README.rst | 4 ++ src/course-home/outline-tab/OutlineTab.jsx | 15 +++++- .../outline-tab/OutlineTab.test.jsx | 32 +++++++++++++ src/course-home/outline-tab/Section.jsx | 16 ++++++- src/course-home/outline-tab/Section.scss | 15 ++++++ src/course-home/outline-tab/SequenceLink.jsx | 22 ++++++++- src/course-home/outline-tab/SequenceLink.scss | 7 +++ src/course-home/outline-tab/hooks.js | 21 +++++++++ src/courseware/course/CourseBreadcrumbs.jsx | 16 ++++--- .../course/CourseBreadcrumbs.test.jsx | 47 ++++++++++++++----- src/index.jsx | 1 + 13 files changed, 175 insertions(+), 23 deletions(-) create mode 100644 src/course-home/outline-tab/Section.scss create mode 100644 src/course-home/outline-tab/SequenceLink.scss create mode 100644 src/course-home/outline-tab/hooks.js diff --git a/.env b/.env index 62cf2ad666..c8064b4664 100644 --- 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 index 94f8a19332..f0cae5b828 100644 --- 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='http://localhost:18740' diff --git a/README.rst b/README.rst index b1d2f39678..23a55e5408 100644 --- a/README.rst +++ b/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. diff --git a/src/course-home/outline-tab/OutlineTab.jsx b/src/course-home/outline-tab/OutlineTab.jsx index 072f0d04c6..22d9041f5b 100644 --- a/src/course-home/outline-tab/OutlineTab.jsx +++ b/src/course-home/outline-tab/OutlineTab.jsx @@ -121,6 +121,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 ( <>
@@ -171,7 +180,11 @@ const OutlineTab = ({ intl }) => {
diff --git a/src/course-home/outline-tab/OutlineTab.test.jsx b/src/course-home/outline-tab/OutlineTab.test.jsx index c6fc547a0c..dfa8ae9987 100644 --- a/src/course-home/outline-tab/OutlineTab.test.jsx +++ b/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" diff --git a/src/course-home/outline-tab/Section.jsx b/src/course-home/outline-tab/Section.jsx index 3de888a89a..60d83269fd 100644 --- 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'; @@ -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 = (
@@ -72,9 +82,9 @@ const Section = ({ ); return ( -
  • +
  • ))} diff --git a/src/course-home/outline-tab/Section.scss b/src/course-home/outline-tab/Section.scss new file mode 100644 index 0000000000..a8b859db4a --- /dev/null +++ b/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; +} diff --git a/src/course-home/outline-tab/SequenceLink.jsx b/src/course-home/outline-tab/SequenceLink.jsx index 0530d53e66..2e0ef3725c 100644 --- a/src/course-home/outline-tab/SequenceLink.jsx +++ b/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 = {title}; const displayTitle = showLink ? coursewareUrl : title; + const sequenceLinkRef = useScrollTo(selected); + const dueDateMessage = ( -
    +
  • +
    {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); diff --git a/src/course-home/outline-tab/SequenceLink.scss b/src/course-home/outline-tab/SequenceLink.scss new file mode 100644 index 0000000000..bfcf6e950a --- /dev/null +++ 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/course-home/outline-tab/hooks.js b/src/course-home/outline-tab/hooks.js new file mode 100644 index 0000000000..faf986b978 --- /dev/null +++ b/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, +}; diff --git a/src/courseware/course/CourseBreadcrumbs.jsx b/src/courseware/course/CourseBreadcrumbs.jsx index 3eb0023fba..6b4597150e 100644 --- a/src/courseware/course/CourseBreadcrumbs.jsx +++ b/src/courseware/course/CourseBreadcrumbs.jsx @@ -14,6 +14,15 @@ const CourseBreadcrumb = ({ content, withSeparator, courseId, sequenceId, unitId, isStaff, }) => { const defaultContent = content.filter(destination => destination.default)[0] || { id: courseId, label: '', sequences: [] }; + + 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 && ( @@ -23,12 +32,7 @@ const CourseBreadcrumb = ({
  • { getConfig().ENABLE_JUMPNAV !== 'true' || content.length < 2 || !isStaff ? ( - + {defaultContent.label} ) diff --git a/src/courseware/course/CourseBreadcrumbs.test.jsx b/src/courseware/course/CourseBreadcrumbs.test.jsx index 23285f1410..60b70ae617 100644 --- a/src/courseware/course/CourseBreadcrumbs.test.jsx +++ b/src/courseware/course/CourseBreadcrumbs.test.jsx @@ -106,23 +106,46 @@ describe('CourseBreadcrumbs', () => { }, ], ]); - render( - - - - , - , - ); it('renders course breadcrumbs as expected', async () => { + await render( + + + + , + , + ); 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.queryAllByRole('button')).toHaveLength(2); }); + it('renders legacy navigation links as expected', async () => { + getConfig.mockImplementation(() => ({ + ENABLE_JUMPNAV: 'false', + ENABLE_LEGACY_NAV: 'true', + })); + await render( + + + + , + , + ); + 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 index 7fec13b112..fe4dc124d2 100755 --- a/src/index.jsx +++ b/src/index.jsx @@ -123,6 +123,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,