diff --git a/.env b/.env index b162247399..593c30b8c6 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 34e014efb0..ec6c3a6ced 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='' 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/LmsHtmlFragment.jsx b/src/course-home/outline-tab/LmsHtmlFragment.jsx index 191663554c..3c061d0fad 100644 --- a/src/course-home/outline-tab/LmsHtmlFragment.jsx +++ b/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; } } diff --git a/src/course-home/outline-tab/OutlineTab.jsx b/src/course-home/outline-tab/OutlineTab.jsx index a2ce8101ea..c7c46a39fc 100644 --- a/src/course-home/outline-tab/OutlineTab.jsx +++ b/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 ( <>
@@ -173,7 +182,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..0a50c669bf 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" @@ -257,6 +289,22 @@ describe('Outline Tab', () => { }); }); + it('ignores comments and misformatted HTML', async () => { + setTabData({ + welcome_message_html: '

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

', + }); + 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(); 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/course-home/outline-tab/widgets/WelcomeMessage.jsx b/src/course-home/outline-tab/widgets/WelcomeMessage.jsx index ed64b1b547..f7839c5f25 100644 --- a/src/course-home/outline-tab/widgets/WelcomeMessage.jsx +++ b/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)} /> )} diff --git a/src/courseware/course/CourseBreadcrumbs.jsx b/src/courseware/course/CourseBreadcrumbs.jsx index a49906a894..21d343144b 100644 --- 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) { + 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 && ( @@ -32,22 +41,11 @@ const CourseBreadcrumb = ({ )}
  • {showRegularLink ? ( - + {defaultContent.label} ) : ( diff --git a/src/courseware/course/CourseBreadcrumbs.test.jsx b/src/courseware/course/CourseBreadcrumbs.test.jsx index f51ead34c3..d1f557fb22 100644 --- a/src/courseware/course/CourseBreadcrumbs.test.jsx +++ b/src/courseware/course/CourseBreadcrumbs.test.jsx @@ -112,23 +112,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.queryAllByTestId('breadcrumb-item')).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 d99be587a7..6de75a06ab 100755 --- 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, diff --git a/src/index.scss b/src/index.scss index 555c5b8f7e..4395b245f7 100755 --- a/src/index.scss +++ b/src/index.scss @@ -370,6 +370,18 @@ } } +.reactive-crumbs { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + &:hover, + &:focus, + &:focus-within, + &:active { + overflow: visible; + } +} + .raised-card { border: 0 !important; box-shadow: 0 .0625rem .125rem rgba(0, 0, 0, .2) !important;