Skip to content

Commit

Permalink
feat: [FC-0056] cover course outline sidebar into plugin slot
Browse files Browse the repository at this point in the history
  • Loading branch information
ihor-romaniuk committed Apr 26, 2024
1 parent b70509f commit 219ecb8
Show file tree
Hide file tree
Showing 23 changed files with 106 additions and 126 deletions.
13 changes: 13 additions & 0 deletions example.env.config.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import UnitTranslationPlugin from '@edx/unit-translation-selector-plugin';
import { PLUGIN_OPERATIONS, DIRECT_PLUGIN } from '@openedx/frontend-plugin-framework';

import {
OUTLINE_SIDEBAR_DESKTOP_PLUGIN_SLOT_ID,
OUTLINE_SIDEBAR_MOBILE_PLUGIN_SLOT_ID,
} from '@src/courseware/course/sidebar/sidebars/course-outline';

// Load environment variables from .env file
const config = {
...process.env,
Expand All @@ -18,6 +23,14 @@ const config = {
},
],
},
[OUTLINE_SIDEBAR_DESKTOP_PLUGIN_SLOT_ID]: {
keepDefault: true,
plugins: [],
},
[OUTLINE_SIDEBAR_MOBILE_PLUGIN_SLOT_ID]: {
keepDefault: true,
plugins: [],
},
},
};

