diff --git a/src/course-outline/CourseOutline.jsx b/src/course-outline/CourseOutline.jsx index abc88251a5..797b9567ec 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 { @@ -98,7 +96,7 @@ const CourseOutline = ({ courseId }) => { title: processingNotificationTitle, } = useSelector(getProcessingNotification); - const moveSection = useCallback((dragIndex, hoverIndex, dragElement) => { + const moveSection = useCallback((dragIndex, hoverIndex) => { setSections((prevSections) => { const newList = update(prevSections, { $splice: [ @@ -106,9 +104,6 @@ const CourseOutline = ({ courseId }) => { [hoverIndex, 0, prevSections[dragIndex]], ], }); - // set listRef to element that was dragged to make sure scrolling - // uses the correct element while calculating visibility. - listRef.current = dragElement; return newList; }); }, []); @@ -123,13 +118,6 @@ const CourseOutline = ({ courseId }) => { setSections(sectionsList); }, [sectionsList]); - useEffect(() => { - // scrollToElement called in another useEffect to make sure sections are rendered first. - if (listRef.current) { - scrollToElement(listRef.current); - } - }, [sections]); - if (isLoading) { // eslint-disable-next-line react/jsx-no-useless-fragment return <>; @@ -210,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/data/thunk.js b/src/course-outline/data/thunk.js index ea15b9bc03..81996791e2 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/section-card/SectionCard.jsx b/src/course-outline/section-card/SectionCard.jsx index bbd5649ce0..e3c1259e27 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 @@ -91,7 +97,7 @@ const SectionCard = forwardRef(({ return; } // Time to actually perform the action - moveSection(dragIndex, hoverIndex, moveRef.current); + moveSection(dragIndex, hoverIndex); // Note: we're mutating the monitor item here! // Generally it's better to avoid mutations, // but it's good here for the sake of performance @@ -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 817a9053aa..f10acc271c 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,