From bb0b6d9da9b5230195cc7f6178487b7db607d7b7 Mon Sep 17 00:00:00 2001
From: Navin Karkera <navin@disroot.org>
Date: Wed, 13 Dec 2023 17:46:51 +0530
Subject: [PATCH] fix: handle scrolling with drag-n-drop

---
 src/course-outline/CourseOutline.jsx          | 21 +++++++++--------
 .../section-card/SectionCard.jsx              |  2 +-
 src/course-outline/utils.jsx                  | 23 ++++++++++++++-----
 3 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/src/course-outline/CourseOutline.jsx b/src/course-outline/CourseOutline.jsx
index 44726ed4a8..abc88251a5 100644
--- a/src/course-outline/CourseOutline.jsx
+++ b/src/course-outline/CourseOutline.jsx
@@ -98,7 +98,7 @@ const CourseOutline = ({ courseId }) => {
     title: processingNotificationTitle,
   } = useSelector(getProcessingNotification);
 
-  const moveSection = useCallback((dragIndex, hoverIndex) => {
+  const moveSection = useCallback((dragIndex, hoverIndex, dragElement) => {
     setSections((prevSections) => {
       const newList = update(prevSections, {
         $splice: [
@@ -106,6 +106,9 @@ 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;
     });
   }, []);
@@ -117,16 +120,16 @@ const CourseOutline = ({ courseId }) => {
   };
 
   useEffect(() => {
-    if (sectionsList) {
-      setSections((prevSections) => {
-        if (prevSections.length < sectionsList.length) {
-          scrollToElement(listRef);
-        }
-        return sectionsList;
-      });
-    }
+    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 <></>;
diff --git a/src/course-outline/section-card/SectionCard.jsx b/src/course-outline/section-card/SectionCard.jsx
index 1655776d60..bbd5649ce0 100644
--- a/src/course-outline/section-card/SectionCard.jsx
+++ b/src/course-outline/section-card/SectionCard.jsx
@@ -91,7 +91,7 @@ const SectionCard = forwardRef(({
         return;
       }
       // Time to actually perform the action
-      moveSection(dragIndex, hoverIndex);
+      moveSection(dragIndex, hoverIndex, moveRef.current);
       // 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
diff --git a/src/course-outline/utils.jsx b/src/course-outline/utils.jsx
index 29b049cf89..7853eb3109 100644
--- a/src/course-outline/utils.jsx
+++ b/src/course-outline/utils.jsx
@@ -109,12 +109,23 @@ const getHighlightsFormValues = (currentHighlights) => {
   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 {