diff --git a/src/courseware/CoursewareContainer.test.jsx b/src/courseware/CoursewareContainer.test.jsx index 1343473b3b..3accecd21e 100644 --- a/src/courseware/CoursewareContainer.test.jsx +++ b/src/courseware/CoursewareContainer.test.jsx @@ -1,7 +1,7 @@ import { getConfig, history } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { AppProvider } from '@edx/frontend-platform/react'; -import { waitForElementToBeRemoved, fireEvent } from '@testing-library/dom'; +import { waitForElementToBeRemoved } from '@testing-library/dom'; import '@testing-library/jest-dom'; import { render, screen } from '@testing-library/react'; import React from 'react'; @@ -193,15 +193,13 @@ describe('CoursewareContainer', () => { expect(courseHeader.querySelector('.course-title')).toHaveTextContent(courseHomeMetadata.title); } - function assertSequenceNavigation(container, expectedUnitCount = 3) { - // Ensure we had appropriate sequence navigation buttons. We should only have one unit. + function assertNoSequenceNavigation(container) { const sequenceNavButtons = container.querySelectorAll('nav.sequence-navigation a, nav.sequence-navigation button'); - expect(sequenceNavButtons).toHaveLength(expectedUnitCount + 2); + expect(sequenceNavButtons).toHaveLength(0); - expect(sequenceNavButtons[0]).toHaveTextContent('Previous'); - // Prove this button is rendering an SVG tasks icon, meaning it's a unit/vertical. - expect(sequenceNavButtons[1].querySelector('svg')).toHaveClass('fa-tasks'); - expect(sequenceNavButtons[sequenceNavButtons.length - 1]).toHaveTextContent('Next'); + expect(container.querySelector('button, a')).not.toHaveTextContent('Previous'); + expect(container.querySelector('svg.fa-tasks')).toBeNull(); + expect(container.querySelector('button, a')).not.toHaveTextContent('Next'); } beforeEach(async () => { @@ -224,7 +222,7 @@ describe('CoursewareContainer', () => { const container = await loadContainer(); assertLoadedHeader(container); - assertSequenceNavigation(container); + assertNoSequenceNavigation(container); expect(container.querySelector('.fake-unit')).toHaveTextContent('Unit Contents'); expect(container.querySelector('.fake-unit')).toHaveTextContent(courseId); @@ -247,7 +245,7 @@ describe('CoursewareContainer', () => { const container = await loadContainer(); assertLoadedHeader(container); - assertSequenceNavigation(container); + assertNoSequenceNavigation(container); expect(container.querySelector('.fake-unit')).toHaveTextContent('Unit Contents'); expect(container.querySelector('.fake-unit')).toHaveTextContent(courseId); @@ -274,29 +272,12 @@ describe('CoursewareContainer', () => { setUpMockRequests({ courseBlocks }); }); - // describe('when the URL contains a unit ID', () => { - // it('should ignore the section ID and redirect based on the unit ID', async () => { - // const urlUnit = unitTree[1][1][1]; - // setUrl(sectionTree[1].id, urlUnit.id); - // const container = await loadContainer(); - // assertLoadedHeader(container); - // assertSequenceNavigation(container, 2); - // assertLocation(container, sequenceTree[1][1].id, urlUnit.id); - // }); - - // it('should ignore invalid unit IDs and redirect to the course root', async () => { - // setUrl(sectionTree[1].id, 'foobar'); - // await loadContainer(); - // expect(global.location.href).toEqual(`http://localhost/course/${courseId}`); - // }); - // }); - describe('when the URL does not contain a unit ID', () => { it('should choose a unit within the section\'s first sequence', async () => { setUrl(sectionTree[1].id); const container = await loadContainer(); assertLoadedHeader(container); - assertSequenceNavigation(container, 2); + assertNoSequenceNavigation(container); assertLocation(container, sequenceTree[1][0].id, unitTree[1][0][0].id); }); }); @@ -342,27 +323,6 @@ describe('CoursewareContainer', () => { }); }); - // describe('when the URL only contains a unit ID', () => { - // const { courseBlocks, unitTree, sequenceTree } = buildBinaryCourseBlocks(courseId, courseMetadata.name); - - // beforeEach(async () => { - // setUpMockRequests({ courseBlocks }); - // }); - - // it('should insert the sequence ID into the URL', async () => { - // const unit = unitTree[1][0][1]; - // history.push(`/course/${courseId}/${unit.id}`); - // const container = await loadContainer(); - - // assertLoadedHeader(container); - // assertSequenceNavigation(container, 2); - // const expectedSequenceId = sequenceTree[1][0].id; - // const expectedUrl = `http://localhost/course/${courseId}/${expectedSequenceId}/${unit.id}`; - // expect(global.location.href).toEqual(expectedUrl); - // expect(container.querySelector('.fake-unit')).toHaveTextContent(unit.id); - // }); - // }); - describe('when the URL contains a course ID and sequence ID', () => { const sequenceBlock = defaultSequenceBlock; const unitBlocks = defaultUnitBlocks; @@ -372,7 +332,7 @@ describe('CoursewareContainer', () => { const container = await loadContainer(); assertLoadedHeader(container); - assertSequenceNavigation(container); + assertNoSequenceNavigation(container); expect(container.querySelector('.fake-unit')).toHaveTextContent('Unit Contents'); expect(container.querySelector('.fake-unit')).toHaveTextContent(courseId); @@ -391,7 +351,7 @@ describe('CoursewareContainer', () => { const container = await loadContainer(); assertLoadedHeader(container); - assertSequenceNavigation(container); + assertNoSequenceNavigation(container); expect(container.querySelector('.fake-unit')).toHaveTextContent('Unit Contents'); expect(container.querySelector('.fake-unit')).toHaveTextContent(courseId); @@ -408,44 +368,24 @@ describe('CoursewareContainer', () => { const container = await loadContainer(); assertLoadedHeader(container); - assertSequenceNavigation(container); + assertNoSequenceNavigation(container); expect(container.querySelector('.fake-unit')).toHaveTextContent('Unit Contents'); expect(container.querySelector('.fake-unit')).toHaveTextContent(courseId); expect(container.querySelector('.fake-unit')).toHaveTextContent(unitBlocks[2].id); }); - it('should navigate between units and check block completion', async () => { - axiosMock.onPost(`${courseId}/xblock/${sequenceBlock.id}/handler/get_completion`).reply(200, { - complete: true, - }); + it('should render the sequence_navigation plugin slot correctly', async () => { + axiosMock + .onPost(`${courseId}/xblock/${sequenceBlock.id}/handler/get_completion`) + .reply(200, { complete: true }); history.push(`/course/${courseId}/${sequenceBlock.id}/${unitBlocks[0].id}`); - const container = await loadContainer(); - - const sequenceNavButtons = container.querySelectorAll('nav.sequence-navigation a, nav.sequence-navigation button'); - const sequenceNextButton = sequenceNavButtons[4]; - expect(sequenceNextButton).toHaveTextContent('Next'); - fireEvent.click(sequenceNextButton); + await loadContainer(); - expect(global.location.href).toEqual(`http://localhost/course/${courseId}/${sequenceBlock.id}/${unitBlocks[1].id}`); + expect(screen.getByTestId('org.openedx.frontend.learning.sequence_navigation.v1')).toBeInTheDocument(); }); }); - - // describe('when the current sequence is an exam', () => { - // const { location } = window; - - // beforeEach(() => { - // delete window.location; - // window.location = { - // assign: jest.fn(), - // }; - // }); - - // afterEach(() => { - // window.location = location; - // }); - // }); }); describe('when receiving a course_access error_code', () => { diff --git a/src/courseware/course/Course.jsx b/src/courseware/course/Course.jsx index 919f063b9f..e035f529a6 100644 --- a/src/courseware/course/Course.jsx +++ b/src/courseware/course/Course.jsx @@ -1,14 +1,13 @@ 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 { useLocation, useNavigate } from 'react-router-dom'; import { breakpoints, useWindowSize } from '@openedx/paragon'; import { AlertList } from '@src/generic/user-messages'; import { useModel } from '@src/generic/model-store'; -import { getCoursewareOutlineSidebarSettings } from '../data/selectors'; import Chat from './chat/Chat'; import SidebarProvider from './sidebar/SidebarContextProvider'; import NewSidebarProvider from './new-sidebar/SidebarContextProvider'; @@ -37,8 +36,6 @@ const Course = ({ } = useModel('courseHomeMeta', courseId); const sequence = useModel('sequences', sequenceId); const section = useModel('sections', sequence ? sequence.sectionId : null); - const { enableNavigationSidebar } = useSelector(getCoursewareOutlineSidebarSettings); - const navigationDisabled = enableNavigationSidebar || (sequence?.navigationDisabled ?? false); const navigate = useNavigate(); const { pathname } = useLocation(); @@ -84,17 +81,13 @@ const Course = ({ {`${pageTitleBreadCrumbs.join(' | ')} | ${getConfig().SITE_NAME}`}
- {navigationDisabled || ( - <> - - - )} + {shouldDisplayChat && ( <> { }); }); - it('renders course breadcrumbs as expected', async () => { + it('doesn\'t renders course breadcrumbs by default', async () => { const courseMetadata = Factory.build('courseMetadata'); const unitBlocks = Array.from({ length: 3 }).map(() => Factory.build( 'block', @@ -210,7 +210,7 @@ describe('Course', () => { { courseId: courseMetadata.id }, )); const testStore = await initializeTestStore({ - courseMetadata, unitBlocks, enableNavigationSidebar: { enable_navigation_sidebar: false }, + courseMetadata, unitBlocks, }, false); const { courseware, models } = testStore.getState(); const { courseId, sequenceId } = courseware; @@ -226,10 +226,10 @@ describe('Course', () => { await waitFor(() => { expect(screen.queryByText('Loading learning sequence...')).not.toBeInTheDocument(); }); - // expect the section and sequence "titles" to be loaded in as breadcrumb labels. - waitFor(() => { - expect(screen.findByText(Object.values(models.sections)[0].title)).toBeInTheDocument(); - expect(screen.findByText(Object.values(models.sequences)[0].title)).toBeInTheDocument(); + // expect the section and sequence "titles" not to be loaded in as breadcrumb labels. + await waitFor(() => { + expect(screen.queryByText(Object.values(models.sections)[0].title)).not.toBeInTheDocument(); + expect(screen.queryByText(Object.values(models.sequences)[0].title)).not.toBeInTheDocument(); }); }); diff --git a/src/courseware/course/sequence/Sequence.jsx b/src/courseware/course/sequence/Sequence.jsx index e0b3c0a82c..a35c2b4335 100644 --- a/src/courseware/course/sequence/Sequence.jsx +++ b/src/courseware/course/sequence/Sequence.jsx @@ -19,7 +19,6 @@ import { CourseOutlineSidebarTriggerSlot } from '@src/plugin-slots/CourseOutline import { NotificationsDiscussionsSidebarSlot } from '@src/plugin-slots/NotificationsDiscussionsSidebarSlot'; import SequenceNavigationSlot from '@src/plugin-slots/SequenceNavigationSlot'; -import { getCoursewareOutlineSidebarSettings } from '../../data/selectors'; import CourseLicense from '../course-license'; import messages from './messages'; import HiddenAfterDue from './hidden-after-due'; @@ -48,7 +47,7 @@ const Sequence = ({ const unit = useModel('units', unitId); const sequenceStatus = useSelector(state => state.courseware.sequenceStatus); const sequenceMightBeUnit = useSelector(state => state.courseware.sequenceMightBeUnit); - const { enableNavigationSidebar: isEnabledOutlineSidebar } = useSelector(getCoursewareOutlineSidebarSettings); + const handleNext = () => { const nextIndex = sequence.unitIds.indexOf(unitId) + 1; const newUnitId = sequence.unitIds[nextIndex]; @@ -91,6 +90,30 @@ const Sequence = ({ sendTrackingLogEvent(eventName, payload); }; + /* istanbul ignore next */ + const nextHandler = () => { + logEvent('edx.ui.lms.sequence.next_selected', 'top'); + handleNext(); + }; + + /* istanbul ignore next */ + const previousHandler = () => { + logEvent('edx.ui.lms.sequence.previous_selected', 'top'); + handlePrevious(); + }; + + /* istanbul ignore next */ + const onNavigate = (destinationUnitId) => { + logEvent('edx.ui.lms.sequence.tab_selected', 'top', destinationUnitId); + handleNavigate(destinationUnitId); + }; + + const sequenceNavProps = { + nextHandler, + previousHandler, + onNavigate, + }; + useSequenceBannerTextAlert(sequenceId); useSequenceEntranceExamAlert(courseId, sequenceId, intl); @@ -171,30 +194,25 @@ const Sequence = ({ />
- {!isEnabledOutlineSidebar && ( -
- { - logEvent('edx.ui.lms.sequence.next_selected', 'top'); - handleNext(); - }} - onNavigate={(destinationUnitId) => { - logEvent('edx.ui.lms.sequence.tab_selected', 'top', destinationUnitId); - handleNavigate(destinationUnitId); - }} - previousHandler={() => { - logEvent('edx.ui.lms.sequence.previous_selected', 'top'); - handlePrevious(); - }} - {...{ - nextSequenceHandler, - handleNavigate, - }} - /> -
- )} +
+ {/** + SequenceNavigationSlot renders nothing by default. + However, we still pass nextHandler, previousHandler, and onNavigate, + because, as per the slot's contract, if this slot is replaced + with the default SequenceNavigation component, these props are required. + These handlers are excluded from test coverage via istanbul ignore, + since they are not used unless the slot is overridden. + */} + +
{unitHasLoaded && renderUnitNavigation(false)} diff --git a/src/courseware/course/sequence/Sequence.test.jsx b/src/courseware/course/sequence/Sequence.test.jsx index 5473e24c30..ae58bb18fc 100644 --- a/src/courseware/course/sequence/Sequence.test.jsx +++ b/src/courseware/course/sequence/Sequence.test.jsx @@ -24,7 +24,6 @@ describe('Sequence', () => { { type: 'vertical' }, { courseId: courseMetadata.id }, )); - const enableNavigationSidebar = { enable_navigation_sidebar: false }; beforeAll(async () => { const store = await initializeTestStore({ courseMetadata, unitBlocks }); @@ -96,7 +95,6 @@ describe('Sequence', () => { unitBlocks, sequenceBlocks, sequenceMetadata, - enableNavigationSidebar: { enable_navigation_sidebar: true }, }, false); const { container } = render( , @@ -131,7 +129,7 @@ describe('Sequence', () => { { courseId: courseMetadata.id, unitBlocks, sequenceBlock: sequenceBlocks[0] }, )]; const testStore = await initializeTestStore({ - courseMetadata, unitBlocks, sequenceBlocks, sequenceMetadata, enableNavigationSidebar, + courseMetadata, unitBlocks, sequenceBlocks, sequenceMetadata, }, false); render( , @@ -190,7 +188,7 @@ describe('Sequence', () => { beforeAll(async () => { testStore = await initializeTestStore({ - courseMetadata, unitBlocks, sequenceBlocks, enableNavigationSidebar, + courseMetadata, unitBlocks, sequenceBlocks, }, false); }); @@ -366,7 +364,6 @@ describe('Sequence', () => { unitBlocks, sequenceBlocks: testSequenceBlocks, sequenceMetadata: testSequenceMetadata, - enableNavigationSidebar, }, false); const testData = { ...mockData, diff --git a/src/courseware/course/sequence/SequenceContent.jsx b/src/courseware/course/sequence/SequenceContent.jsx index 905ffbf255..6caa0fce27 100644 --- a/src/courseware/course/sequence/SequenceContent.jsx +++ b/src/courseware/course/sequence/SequenceContent.jsx @@ -16,7 +16,6 @@ const SequenceContent = ({ unitId, unitLoadedHandler, isOriginalUserStaff, - isEnabledOutlineSidebar, renderUnitNavigation, }) => { const intl = useIntl(); @@ -63,7 +62,6 @@ const SequenceContent = ({ id={unitId} onLoaded={unitLoadedHandler} isOriginalUserStaff={isOriginalUserStaff} - isEnabledOutlineSidebar={isEnabledOutlineSidebar} renderUnitNavigation={renderUnitNavigation} /> ); @@ -76,7 +74,6 @@ SequenceContent.propTypes = { unitId: PropTypes.string, unitLoadedHandler: PropTypes.func.isRequired, isOriginalUserStaff: PropTypes.bool.isRequired, - isEnabledOutlineSidebar: PropTypes.bool.isRequired, renderUnitNavigation: PropTypes.func.isRequired, }; diff --git a/src/courseware/course/sequence/SequenceContent.test.jsx b/src/courseware/course/sequence/SequenceContent.test.jsx index a2f14490d3..e9c3a2d785 100644 --- a/src/courseware/course/sequence/SequenceContent.test.jsx +++ b/src/courseware/course/sequence/SequenceContent.test.jsx @@ -15,6 +15,7 @@ describe('Sequence Content', () => { sequenceId: courseware.sequenceId, unitId: models.sequences[courseware.sequenceId].unitIds[0], unitLoadedHandler: () => { }, + renderUnitNavigation: () => { }, }; }); @@ -38,7 +39,7 @@ describe('Sequence Content', () => { }); it('displays message for no content', () => { - render(, { wrapWithRouter: true }); + render(, { wrapWithRouter: true }); expect(screen.getByText('There is no content here.')).toBeInTheDocument(); }); }); diff --git a/src/courseware/course/sequence/Unit/index.jsx b/src/courseware/course/sequence/Unit/index.jsx index 37eb396d88..3a9c71bc94 100644 --- a/src/courseware/course/sequence/Unit/index.jsx +++ b/src/courseware/course/sequence/Unit/index.jsx @@ -22,7 +22,6 @@ const Unit = ({ onLoaded, id, isOriginalUserStaff, - isEnabledOutlineSidebar, renderUnitNavigation, }) => { const { formatMessage } = useIntl(); @@ -48,7 +47,7 @@ const Unit = ({ return (
- + enabled && 'UnitNaviagtion'), }; @@ -68,16 +67,8 @@ describe('', () => { expect(screen.getByText('Bookmark this page')).toBeInTheDocument(); }); - it('does not render unit navigation buttons', () => { - renderComponent(defaultProps); - - const nextButton = screen.queryByText('UnitNaviagtion'); - - expect(nextButton).toBeNull(); - }); - - it('renders unit navigation buttons when isEnabledOutlineSidebar is true', () => { - const props = { ...defaultProps, isEnabledOutlineSidebar: true }; + it('renders unit navigation buttons', () => { + const props = { ...defaultProps }; renderComponent(props); const nextButton = screen.getByText('UnitNaviagtion'); diff --git a/src/courseware/course/sequence/sequence-navigation/SequenceNavigation.jsx b/src/courseware/course/sequence/sequence-navigation/SequenceNavigation.jsx index 22f61c478a..81cd5a6bc7 100644 --- a/src/courseware/course/sequence/sequence-navigation/SequenceNavigation.jsx +++ b/src/courseware/course/sequence/sequence-navigation/SequenceNavigation.jsx @@ -100,13 +100,13 @@ const SequenceNavigation = ({ ); }; - return sequenceStatus === LOADED && ( + return sequenceStatus === LOADED ? ( - ); + ) : null; }; SequenceNavigation.propTypes = { diff --git a/src/courseware/course/sidebar/SidebarContextProvider.jsx b/src/courseware/course/sidebar/SidebarContextProvider.jsx index 05472c01dd..9b3b824d53 100644 --- a/src/courseware/course/sidebar/SidebarContextProvider.jsx +++ b/src/courseware/course/sidebar/SidebarContextProvider.jsx @@ -1,13 +1,11 @@ import { breakpoints, useWindowSize } from '@openedx/paragon'; import PropTypes from 'prop-types'; -import { useSelector } from 'react-redux'; import { useEffect, useState, useMemo, useCallback, } from 'react'; import { useModel } from '@src/generic/model-store'; import { getLocalStorage, setLocalStorage } from '@src/data/localStorage'; -import { getCoursewareOutlineSidebarSettings } from '../../data/selectors'; import * as discussionsSidebar from './sidebars/discussions'; import * as notificationsSidebar from './sidebars/notifications'; @@ -25,11 +23,10 @@ const SidebarProvider = ({ const shouldDisplayFullScreen = useWindowSize().width < breakpoints.extraLarge.minWidth; const shouldDisplaySidebarOpen = useWindowSize().width > breakpoints.extraLarge.minWidth; const query = new URLSearchParams(window.location.search); - const { alwaysOpenAuxiliarySidebar } = useSelector(getCoursewareOutlineSidebarSettings); const isInitiallySidebarOpen = shouldDisplaySidebarOpen || query.get('sidebar') === 'true'; let initialSidebar = shouldDisplayFullScreen ? getLocalStorage(`sidebar.${courseId}`) : null; - if (!shouldDisplayFullScreen && isInitiallySidebarOpen && alwaysOpenAuxiliarySidebar) { + if (!shouldDisplayFullScreen && isInitiallySidebarOpen) { initialSidebar = isUnitHasDiscussionTopics ? SIDEBARS[discussionsSidebar.ID].ID : verifiedMode && SIDEBARS[notificationsSidebar.ID].ID; diff --git a/src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTray.jsx b/src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTray.jsx index 8cd17712f5..8eaf5e8a0b 100644 --- a/src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTray.jsx +++ b/src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTray.jsx @@ -23,7 +23,6 @@ const CourseOutlineTray = () => { const { courseId, unitId, - isEnabledSidebar, currentSidebar, handleToggleCollapse, isActiveEntranceExam, @@ -77,7 +76,7 @@ const CourseOutlineTray = () => {
); - if (!isEnabledSidebar || isActiveEntranceExam || currentSidebar !== ID) { + if (isActiveEntranceExam || currentSidebar !== ID) { return null; } diff --git a/src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTray.test.jsx b/src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTray.test.jsx index 1f826a690a..aba41c8291 100644 --- a/src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTray.test.jsx +++ b/src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTray.test.jsx @@ -67,15 +67,6 @@ describe('', () => { expect(screen.queryByRole('button', { name: 'Course outline' })).not.toBeInTheDocument(); }); - it('doesn\'t render when outline sidebar is disabled', async () => { - await initTestStore({ enableNavigationSidebar: { enable_navigation_sidebar: 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(); diff --git a/src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTrigger.jsx b/src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTrigger.jsx index dfc698de3d..abccd14aed 100644 --- a/src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTrigger.jsx +++ b/src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTrigger.jsx @@ -15,13 +15,12 @@ const CourseOutlineTrigger = ({ isMobileView }) => { shouldDisplayFullScreen, handleToggleCollapse, isActiveEntranceExam, - isEnabledSidebar, } = useCourseOutlineSidebar(); const isDisplayForDesktopView = !isMobileView && !shouldDisplayFullScreen && currentSidebar !== ID; const isDisplayForMobileView = isMobileView && shouldDisplayFullScreen; - if ((!isDisplayForDesktopView && !isDisplayForMobileView) || !isEnabledSidebar || isActiveEntranceExam) { + if ((!isDisplayForDesktopView && !isDisplayForMobileView) || isActiveEntranceExam) { return null; } diff --git a/src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTrigger.test.jsx b/src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTrigger.test.jsx index cca273db71..3b931acdad 100644 --- a/src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTrigger.test.jsx +++ b/src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTrigger.test.jsx @@ -45,7 +45,7 @@ describe('', () => { it('renders correctly for desktop when sidebar is enabled', async () => { const user = userEvent.setup(); const mockToggleSidebar = jest.fn(); - await initTestStore({ enableNavigationSidebar: { enable_navigation_sidebar: true } }); + await initTestStore(); renderWithProvider({ toggleSidebar: mockToggleSidebar }, { isMobileView: false }); const toggleButton = await screen.getByRole('button', { @@ -62,7 +62,7 @@ describe('', () => { it('renders correctly for mobile when sidebar is enabled', async () => { const user = userEvent.setup(); const mockToggleSidebar = jest.fn(); - await initTestStore({ enableNavigationSidebar: { enable_navigation_sidebar: true } }); + await initTestStore(); renderWithProvider({ toggleSidebar: mockToggleSidebar, shouldDisplayFullScreen: true, @@ -82,7 +82,7 @@ describe('', () => { it('changes current sidebar value on click', async () => { const user = userEvent.setup(); const mockToggleSidebar = jest.fn(); - await initTestStore({ enableNavigationSidebar: { enable_navigation_sidebar: true } }); + await initTestStore(); renderWithProvider({ toggleSidebar: mockToggleSidebar, shouldDisplayFullScreen: true, @@ -99,14 +99,4 @@ describe('', () => { expect(mockToggleSidebar).toHaveBeenCalledTimes(1); expect(mockToggleSidebar).toHaveBeenCalledWith(null); }); - - it('does not render when isEnabled is false', async () => { - await initTestStore({ enableNavigationSidebar: { enable_navigation_sidebar: false } }); - renderWithProvider({}, { isMobileView: false }); - - const toggleButton = await screen.queryByRole('button', { - name: messages.toggleCourseOutlineTrigger.defaultMessage, - }); - expect(toggleButton).not.toBeInTheDocument(); - }); }); diff --git a/src/courseware/course/sidebar/sidebars/course-outline/hooks.js b/src/courseware/course/sidebar/sidebars/course-outline/hooks.js index 6d4ea8ca35..30827518de 100644 --- a/src/courseware/course/sidebar/sidebars/course-outline/hooks.js +++ b/src/courseware/course/sidebar/sidebars/course-outline/hooks.js @@ -26,7 +26,6 @@ export const useCourseOutlineSidebar = () => { const dispatch = useDispatch(); const isCollapsedOutlineSidebar = window.sessionStorage.getItem('hideCourseOutlineSidebar'); const { - enableNavigationSidebar: isEnabledSidebar, enableCompletionTracking: isEnabledCompletionTracking, } = useSelector(getCoursewareOutlineSidebarSettings); const courseOutlineShouldUpdate = useSelector(getCourseOutlineShouldUpdate); @@ -48,7 +47,7 @@ export const useCourseOutlineSidebar = () => { shouldDisplayFullScreen, } = useContext(SidebarContext); - const isOpenSidebar = !initialSidebar && isEnabledSidebar && !isCollapsedOutlineSidebar; + const isOpenSidebar = !initialSidebar && !isCollapsedOutlineSidebar; const [isOpen, setIsOpen] = useState(true); const { @@ -109,10 +108,10 @@ export const useCourseOutlineSidebar = () => { }, [initialSidebar, unitId]); useEffect(() => { - if ((isEnabledSidebar && courseOutlineStatus !== LOADED) || courseOutlineShouldUpdate) { + if (courseOutlineStatus !== LOADED || courseOutlineShouldUpdate) { dispatch(getCourseOutlineStructure(courseId)); } - }, [courseId, isEnabledSidebar, courseOutlineShouldUpdate]); + }, [courseId, courseOutlineShouldUpdate]); // Collapse sidebar if screen resized to a width that displays the sidebar automatically useLayoutEffect(() => { @@ -134,7 +133,6 @@ export const useCourseOutlineSidebar = () => { unitId, currentSidebar, shouldDisplayFullScreen, - isEnabledSidebar, isEnabledCompletionTracking, isOpen, setIsOpen, diff --git a/src/courseware/data/api.js b/src/courseware/data/api.js index b19dce4cdf..f7e45fae05 100644 --- a/src/courseware/data/api.js +++ b/src/courseware/data/api.js @@ -104,17 +104,15 @@ export async function getCourseOutline(courseId) { } /** - * Get waffle flag value that enable courseware outline sidebar and always open auxiliary sidebar. + * Get waffle flag value that enables completion tracking. * @param {string} courseId - The unique identifier for the course. - * @returns {Promise<{enable_navigation_sidebar: boolean, enable_navigation_sidebar: boolean}>} - The object - * of boolean values of enabling of the outline sidebar and is always open auxiliary sidebar. + * @returns {Promise<{enable_completion_tracking: boolean}>} - The object + * of boolean values of enabling of the completion tracking. */ export async function getCoursewareOutlineSidebarToggles(courseId) { const url = new URL(`${getConfig().LMS_BASE_URL}/courses/${courseId}/courseware-navigation-sidebar/toggles/`); const { data } = await getAuthenticatedHttpClient().get(url.href); return { - enable_navigation_sidebar: data.enable_navigation_sidebar || false, - always_open_auxiliary_sidebar: data.always_open_auxiliary_sidebar || false, enable_completion_tracking: data.enable_completion_tracking || false, }; } diff --git a/src/courseware/data/redux.test.js b/src/courseware/data/redux.test.js index 91b6e210e7..5a2c54c8fa 100644 --- a/src/courseware/data/redux.test.js +++ b/src/courseware/data/redux.test.js @@ -111,8 +111,6 @@ describe('Data layer integration tests', () => { axiosMock.onGet(courseUrl).reply(200, courseMetadata); axiosMock.onGet(learningSequencesUrlRegExp).reply(200, buildOutlineFromBlocks(courseBlocks)); axiosMock.onGet(coursewareSidebarSettingsUrl).reply(200, { - enable_navigation_sidebar: true, - always_open_auxiliary_sidebar: true, enable_completion_tracking: true, }); @@ -125,8 +123,6 @@ describe('Data layer integration tests', () => { expect(state.courseware.sequenceStatus).toEqual('loading'); expect(state.courseware.sequenceId).toEqual(null); expect(state.courseware.coursewareOutlineSidebarSettings).toEqual({ - enableNavigationSidebar: true, - alwaysOpenAuxiliarySidebar: true, enableCompletionTracking: true, }); @@ -141,8 +137,7 @@ describe('Data layer integration tests', () => { axiosMock.onGet(courseUrl).reply(200, courseMetadata); axiosMock.onGet(learningSequencesUrlRegExp).reply(200, simpleOutline); axiosMock.onGet(coursewareSidebarSettingsUrl).reply(200, { - enable_navigation_sidebar: false, - always_open_auxiliary_sidebar: false, + enable_completion_tracking: false, }); await executeThunk(thunks.fetchCourse(courseId), store.dispatch); @@ -154,8 +149,6 @@ describe('Data layer integration tests', () => { expect(state.courseware.sequenceStatus).toEqual('loading'); expect(state.courseware.sequenceId).toEqual(null); expect(state.courseware.coursewareOutlineSidebarSettings).toEqual({ - enableNavigationSidebar: false, - alwaysOpenAuxiliarySidebar: false, enableCompletionTracking: false, }); diff --git a/src/courseware/data/thunks.js b/src/courseware/data/thunks.js index 15d36f7251..2312cf4daa 100644 --- a/src/courseware/data/thunks.js +++ b/src/courseware/data/thunks.js @@ -88,12 +88,10 @@ export function fetchCourse(courseId) { if (fetchedCoursewareOutlineSidebarTogglesResult) { const { - enable_navigation_sidebar: enableNavigationSidebar, - always_open_auxiliary_sidebar: alwaysOpenAuxiliarySidebar, enable_completion_tracking: enableCompletionTracking, } = coursewareOutlineSidebarTogglesResult.value; dispatch(setCoursewareOutlineSidebarToggles( - { enableNavigationSidebar, alwaysOpenAuxiliarySidebar, enableCompletionTracking }, + { enableCompletionTracking }, )); } diff --git a/src/plugin-slots/CourseBreadcrumbsSlot/README.md b/src/plugin-slots/CourseBreadcrumbsSlot/README.md index 12863d4e08..4538e41412 100644 --- a/src/plugin-slots/CourseBreadcrumbsSlot/README.md +++ b/src/plugin-slots/CourseBreadcrumbsSlot/README.md @@ -14,6 +14,44 @@ This slot is used to replace/modify/hide the course breadcrumbs. ### Default content ![Breadcrumbs slot with default content](./screenshot_default.png) +### Replace with default breadcrumbs component +You can also inject the default `CourseBreadcrumbs` component explicitly using the slot system, for example to wrap or style it differently. +![Breadcrumbs slot with default content](./screenshot_with_default_breadcrumbs.png) + +```js +import { DIRECT_PLUGIN, PLUGIN_OPERATIONS } from '@openedx/frontend-plugin-framework'; + +import CourseBreadcrumbs from './src/courseware/course/breadcrumbs'; + +const config = { + pluginSlots: { + 'org.openedx.frontend.learning.course_breadcrumbs.v1': { + keepDefault: false, + plugins: [ + { + op: PLUGIN_OPERATIONS.Insert, + widget: { + id: 'default_breadcrumbs_component', + type: DIRECT_PLUGIN, + RenderWidget: ({ courseId, sectionId, sequenceId, isStaff, unitId }) => ( + + ), + }, + }, + ] + } + }, +} + +export default config; +``` + ### Replaced with custom component ![🍞 in breadcrumbs slot](./screenshot_custom.png) diff --git a/src/plugin-slots/CourseBreadcrumbsSlot/index.tsx b/src/plugin-slots/CourseBreadcrumbsSlot/index.tsx index 8a438968a1..3f672476f8 100644 --- a/src/plugin-slots/CourseBreadcrumbsSlot/index.tsx +++ b/src/plugin-slots/CourseBreadcrumbsSlot/index.tsx @@ -2,8 +2,6 @@ import React from 'react'; import { PluginSlot } from '@openedx/frontend-plugin-framework'; -import CourseBreadcrumbs from '../../courseware/course/breadcrumbs'; - interface Props { courseId: string; sectionId?: string; @@ -21,13 +19,12 @@ export const CourseBreadcrumbsSlot : React.FC = ({ slotOptions={{ mergeProps: true, }} - > - - + pluginProps={{ + courseId, + sectionId, + sequenceId, + unitId, + isStaff, + }} + /> ); diff --git a/src/plugin-slots/CourseBreadcrumbsSlot/screenshot_default.png b/src/plugin-slots/CourseBreadcrumbsSlot/screenshot_default.png index c10ca827d5..a2e223c364 100644 Binary files a/src/plugin-slots/CourseBreadcrumbsSlot/screenshot_default.png and b/src/plugin-slots/CourseBreadcrumbsSlot/screenshot_default.png differ diff --git a/src/plugin-slots/CourseBreadcrumbsSlot/screenshot_with_default_breadcrumbs.png b/src/plugin-slots/CourseBreadcrumbsSlot/screenshot_with_default_breadcrumbs.png new file mode 100644 index 0000000000..c10ca827d5 Binary files /dev/null and b/src/plugin-slots/CourseBreadcrumbsSlot/screenshot_with_default_breadcrumbs.png differ diff --git a/src/plugin-slots/SequenceNavigationSlot/README.md b/src/plugin-slots/SequenceNavigationSlot/README.md index b86816253b..ba2ffeb9d7 100644 --- a/src/plugin-slots/SequenceNavigationSlot/README.md +++ b/src/plugin-slots/SequenceNavigationSlot/README.md @@ -16,9 +16,47 @@ This slot is used to replace/modify/hide the sequence navigation component that ## Example ### Default content -![Sequence navigation slot with default content](./screenshot_default.png) +![screenshot_default.png](screenshot_default.png) -### Replaced with custom component +### Replace with default sequence navigation component +You can also inject the default `SequenceNavigation` component explicitly using the slot system, for example to wrap or style it differently. +![Sequence navigation slot with default content](./screenshot_with_default_nav.png) + +```js +import { DIRECT_PLUGIN, PLUGIN_OPERATIONS } from '@openedx/frontend-plugin-framework'; + +import { SequenceNavigation } from './src/courseware/course/sequence/sequence-navigation'; + +const config = { + pluginSlots: { + 'org.openedx.frontend.learning.sequence_navigation.v1': { + keepDefault: false, + plugins: [ + { + op: PLUGIN_OPERATIONS.Insert, + widget: { + id: 'custom_sequence_navigation', + type: DIRECT_PLUGIN, + RenderWidget: ({ sequenceId, unitId, nextHandler, onNavigate, previousHandler }) => ( + + ), + }, + }, + ], + }, + }, +}; + +export default config; +``` + +### Replaced with a custom component ![📖 in sequence navigation slot](./screenshot_custom.png) The following `env.config.jsx` will replace the sequence navigation with a custom implementation that uses all available props. diff --git a/src/plugin-slots/SequenceNavigationSlot/index.jsx b/src/plugin-slots/SequenceNavigationSlot/index.jsx index c48652a4c0..b4ceb2f409 100644 --- a/src/plugin-slots/SequenceNavigationSlot/index.jsx +++ b/src/plugin-slots/SequenceNavigationSlot/index.jsx @@ -2,8 +2,6 @@ import React from 'react'; import PropTypes from 'prop-types'; import { PluginSlot } from '@openedx/frontend-plugin-framework'; -import { SequenceNavigation } from '../../courseware/course/sequence/sequence-navigation'; - const SequenceNavigationSlot = ({ sequenceId, unitId, @@ -23,15 +21,7 @@ const SequenceNavigationSlot = ({ onNavigate, previousHandler, }} - > - - + /> ); SequenceNavigationSlot.propTypes = { diff --git a/src/plugin-slots/SequenceNavigationSlot/screenshot_default.png b/src/plugin-slots/SequenceNavigationSlot/screenshot_default.png index 036cca1c3f..93b8c0cb5f 100644 Binary files a/src/plugin-slots/SequenceNavigationSlot/screenshot_default.png and b/src/plugin-slots/SequenceNavigationSlot/screenshot_default.png differ diff --git a/src/plugin-slots/SequenceNavigationSlot/screenshot_with_default_nav.png b/src/plugin-slots/SequenceNavigationSlot/screenshot_with_default_nav.png new file mode 100644 index 0000000000..036cca1c3f Binary files /dev/null and b/src/plugin-slots/SequenceNavigationSlot/screenshot_with_default_nav.png differ diff --git a/src/plugin-slots/UnitTitleSlot/README.md b/src/plugin-slots/UnitTitleSlot/README.md index 4da59b47b3..9308ddfddb 100644 --- a/src/plugin-slots/UnitTitleSlot/README.md +++ b/src/plugin-slots/UnitTitleSlot/README.md @@ -8,12 +8,13 @@ ### Props: * `unitId` * `unit` -* `isEnabledOutlineSidebar` * `renderUnitNavigation` ## Description This slot is used for adding content before or after the Unit title. +`isEnabledOutlineSidebar` is no longer used in the default implementation, +but is still passed as a plugin prop with a default value of `true` for backward compatibility. ## Example @@ -34,9 +35,9 @@ const config = { widget: { id: 'custom_unit_title_content', type: DIRECT_PLUGIN, - RenderWidget: ({ unitId, unit, isEnabledOutlineSidebar, renderUnitNavigation }) => ( + RenderWidget: ({ unitId, unit, renderUnitNavigation }) => ( <> - {isEnabledOutlineSidebar && renderUnitNavigation(true)} + {renderUnitNavigation(true)}

📙: {unit.title}

📙: {unitId}

diff --git a/src/plugin-slots/UnitTitleSlot/index.jsx b/src/plugin-slots/UnitTitleSlot/index.jsx index f753efc921..d21ef42eb0 100644 --- a/src/plugin-slots/UnitTitleSlot/index.jsx +++ b/src/plugin-slots/UnitTitleSlot/index.jsx @@ -8,7 +8,6 @@ import messages from '@src/courseware/course/sequence/messages'; const UnitTitleSlot = ({ unitId, unit, - isEnabledOutlineSidebar, renderUnitNavigation, }) => { const { formatMessage } = useIntl(); @@ -21,7 +20,7 @@ const UnitTitleSlot = ({ pluginProps={{ unitId, unit, - isEnabledOutlineSidebar, + isEnabledOutlineSidebar: true, renderUnitNavigation, }} > @@ -29,7 +28,7 @@ const UnitTitleSlot = ({

{unit.title}

- {isEnabledOutlineSidebar && renderUnitNavigation(true)} + {renderUnitNavigation(true)}

{formatMessage(messages.headerPlaceholder)}

{ }); it.each([true, false])( - 'should load courseware checkpoint correctly if tour enabled is $showCoursewareTour', + 'displays courseware checkpoint only when $showCoursewareTour is enabled', async (showCoursewareTour) => { axiosMock.onGet(tourDataUrl).reply(200, { course_home_tour_status: 'no-tour', @@ -293,13 +293,6 @@ describe('Courseware Tour', () => { }); const container = await loadContainer(); - const sequenceNavButtons = container.querySelectorAll('nav.sequence-navigation a, nav.sequence-navigation button'); - const sequenceNextButton = sequenceNavButtons[4]; - expect(sequenceNextButton).toHaveTextContent('Next'); - fireEvent.click(sequenceNextButton); - - expect(global.location.href).toEqual(`http://localhost/course/${courseId}/${defaultSequenceBlock.id}/${unitBlocks[1].id}`); - const checkpoint = container.querySelectorAll('#pgn__checkpoint'); expect(checkpoint).toHaveLength(showCoursewareTour ? 1 : 0); }, diff --git a/src/setupTest.js b/src/setupTest.js index 0e51c8148e..60a87751e1 100755 --- a/src/setupTest.js +++ b/src/setupTest.js @@ -177,8 +177,6 @@ export async function initializeTestStore(options = {}, overrideStore = true) { courseHomeMetadataUrl = appendBrowserTimezoneToUrl(courseHomeMetadataUrl); const provider = options?.provider || 'legacy'; - const enableNavigationSidebar = options.enableNavigationSidebar || { enable_navigation_sidebar: true }; - const alwaysOpenAuxiliarySidebar = options.alwaysOpenAuxiliarySidebar || { always_open_auxiliary_sidebar: true }; const enableCompletionTracking = options.enableCompletionTracking || { enable_completion_tracking: true }; axiosMock.onGet(courseMetadataUrl).reply(200, courseMetadata); @@ -186,8 +184,6 @@ export async function initializeTestStore(options = {}, overrideStore = true) { axiosMock.onGet(learningSequencesUrlRegExp).reply(200, buildOutlineFromBlocks(courseBlocks)); axiosMock.onGet(discussionConfigUrl).reply(200, { provider }); axiosMock.onGet(coursewareSidebarSettingsUrl).reply(200, { - ...enableNavigationSidebar, - ...alwaysOpenAuxiliarySidebar, ...enableCompletionTracking, });