Expand Down
3 changes: 0 additions & 3 deletions src/course-home/data/__snapshots__/redux.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ Object {
"courseId": null,
"courseOutline": Object {},
"courseOutlineShouldUpdate": false,
"courseOutlineSidebarSettings": Object {},
"courseOutlineStatus": "loading",
"courseStatus": "loading",
"rightSidebarSettings": Object {},
Expand Down Expand Up @@ -408,7 +407,6 @@ Object {
"courseId": null,
"courseOutline": Object {},
"courseOutlineShouldUpdate": false,
"courseOutlineSidebarSettings": Object {},
"courseOutlineStatus": "loading",
"courseStatus": "loading",
"rightSidebarSettings": Object {},
Expand Down Expand Up @@ -681,7 +679,6 @@ Object {
"courseId": null,
"courseOutline": Object {},
"courseOutlineShouldUpdate": false,
"courseOutlineSidebarSettings": Object {},
"courseOutlineStatus": "loading",
"courseStatus": "loading",
"rightSidebarSettings": Object {},
Expand Down
14 changes: 0 additions & 14 deletions src/courseware/CoursewareContainer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@ import {
getSequenceForUnitDeprecated,
saveSequencePosition,
} from './data';
import {
fetchOutlineTab,
} from '../course-home/data';
import { TabPage } from '../tab-page';

import Course from './course';
Expand Down Expand Up @@ -142,12 +139,6 @@ class CoursewareContainer extends Component {
this.props.fetchCourse(courseId);
});

checkFetchOutlineData = memoize((courseId) => {
if (courseId) {
this.props.fetchOutlineTab(courseId);
}
});

checkFetchSequence = memoize((sequenceId) => {
if (sequenceId) {
this.props.fetchSequence(sequenceId);
Expand All @@ -156,14 +147,12 @@ class CoursewareContainer extends Component {

componentDidMount() {
const {
courseId,
routeCourseId,
routeSequenceId,
} = this.props;
// Load data whenever the course or sequence ID changes.
this.checkFetchCourse(routeCourseId);
this.checkFetchSequence(routeSequenceId);
this.checkFetchOutlineData(courseId);
}

componentDidUpdate() {
Expand All @@ -185,7 +174,6 @@ class CoursewareContainer extends Component {
// Load data whenever the course or sequence ID changes.
this.checkFetchCourse(routeCourseId);
this.checkFetchSequence(routeSequenceId);
this.checkFetchOutlineData(courseId);

// Check if we should save our sequence position. Only do this when the route unit ID changes.
this.checkSaveSequencePosition(routeUnitId);
Expand Down Expand Up @@ -345,7 +333,6 @@ CoursewareContainer.propTypes = {
checkBlockCompletion: PropTypes.func.isRequired,
fetchCourse: PropTypes.func.isRequired,
fetchSequence: PropTypes.func.isRequired,
fetchOutlineTab: PropTypes.func.isRequired,
navigate: PropTypes.func.isRequired,
};

Expand Down Expand Up @@ -468,5 +455,4 @@ export default connect(mapStateToProps, {
saveSequencePosition,
fetchCourse,
fetchSequence,
fetchOutlineTab,
})(withParamsAndNavigation(CoursewareContainer));
21 changes: 13 additions & 8 deletions src/courseware/course/Course.jsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
import { useEffect, useState } from 'react';
import PropTypes from 'prop-types';
import { Helmet } from 'react-helmet';
import { useDispatch, useSelector } from 'react-redux';
import { useDispatch } from 'react-redux';
import { getConfig } from '@edx/frontend-platform';
import { breakpoints, useWindowSize } from '@openedx/paragon';
import { PluginSlot } from '@openedx/frontend-plugin-framework';

import { AlertList } from '../../generic/user-messages';
import { useModel } from '../../generic/model-store';
import { getCoursewareOutlineSidebarSettings } from '../data/selectors';
import { Trigger as CourseOutlineTrigger } from './sidebar/sidebars/course-outline';
import { AlertList } from '@src/generic/user-messages';
import { useModel } from '@src/generic/model-store';
import {
OUTLINE_SIDEBAR_DESKTOP_PLUGIN_SLOT_ID,
Trigger as CourseOutlineTrigger,
checkIsOutlineSidebarAvailable,
} from './sidebar/sidebars/course-outline';
import Chat from './chat/Chat';
import SidebarProvider from './sidebar/SidebarContextProvider';
import SidebarTriggers from './sidebar/SidebarTriggers';
Expand Down Expand Up @@ -36,8 +40,7 @@ const Course = ({
const sequence = useModel('sequences', sequenceId);
const section = useModel('sections', sequence ? sequence.sectionId : null);
const enableNewSidebar = getConfig().ENABLE_NEW_SIDEBAR;
const { enabled: isEnabledOutlineSidebar = false } = useSelector(getCoursewareOutlineSidebarSettings);
const navigationDisabled = isEnabledOutlineSidebar || (sequence?.navigationDisabled ?? false);
const navigationDisabled = checkIsOutlineSidebarAvailable() || (sequence?.navigationDisabled ?? false);

const pageTitleBreadCrumbs = [
sequence,
Expand Down Expand Up @@ -100,7 +103,9 @@ const Course = ({
</>
)}
<div className="w-100 d-flex align-items-center">
<CourseOutlineTrigger isMobileView />
<PluginSlot id={OUTLINE_SIDEBAR_DESKTOP_PLUGIN_SLOT_ID}>
<CourseOutlineTrigger isMobileView />
</PluginSlot>
{enableNewSidebar === 'true' ? <NewSidebarTriggers /> : <SidebarTriggers /> }
</div>
</div>
Expand Down
6 changes: 2 additions & 4 deletions src/courseware/course/Course.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('Course', () => {

it('loads learning sequence', async () => {
render(<Course {...mockData} />, { wrapWithRouter: true });
expect(screen.queryByRole('navigation', { name: 'breadcrumb' })).not.toBeInTheDocument();
expect(screen.queryByRole('navigation', { name: 'breadcrumb' })).toBeInTheDocument();
expect(await screen.findByText('Loading learning sequence...')).toBeInTheDocument();

expect(screen.queryByRole('alert')).not.toBeInTheDocument();
Expand Down Expand Up @@ -210,9 +210,7 @@ describe('Course', () => {
{ type: 'vertical' },
{ courseId: courseMetadata.id },
));
const testStore = await initializeTestStore({
courseMetadata, unitBlocks, outlineSidebarSettings: { enabled: false },
}, false);
const testStore = await initializeTestStore({ courseMetadata, unitBlocks }, false);
const { courseware, models } = testStore.getState();
const { courseId, sequenceId } = courseware;
const testData = {
Expand Down
13 changes: 9 additions & 4 deletions src/courseware/course/sequence/Sequence.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,18 @@ import { useSelector } from 'react-redux';
import SequenceExamWrapper from '@edx/frontend-lib-special-exams';
import { PluginSlot } from '@openedx/frontend-plugin-framework';

import { useModel } from '@src/generic/model-store';
import { useSequenceBannerTextAlert, useSequenceEntranceExamAlert } from '@src/alerts/sequence-alerts/hooks';
import PageLoading from '../../../generic/PageLoading';
import { useModel } from '../../../generic/model-store';
import { useSequenceBannerTextAlert, useSequenceEntranceExamAlert } from '../../../alerts/sequence-alerts/hooks';

import CourseLicense from '../course-license';
import Sidebar from '../sidebar/Sidebar';
import NewSidebar from '../new-sidebar/Sidebar';
import { LAYOUT_RIGHT, LAYOUT_LEFT } from '../sidebar/common/constants';
import { Trigger as CourseOutlineTrigger } from '../sidebar/sidebars/course-outline';
import {
OUTLINE_SIDEBAR_MOBILE_PLUGIN_SLOT_ID,
Trigger as CourseOutlineTrigger,
} from '../sidebar/sidebars/course-outline';
import messages from './messages';
import HiddenAfterDue from './hidden-after-due';
import { SequenceNavigation, UnitNavigation } from './sequence-navigation';
Expand Down Expand Up @@ -146,7 +149,9 @@ const Sequence = ({
const defaultContent = (
<>
<div className="sequence-container d-inline-flex flex-row w-100">
<CourseOutlineTrigger />
<PluginSlot id={OUTLINE_SIDEBAR_MOBILE_PLUGIN_SLOT_ID}>
<CourseOutlineTrigger />
</PluginSlot>
<Sidebar layout={LAYOUT_LEFT} />
<div className="sequence w-100">
<div className="sequence-navigation-container">
Expand Down
8 changes: 4 additions & 4 deletions src/courseware/course/sequence/Sequence.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ describe('Sequence', () => {
);

await waitFor(() => expect(screen.queryByText('Loading locked content messaging...')).toBeInTheDocument());
// `Previous`, `Prerequisite` and `Close Tray` buttons.
expect(screen.getAllByRole('button').length).toEqual(3);
// `Previous` and `Bookmark` buttons.
expect(screen.getAllByRole('button').length).toEqual(2);
// `Active` and `Next` buttons.
expect(screen.getAllByRole('link').length).toEqual(2);

Expand Down Expand Up @@ -136,8 +136,8 @@ describe('Sequence', () => {
it('handles loading unit', async () => {
render(<Sequence {...mockData} />, { wrapWithRouter: true });
expect(await screen.findByText('Loading learning sequence...')).toBeInTheDocument();
// `Previous`, `Prerequisite` and `Close Tray` buttons.
expect(screen.getAllByRole('button')).toHaveLength(3);
// `Previous` and `Bookmark` buttons.
expect(screen.getAllByRole('button')).toHaveLength(2);
// Renders `Next` button plus one button for each unit.
expect(screen.getAllByRole('link')).toHaveLength(1 + unitBlocks.length);

Expand Down
2 changes: 1 addition & 1 deletion src/courseware/course/sidebar/Sidebar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const Sidebar = ({ layout }) => {
return null;
}

if (layout !== SIDEBARS[currentSidebar].LAYOUT) {
if (layout !== SIDEBARS[currentSidebar]?.LAYOUT) {
return null;
}

Expand Down
11 changes: 8 additions & 3 deletions src/courseware/course/sidebar/SidebarContextProvider.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ import { useModel } from '@src/generic/model-store';
import { getLocalStorage, setLocalStorage } from '@src/data/localStorage';
import { getRightSidebarSettings } from '../../data/selectors';

import * as courseOutlineSidebar from './sidebars/course-outline';
import {
ID as courseOutlineSidebarId,
OUTLINE_SIDEBAR_SESSION_STORAGE_NAME,
checkIsOutlineSidebarAvailable,
} from './sidebars/course-outline';
import * as discussionsSidebar from './sidebars/discussions';
import * as notificationsSidebar from './sidebars/notifications';
import SidebarContext from './SidebarContext';
Expand All @@ -27,7 +31,7 @@ const SidebarProvider = ({
const shouldDisplaySidebarOpen = useWindowSize().width > breakpoints.extraLarge.minWidth;
const query = new URLSearchParams(window.location.search);
const isDefaultDisplayRightSidebar = useSelector(getRightSidebarSettings).enabled;
const isCollapsedOutlineSidebar = window.sessionStorage.getItem('hideCourseOutlineSidebar');
const isCollapsedOutlineSidebar = window.sessionStorage.getItem(OUTLINE_SIDEBAR_SESSION_STORAGE_NAME);
const isInitiallySidebarOpen = shouldDisplaySidebarOpen || query.get('sidebar') === 'true';

let initialSidebar = null;
Expand All @@ -37,7 +41,8 @@ const SidebarProvider = ({
? SIDEBARS[discussionsSidebar.ID].ID
: verifiedMode && SIDEBARS[notificationsSidebar.ID].ID;
} else {

Check warning on line 43 in src/courseware/course/sidebar/SidebarContextProvider.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/sidebar/SidebarContextProvider.jsx#L43

Added line #L43 was not covered by tests
initialSidebar = !isCollapsedOutlineSidebar && SIDEBARS[courseOutlineSidebar.ID].ID;
initialSidebar = checkIsOutlineSidebarAvailable()
&& !isCollapsedOutlineSidebar && SIDEBARS[courseOutlineSidebarId].ID;

Check warning on line 45 in src/courseware/course/sidebar/SidebarContextProvider.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/sidebar/SidebarContextProvider.jsx#L45

Added line #L45 was not covered by tests
}
}
const [currentSidebar, setCurrentSidebar] = useState(initialSidebar);
Expand Down
2 changes: 1 addition & 1 deletion src/courseware/course/sidebar/SidebarTriggers.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const SidebarTriggers = () => {
return (
<div
className={classNames({ 'ml-1': !isMobileView, 'border-primary-700 sidebar-active': isActive })}
style={{ borderBottom: isActive ? '2px solid' : null }}
style={{ borderBottom: isActive ? '2px solid' : '2px solid transparent' }}
key={sidebarId}
>
<Trigger onClick={() => toggleSidebar(sidebarId)} key={sidebarId} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,21 @@ import {
ChevronLeft as ChevronLeftIcon,
} from '@openedx/paragon/icons';

import { useModel } from '../../../../../generic/model-store';
import { LOADING, LOADED } from '../../../../../course-home/data/slice';
import PageLoading from '../../../../../generic/PageLoading';
import { useModel } from '@src/generic/model-store';
import PageLoading from '@src/generic/PageLoading';
import { LOADING, LOADED } from '@src/course-home/data/slice';
import {
getSequenceId,
getCourseOutline,
getCourseOutlineStatus,
getCoursewareOutlineSidebarSettings,
getCourseOutlineShouldUpdate,
} from '../../../../data/selectors';
import { getCourseOutlineStructure } from '../../../../data/thunks';
import SidebarContext from '../../SidebarContext';
import { ID } from './CourseOutlineTrigger';
import SidebarSection from './SidebarSection';
import SidebarSequence from './SidebarSequence';
import { OUTLINE_SIDEBAR_SESSION_STORAGE_NAME } from './constants';
import messages from './messages';

const CourseOutlineTray = ({ intl }) => {
Expand All @@ -34,7 +34,6 @@ const CourseOutlineTray = ({ intl }) => {
const { sections = {}, sequences = {} } = useSelector(getCourseOutline);
const courseOutlineStatus = useSelector(getCourseOutlineStatus);
const courseOutlineShouldUpdate = useSelector(getCourseOutlineShouldUpdate);
const { enabled: isEnabled } = useSelector(getCoursewareOutlineSidebarSettings);

const {
courseId,
Expand All @@ -61,10 +60,10 @@ const CourseOutlineTray = ({ intl }) => {
const handleToggleCollapse = () => {
if (currentSidebar === ID) {
toggleSidebar(null);
window.sessionStorage.setItem('hideCourseOutlineSidebar', 'true');
window.sessionStorage.setItem(OUTLINE_SIDEBAR_SESSION_STORAGE_NAME, 'true');
} else {
toggleSidebar(ID);
window.sessionStorage.removeItem('hideCourseOutlineSidebar');
window.sessionStorage.removeItem(OUTLINE_SIDEBAR_SESSION_STORAGE_NAME);

Check warning on line 66 in src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTray.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTray.jsx#L64-L66

Added lines #L64 - L66 were not covered by tests
}
};

Expand Down Expand Up @@ -104,12 +103,12 @@ const CourseOutlineTray = ({ intl }) => {
);

useEffect(() => {
if ((isEnabled && courseOutlineStatus !== LOADED) || courseOutlineShouldUpdate) {
if ((courseOutlineStatus !== LOADED) || courseOutlineShouldUpdate) {
dispatch(getCourseOutlineStructure(courseId));
}
}, [courseId, isEnabled, courseOutlineShouldUpdate]);
}, [courseId, courseOutlineShouldUpdate]);

if (!isEnabled || isActiveEntranceExam) {
if (isActiveEntranceExam) {
return null;

Check warning on line 112 in src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTray.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTray.jsx#L112

Added line #L112 was not covered by tests
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,6 @@ describe('<CourseOutlineTray />', () => {
expect(screen.queryByRole('button', { name: 'Course Outline' })).not.toBeInTheDocument();
});

it('doesn\'t render when outline sidebar is disabled', async () => {
await initTestStore({ outlineSidebarSettings: { enabled: false } });
renderWithProvider();

await expect(screen.queryByText(messages.loading.defaultMessage)).not.toBeInTheDocument();
expect(screen.queryByRole('button', { name: section.title })).not.toBeInTheDocument();
expect(screen.queryByRole('button', { name: messages.toggleCourseOutlineTrigger.defaultMessage })).not.toBeInTheDocument();
});

it('renders correctly when course outline is loaded', async () => {
await initTestStore();
renderWithProvider();
Expand Down
Loading

0 comments on commit 219ecb8

Please sign in to comment.