Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .env
Original file line number Diff line number Diff line change
Expand Up @@ -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=''
Expand Down
1 change: 1 addition & 0 deletions .env.development
Original file line number Diff line number Diff line change
Expand Up @@ -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=''
Expand Down
4 changes: 4 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/course-home/outline-tab/LmsHtmlFragment.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
15 changes: 14 additions & 1 deletion src/course-home/outline-tab/OutlineTab.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down Expand Up @@ -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]}
/>
Expand Down
48 changes: 48 additions & 0 deletions src/course-home/outline-tab/OutlineTab.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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"
Expand Down Expand Up @@ -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();
Expand Down
16 changes: 14 additions & 2 deletions src/course-home/outline-tab/Section.jsx
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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,
Expand All @@ -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);

Expand All @@ -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">
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -104,6 +114,8 @@ const Section = ({
courseId={courseId}
sequence={sequences[sequenceId]}
first={index === 0}
last={index === (sequenceIds.length - 1)}
selected={hashValue === sequenceId}
/>
))}
</ol>
Expand Down
15 changes: 15 additions & 0 deletions src/course-home/outline-tab/Section.scss
Original file line number Diff line number Diff line change
@@ -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;
}
22 changes: 20 additions & 2 deletions src/course-home/outline-tab/SequenceLink.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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"
Expand Down Expand Up @@ -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 ? (
Expand Down Expand Up @@ -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);
7 changes: 7 additions & 0 deletions src/course-home/outline-tab/SequenceLink.scss
Original file line number Diff line number Diff line change
@@ -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;
}
21 changes: 21 additions & 0 deletions src/course-home/outline-tab/hooks.js
Original file line number Diff line number Diff line change
@@ -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,
};
22 changes: 18 additions & 4 deletions src/course-home/outline-tab/widgets/WelcomeMessage.jsx
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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();

Expand Down Expand Up @@ -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)}
/>
)}
Expand Down
24 changes: 11 additions & 13 deletions src/courseware/course/CourseBreadcrumbs.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
) : (
Expand Down
Loading