diff --git a/src/course-outline/CourseOutline.jsx b/src/course-outline/CourseOutline.jsx index 44726ed4a..797b9567e 100644 --- a/src/course-outline/CourseOutline.jsx +++ b/src/course-outline/CourseOutline.jsx @@ -1,5 +1,5 @@ import { - React, useState, useCallback, useEffect, useRef, + React, useState, useCallback, useEffect, } from 'react'; import update from 'immutability-helper'; import { DndProvider } from 'react-dnd'; @@ -40,10 +40,8 @@ import ConfigureModal from './configure-modal/ConfigureModal'; import DeleteModal from './delete-modal/DeleteModal'; import { useCourseOutline } from './hooks'; import messages from './messages'; -import { scrollToElement } from './utils'; const CourseOutline = ({ courseId }) => { - const listRef = useRef(null); const intl = useIntl(); const { @@ -117,14 +115,7 @@ const CourseOutline = ({ courseId }) => { }; useEffect(() => { - if (sectionsList) { - setSections((prevSections) => { - if (prevSections.length < sectionsList.length) { - scrollToElement(listRef); - } - return sectionsList; - }); - } + setSections(sectionsList); }, [sectionsList]); if (isLoading) { @@ -207,7 +198,6 @@ const CourseOutline = ({ courseId }) => { onNewSubsectionSubmit={handleNewSubsectionSubmit} moveSection={moveSection} finalizeSectionOrder={finalizeSectionOrder} - ref={listRef} > {section.childInfo.children.map((subsection) => ( { onOpenDeleteModal={openDeleteModal} onEditSubmit={handleEditSubmit} onDuplicateSubmit={handleDuplicateSubsectionSubmit} - ref={listRef} /> ))} diff --git a/src/course-outline/CourseOutline.test.jsx b/src/course-outline/CourseOutline.test.jsx index 2a0710c6b..eac6fa5c8 100644 --- a/src/course-outline/CourseOutline.test.jsx +++ b/src/course-outline/CourseOutline.test.jsx @@ -124,6 +124,10 @@ describe('', () => { it('adds new section correctly', async () => { const { findAllByTestId } = render(); let element = await findAllByTestId('section-card'); + window.HTMLElement.prototype.getBoundingClientRect = jest.fn(() => ({ + top: 0, + bottom: 4000, + })); expect(element.length).toBe(4); axiosMock @@ -147,6 +151,10 @@ describe('', () => { const [section] = await findAllByTestId('section-card'); let subsections = await within(section).findAllByTestId('subsection-card'); expect(subsections.length).toBe(1); + window.HTMLElement.prototype.getBoundingClientRect = jest.fn(() => ({ + top: 0, + bottom: 4000, + })); axiosMock .onPost(getXBlockBaseApiUrl()) @@ -160,6 +168,7 @@ describe('', () => { subsections = await within(section).findAllByTestId('subsection-card'); expect(subsections.length).toBe(2); + expect(window.HTMLElement.prototype.scrollIntoView).toBeCalled(); }); it('render error alert after failed reindex correctly', async () => { @@ -490,7 +499,7 @@ describe('', () => { expect(getByRole('button', { name: '5 Section highlights' })).toBeInTheDocument(); }); - it('check section order list when set section order query is successful', async () => { + it('check section list is ordered successfully', async () => { const { getAllByTestId } = render(); const courseBlockId = courseOutlineIndexMock.courseStructure.id; let { children } = courseOutlineIndexMock.courseStructure.childInfo; @@ -511,7 +520,7 @@ describe('', () => { }); }); - it('check section order list when set section order query is unsuccessful', async () => { + it('check section list is restored to original order when API call fails', async () => { const { getAllByTestId } = render(); const courseBlockId = courseOutlineIndexMock.courseStructure.id; const { children } = courseOutlineIndexMock.courseStructure.childInfo; diff --git a/src/course-outline/data/thunk.js b/src/course-outline/data/thunk.js index ea15b9bc0..81996791e 100644 --- a/src/course-outline/data/thunk.js +++ b/src/course-outline/data/thunk.js @@ -129,12 +129,13 @@ export function fetchCourseReindexQuery(courseId, reindexLink) { }; } -export function fetchCourseSectionQuery(sectionId) { +export function fetchCourseSectionQuery(sectionId, shouldScroll = false) { return async (dispatch) => { dispatch(updateFetchSectionLoadingStatus({ status: RequestStatus.IN_PROGRESS })); try { const data = await getCourseItem(sectionId); + data.shouldScroll = shouldScroll; dispatch(updateSectionList(data)); dispatch(updateFetchSectionLoadingStatus({ status: RequestStatus.SUCCESSFUL })); } catch (error) { @@ -307,6 +308,8 @@ export function duplicateSectionQuery(sectionId, courseBlockId) { courseBlockId, async (locator) => { const duplicatedItem = await getCourseItem(locator); + // Page should scroll to newly duplicated item. + duplicatedItem.shouldScroll = true; dispatch(duplicateSection({ id: sectionId, duplicatedItem })); }, )); @@ -318,7 +321,7 @@ export function duplicateSubsectionQuery(subsectionId, sectionId) { dispatch(duplicateCourseItemQuery( subsectionId, sectionId, - async () => dispatch(fetchCourseSectionQuery(sectionId)), + async () => dispatch(fetchCourseSectionQuery(sectionId, true)), )); }; } @@ -344,6 +347,8 @@ function addNewCourseItemQuery(parentLocator, category, displayName, addItemFn) ).then(async (result) => { if (result) { const data = await getCourseItem(result.locator); + // Page should scroll to newly created item. + data.shouldScroll = true; dispatch(addItemFn(data)); dispatch(updateSavingStatus({ status: RequestStatus.SUCCESSFUL })); dispatch(hideProcessingNotification()); diff --git a/src/course-outline/highlights-modal/HighlightsModal.jsx b/src/course-outline/highlights-modal/HighlightsModal.jsx index 625cd4c85..8d1a73ec2 100644 --- a/src/course-outline/highlights-modal/HighlightsModal.jsx +++ b/src/course-outline/highlights-modal/HighlightsModal.jsx @@ -65,6 +65,7 @@ const HighlightsModal = ({ value={values[key]} floatingLabel={intl.formatMessage(messages.highlight, { index: index + 1 })} maxLength={HIGHLIGHTS_FIELD_MAX_LENGTH} + as="textarea" /> ))} diff --git a/src/course-outline/section-card/SectionCard.jsx b/src/course-outline/section-card/SectionCard.jsx index 1655776d6..e3c1259e2 100644 --- a/src/course-outline/section-card/SectionCard.jsx +++ b/src/course-outline/section-card/SectionCard.jsx @@ -1,5 +1,5 @@ import React, { - forwardRef, useEffect, useState, useRef, + useEffect, useState, useRef, } from 'react'; import { useDrag, useDrop } from 'react-dnd'; import PropTypes from 'prop-types'; @@ -11,11 +11,11 @@ import { Add as IconAdd } from '@edx/paragon/icons'; import { setCurrentItem, setCurrentSection } from '../data/slice'; import { RequestStatus } from '../../data/constants'; import CardHeader from '../card-header/CardHeader'; -import { getItemStatus } from '../utils'; +import { getItemStatus, scrollToElement } from '../utils'; import messages from './messages'; import ItemTypes from './itemTypes'; -const SectionCard = forwardRef(({ +const SectionCard = ({ section, index, children, @@ -30,7 +30,8 @@ const SectionCard = forwardRef(({ onNewSubsectionSubmit, moveSection, finalizeSectionOrder, -}, lastItemRef) => { +}) => { + const currentRef = useRef(null); const intl = useIntl(); const dispatch = useDispatch(); const [isExpanded, setIsExpanded] = useState(isSectionsExpanded); @@ -40,6 +41,13 @@ const SectionCard = forwardRef(({ setIsExpanded(isSectionsExpanded); }, [isSectionsExpanded]); + useEffect(() => { + // if this items has been newly added, scroll to it. + if (currentRef.current && section.shouldScroll) { + scrollToElement(currentRef.current); + } + }, []); + const { id, displayName, @@ -52,8 +60,6 @@ const SectionCard = forwardRef(({ highlights, } = section; - const moveRef = useRef(null); - const [{ handlerId }, drop] = useDrop({ accept: ItemTypes.SECTION, collect(monitor) { @@ -62,7 +68,7 @@ const SectionCard = forwardRef(({ }; }, hover(item, monitor) { - if (!moveRef.current) { + if (!currentRef.current) { return; } const dragIndex = item.index; @@ -72,7 +78,7 @@ const SectionCard = forwardRef(({ return; } // Determine rectangle on screen - const hoverBoundingRect = moveRef.current?.getBoundingClientRect(); + const hoverBoundingRect = currentRef.current?.getBoundingClientRect(); // Get vertical middle const hoverMiddleY = (hoverBoundingRect.bottom - hoverBoundingRect.top) / 2; // Determine mouse position @@ -117,7 +123,7 @@ const SectionCard = forwardRef(({ }, }); const opacity = isDragging ? 0 : 1; - drag(drop(moveRef)); + drag(drop(currentRef)); const sectionStatus = getItemStatus({ published, @@ -166,9 +172,9 @@ const SectionCard = forwardRef(({ data-testid="section-card" data-handler-id={handlerId} style={{ opacity }} - ref={moveRef} + ref={currentRef} > -
+
); -}); +}; SectionCard.defaultProps = { children: null, @@ -239,6 +245,7 @@ SectionCard.propTypes = { visibilityState: PropTypes.string.isRequired, staffOnlyMessage: PropTypes.bool.isRequired, highlights: PropTypes.arrayOf(PropTypes.string).isRequired, + shouldScroll: PropTypes.bool, }).isRequired, index: PropTypes.number.isRequired, children: PropTypes.node, diff --git a/src/course-outline/subsection-card/SubsectionCard.jsx b/src/course-outline/subsection-card/SubsectionCard.jsx index 817a9053a..f10acc271 100644 --- a/src/course-outline/subsection-card/SubsectionCard.jsx +++ b/src/course-outline/subsection-card/SubsectionCard.jsx @@ -1,4 +1,4 @@ -import { forwardRef, useEffect, useState } from 'react'; +import { useEffect, useState, useRef } from 'react'; import PropTypes from 'prop-types'; import { useDispatch } from 'react-redux'; import { useIntl } from '@edx/frontend-platform/i18n'; @@ -8,10 +8,10 @@ import { Add as IconAdd } from '@edx/paragon/icons'; import { setCurrentItem, setCurrentSection, setCurrentSubsection } from '../data/slice'; import { RequestStatus } from '../../data/constants'; import CardHeader from '../card-header/CardHeader'; -import { getItemStatus } from '../utils'; +import { getItemStatus, scrollToElement } from '../utils'; import messages from './messages'; -const SubsectionCard = forwardRef(({ +const SubsectionCard = ({ section, subsection, children, @@ -20,7 +20,8 @@ const SubsectionCard = forwardRef(({ savingStatus, onOpenDeleteModal, onDuplicateSubmit, -}, lastItemRef) => { +}) => { + const currentRef = useRef(null); const intl = useIntl(); const dispatch = useDispatch(); const [isExpanded, setIsExpanded] = useState(false); @@ -64,6 +65,15 @@ const SubsectionCard = forwardRef(({ closeForm(); }; + useEffect(() => { + // if this items has been newly added, scroll to it. + // we need to check section.shouldScroll as whole section is fetched when a + // subsection is duplicated under it. + if (currentRef.current && (section.shouldScroll || subsection.shouldScroll)) { + scrollToElement(currentRef.current); + } + }, []); + useEffect(() => { if (savingStatus === RequestStatus.SUCCESSFUL) { closeForm(); @@ -71,7 +81,7 @@ const SubsectionCard = forwardRef(({ }, [savingStatus]); return ( -
+
); -}); +}; SubsectionCard.defaultProps = { children: null, @@ -123,6 +133,7 @@ SubsectionCard.propTypes = { visibleToStaffOnly: PropTypes.bool, visibilityState: PropTypes.string.isRequired, staffOnlyMessage: PropTypes.bool.isRequired, + shouldScroll: PropTypes.bool, }).isRequired, subsection: PropTypes.shape({ id: PropTypes.string.isRequired, @@ -133,6 +144,7 @@ SubsectionCard.propTypes = { visibleToStaffOnly: PropTypes.bool, visibilityState: PropTypes.string.isRequired, staffOnlyMessage: PropTypes.bool.isRequired, + shouldScroll: PropTypes.bool, }).isRequired, children: PropTypes.node, onOpenPublishModal: PropTypes.func.isRequired, diff --git a/src/course-outline/subsection-card/SubsectionCard.test.jsx b/src/course-outline/subsection-card/SubsectionCard.test.jsx index 4be624736..a0f8c688c 100644 --- a/src/course-outline/subsection-card/SubsectionCard.test.jsx +++ b/src/course-outline/subsection-card/SubsectionCard.test.jsx @@ -13,6 +13,18 @@ import SubsectionCard from './SubsectionCard'; let axiosMock; let store; +const section = { + id: '123', + displayName: 'Section Name', + published: true, + releasedToStudents: true, + visibleToStaffOnly: false, + visibilityState: 'visible', + staffOnlyMessage: false, + hasChanges: false, + highlights: ['highlight 1', 'highlight 2'], +}; + const subsection = { id: '123', displayName: 'Subsection Name', @@ -28,6 +40,7 @@ const renderComponent = (props) => render( { return formValues; }; -const scrollToElement = (ref) => { - ref.current?.scrollIntoView({ - block: 'end', - inline: 'nearest', - behavior: 'smooth', - }); +/** + * Method to scroll into view port, if it's outside the viewport + * + * @param {Object} target - DOM Element + * @returns {undefined} + */ +const scrollToElement = target => { + if (target.getBoundingClientRect().bottom > window.innerHeight) { + // The bottom of the target will be aligned to the bottom of the visible area of the scrollable ancestor. + target.scrollIntoView({ behavior: 'smooth', block: 'end', inline: 'nearest' }); + } + + // Target is outside the view from the top + if (target.getBoundingClientRect().top < 0) { + // The top of the target will be aligned to the top of the visible area of the scrollable ancestor + target.scrollIntoView({ behavior: 'smooth' }); + } }; export {