chore: iframe rendering optimization

openedx · Dec 4, 2024 · 61f957e · 61f957e
1 parent ea4da9f
commit 61f957e
Show file tree

Hide file tree

Showing 11 changed files with 64 additions and 58 deletions.
diff --git a/.gitignore b/.gitignore
@@ -1,6 +1,7 @@
 .DS_Store
 .eslintcache
 .idea
+.run
 node_modules
 npm-debug.log
 coverage

diff --git a/src/course-unit/constants.js b/src/course-unit/constants.js
@@ -57,11 +57,15 @@ export const messageTypes = {
   showXBlockLibraryChangesPreview: 'showXBlockLibraryChangesPreview',
   copyXBlock: 'copyXBlock',
   manageXBlockAccess: 'manageXBlockAccess',
+  completeManageXBlockAccess: 'completeManageXBlockAccess',
   deleteXBlock: 'deleteXBlock',
+  completeXBlockDeleting: 'completeXBlockDeleting',
   duplicateXBlock: 'duplicateXBlock',
-  refreshXBlockPositions: 'refreshPositions',
+  completeXBlockDuplicating: 'completeXBlockDuplicating',
   newXBlockEditor: 'newXBlockEditor',
   toggleCourseXBlockDropdown: 'toggleCourseXBlockDropdown',
+  addXBlock: 'addXBlock',
+  scrollToXBlock: 'scrollToXBlock',
 };
 
 export const COMPONENT_TYPES = {

diff --git a/src/course-unit/data/thunk.js b/src/course-unit/data/thunk.js
@@ -126,6 +126,7 @@ export function editCourseUnitVisibilityAndData(
   isVisible,
   groupAccess,
   isDiscussionEnabled,
+  callback,
   blockId = itemId,
 ) {
   return async (dispatch) => {
@@ -143,6 +144,9 @@ export function editCourseUnitVisibilityAndData(
         isDiscussionEnabled,
       ).then(async (result) => {
         if (result) {
+          if (callback) {
+            callback();
+          }
           const courseUnit = await getCourseUnitData(blockId);
           dispatch(fetchCourseItemSuccess(courseUnit));
           const courseVerticalChildrenData = await getCourseVerticalChildren(blockId);
@@ -160,8 +164,8 @@ export function editCourseUnitVisibilityAndData(
 
 export function createNewCourseXBlock(body, callback, blockId) {
   return async (dispatch) => {
-    dispatch(updateLoadingCourseXblockStatus({ status: RequestStatus.IN_PROGRESS }));
-    dispatch(updateSavingStatus({ status: RequestStatus.PENDING }));
+    // dispatch(updateLoadingCourseXblockStatus({ status: RequestStatus.IN_PROGRESS }));
+    // dispatch(updateSavingStatus({ status: RequestStatus.PENDING }));
 
     if (body.stagedContent) {
       dispatch(showProcessingNotification(NOTIFICATION_MESSAGES.pasting));
@@ -188,8 +192,8 @@ export function createNewCourseXBlock(body, callback, blockId) {
           const courseVerticalChildrenData = await getCourseVerticalChildren(blockId);
           dispatch(updateCourseVerticalChildren(courseVerticalChildrenData));
           dispatch(hideProcessingNotification());
-          dispatch(updateLoadingCourseXblockStatus({ status: RequestStatus.SUCCESSFUL }));
-          dispatch(updateSavingStatus({ status: RequestStatus.SUCCESSFUL }));
+          // dispatch(updateLoadingCourseXblockStatus({ status: RequestStatus.SUCCESSFUL }));
+          // dispatch(updateSavingStatus({ status: RequestStatus.SUCCESSFUL }));
           if (callback) {
             callback(result);
           }
@@ -220,13 +224,14 @@ export function fetchCourseVerticalChildrenData(itemId) {
   };
 }
 
-export function deleteUnitItemQuery(itemId, xblockId) {
+export function deleteUnitItemQuery(itemId, xblockId, callback) {
   return async (dispatch) => {
     dispatch(updateSavingStatus({ status: RequestStatus.PENDING }));
     dispatch(showProcessingNotification(NOTIFICATION_MESSAGES.deleting));
 
     try {
       await deleteUnitItem(xblockId);
+      callback();
       const { userClipboard } = await getCourseSectionVerticalData(itemId);
       dispatch(updateClipboardData(userClipboard));
       const courseUnit = await getCourseUnitData(itemId);
@@ -240,13 +245,14 @@ export function deleteUnitItemQuery(itemId, xblockId) {
   };
 }
 
-export function duplicateUnitItemQuery(itemId, xblockId) {
+export function duplicateUnitItemQuery(itemId, xblockId, callback) {
   return async (dispatch) => {
     dispatch(updateSavingStatus({ status: RequestStatus.PENDING }));
     dispatch(showProcessingNotification(NOTIFICATION_MESSAGES.duplicating));
 
     try {
-      await duplicateUnitItem(itemId, xblockId);
+      const { courseKey, locator } = await duplicateUnitItem(itemId, xblockId);
+      callback(courseKey, locator);
       const courseUnit = await getCourseUnitData(itemId);
       dispatch(fetchCourseItemSuccess(courseUnit));
       dispatch(hideProcessingNotification());

diff --git a/src/course-unit/header-title/HeaderTitle.jsx b/src/course-unit/header-title/HeaderTitle.jsx
@@ -11,8 +11,6 @@ import {
 import ConfigureModal from '../../generic/configure-modal/ConfigureModal';
 import { getCourseUnitData } from '../data/selectors';
 import { updateQueryPendingStatus } from '../data/slice';
-import { messageTypes } from '../constants';
-import { useIframe } from '../context/hooks';
 import messages from './messages';
 
 const HeaderTitle = ({
@@ -28,15 +26,9 @@ const HeaderTitle = ({
   const currentItemData = useSelector(getCourseUnitData);
   const [isConfigureModalOpen, openConfigureModal, closeConfigureModal] = useToggle(false);
   const { selectedPartitionIndex, selectedGroupsLabel } = currentItemData.userPartitionInfo;
-  const { sendMessageToIframe } = useIframe();
 
   const onConfigureSubmit = (...arg) => {
     handleConfigureSubmit(currentItemData.id, ...arg, closeConfigureModal);
-    // TODO: this artificial delay is a temporary solution
-    // to ensure the iframe content is properly refreshed.
-    setTimeout(() => {
-      sendMessageToIframe(messageTypes.refreshXBlock, null);
-    }, 1000);
   };
 
   const getVisibilityMessage = () => {

diff --git a/src/course-unit/hooks.jsx b/src/course-unit/hooks.jsx
@@ -84,6 +84,7 @@ export const useCourseUnit = ({ courseId, blockId }) => {
       isVisible,
       groupAccess,
       isDiscussionEnabled,
+      () => sendMessageToIframe(messageTypes.completeManageXBlockAccess, null),
       blockId,
     ));
     closeModalFn();
@@ -112,16 +113,29 @@ export const useCourseUnit = ({ courseId, blockId }) => {
     }
   };
 
-  const handleCreateNewCourseXBlock = (body, callback) => (
+  const handleCreateNewCourseXBlock = (
+    body,
+    callback = (result) => {
+      sendMessageToIframe(messageTypes.addXBlock, { data: result });
+    },
+  ) => (
     dispatch(createNewCourseXBlock(body, callback, blockId))
   );
 
   const unitXBlockActions = {
     handleDelete: (XBlockId) => {
-      dispatch(deleteUnitItemQuery(blockId, XBlockId));
+      dispatch(deleteUnitItemQuery(
+        blockId,
+        XBlockId,
+        () => sendMessageToIframe(messageTypes.completeXBlockDeleting, null),
+      ));
     },
     handleDuplicate: (XBlockId) => {
-      dispatch(duplicateUnitItemQuery(blockId, XBlockId));
+      dispatch(duplicateUnitItemQuery(
+        blockId,
+        XBlockId,
+        (courseKey, locator) => sendMessageToIframe(messageTypes.completeXBlockDuplicating, { courseKey, locator }),
+      ));
     },
   };
 

diff --git a/src/course-unit/sidebar/PublishControls.jsx b/src/course-unit/sidebar/PublishControls.jsx
@@ -35,12 +35,14 @@ const PublishControls = ({ blockId }) => {
 
   const handleCourseUnitDiscardChanges = () => {
     closeDiscardModal();
-    dispatch(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.discardChanges));
-    // TODO: this artificial delay is a temporary solution
-    // to ensure the iframe content is properly refreshed.
-    setTimeout(() => {
-      sendMessageToIframe(messageTypes.refreshXBlock, null);
-    }, 1000);
+    dispatch(editCourseUnitVisibilityAndData(
+      blockId,
+      PUBLISH_TYPES.discardChanges,
+      null,
+      null,
+      null,
+      () => sendMessageToIframe(messageTypes.refreshXBlock, null),
+    ));
   };
 
   const handleCourseUnitPublish = () => {

diff --git a/src/course-unit/xblock-container-iframe/hooks/types.ts b/src/course-unit/xblock-container-iframe/hooks/types.ts
@@ -4,7 +4,7 @@ export type UseMessageHandlersTypes = {
   dispatch: (action: any) => void;
   setIframeOffset: (height: number) => void;
   handleDeleteXBlock: (usageId: string) => void;
-  handleRefetchXBlocks: () => void;
+  handleScrollToXBlock: (scrollOffset: number) => void;
   handleDuplicateXBlock: (blockType: string, usageId: string) => void;
   handleManageXBlockAccess: (usageId: string) => void;
 };

diff --git a/src/course-unit/xblock-container-iframe/hooks/useIframeContent.tsx b/src/course-unit/xblock-container-iframe/hooks/useIframeContent.tsx
@@ -1,33 +1,20 @@
-import { useEffect, useCallback, RefObject } from 'react';
-
-import { messageTypes } from '../../constants';
+import { useEffect, RefObject } from 'react';
 
 /**
  * Hook for managing iframe content and providing utilities to interact with the iframe.
  *
  * @param {React.RefObject<HTMLIFrameElement>} iframeRef - A React ref for the iframe element.
  * @param {(ref: React.RefObject<HTMLIFrameElement>) => void} setIframeRef -
  * A function to associate the iframeRef with the parent context.
- * @param {(type: string, payload: any) => void} sendMessageToIframe - A function to send messages to the iframe.
  *
  * @returns {Object} - An object containing utility functions.
- * @returns {() => void} return.refreshIframeContent -
- * A function to refresh the iframe content by sending a specific message.
+ * @returns {() => void}
  */
 export const useIframeContent = (
   iframeRef: RefObject<HTMLIFrameElement>,
   setIframeRef: (ref: RefObject<HTMLIFrameElement>) => void,
-  sendMessageToIframe: (type: string, payload: any) => void,
-): { refreshIframeContent: () => void } => {
+): void => {
   useEffect(() => {
     setIframeRef(iframeRef);
   }, [setIframeRef, iframeRef]);
-
-  // TODO: this artificial delay is a temporary solution
-  // to ensure the iframe content is properly refreshed.
-  const refreshIframeContent = useCallback(() => {
-    setTimeout(() => sendMessageToIframe(messageTypes.refreshXBlock, null), 1000);
-  }, [sendMessageToIframe]);
-
-  return { refreshIframeContent };
 };
diff --git a/src/course-unit/xblock-container-iframe/hooks/useMessageHandlers.tsx b/src/course-unit/xblock-container-iframe/hooks/useMessageHandlers.tsx
@@ -16,23 +16,23 @@ export const useMessageHandlers = ({
   dispatch,
   setIframeOffset,
   handleDeleteXBlock,
-  handleRefetchXBlocks,
   handleDuplicateXBlock,
+  handleScrollToXBlock,
   handleManageXBlockAccess,
 }: UseMessageHandlersTypes): MessageHandlersTypes => useMemo(() => ({
   [messageTypes.copyXBlock]: ({ usageId }) => dispatch(copyToClipboard(usageId)),
   [messageTypes.deleteXBlock]: ({ usageId }) => handleDeleteXBlock(usageId),
   [messageTypes.newXBlockEditor]: ({ blockType, usageId }) => navigate(`/course/${courseId}/editor/${blockType}/${usageId}`),
   [messageTypes.duplicateXBlock]: ({ blockType, usageId }) => handleDuplicateXBlock(blockType, usageId),
   [messageTypes.manageXBlockAccess]: ({ usageId }) => handleManageXBlockAccess(usageId),
-  [messageTypes.refreshXBlockPositions]: handleRefetchXBlocks,
+  [messageTypes.scrollToXBlock]: ({ scrollOffset }) => handleScrollToXBlock(scrollOffset),
   [messageTypes.toggleCourseXBlockDropdown]: ({
     courseXBlockDropdownHeight,
   }: { courseXBlockDropdownHeight: number }) => setIframeOffset(courseXBlockDropdownHeight),
 }), [
   courseId,
   handleDeleteXBlock,
-  handleRefetchXBlocks,
   handleDuplicateXBlock,
   handleManageXBlockAccess,
+  handleScrollToXBlock,
 ]);
diff --git a/src/course-unit/xblock-container-iframe/index.tsx b/src/course-unit/xblock-container-iframe/index.tsx
@@ -10,7 +10,6 @@ import DeleteModal from '../../generic/delete-modal/DeleteModal';
 import ConfigureModal from '../../generic/configure-modal/ConfigureModal';
 import { IFRAME_FEATURE_POLICY } from '../../constants';
 import { COMPONENT_TYPES_WITH_NEW_EDITOR } from '../constants';
-import { fetchCourseUnitQuery } from '../data/thunk';
 import { useIframe } from '../context/hooks';
 import {
   useMessageHandlers,
@@ -43,9 +42,10 @@ const XBlockContainerIframe: FC<XBlockContainerIframeProps> = ({
 
   const iframeUrl = useMemo(() => getIframeUrl(blockId), [blockId]);
 
-  const { setIframeRef, sendMessageToIframe } = useIframe();
+  const { setIframeRef } = useIframe();
   const { iframeHeight } = useIFrameBehavior({ id: blockId, iframeUrl });
-  const { refreshIframeContent } = useIframeContent(iframeRef, setIframeRef, sendMessageToIframe);
+
+  useIframeContent(iframeRef, setIframeRef);
 
   useEffect(() => {
     setIframeRef(iframeRef);
@@ -57,9 +57,8 @@ const XBlockContainerIframe: FC<XBlockContainerIframeProps> = ({
       if (COMPONENT_TYPES_WITH_NEW_EDITOR[blockType]) {
         navigate(`/course/${courseId}/editor/${blockType}/${usageId}`);
       }
-      refreshIframeContent();
     },
-    [unitXBlockActions, courseId, navigate, refreshIframeContent],
+    [unitXBlockActions, courseId, navigate],
   );
 
   const handleDeleteXBlock = (usageId: string) => {
@@ -76,35 +75,36 @@ const XBlockContainerIframe: FC<XBlockContainerIframeProps> = ({
     }
   };
 
-  const handleRefetchXBlocks = useCallback(() => {
-    setTimeout(() => dispatch(fetchCourseUnitQuery(blockId)), 1000);
-  }, [dispatch, blockId]);
-
   const onDeleteSubmit = () => {
     if (deleteXBlockId) {
       unitXBlockActions.handleDelete(deleteXBlockId);
       closeDeleteModal();
-      refreshIframeContent();
     }
   };
 
   const onManageXBlockAccessSubmit = (...args: any[]) => {
     if (configureXBlockId) {
       handleConfigureSubmit(configureXBlockId, ...args, closeConfigureModal);
       setAccessManagedXBlockData({});
-      refreshIframeContent();
     }
   };
 
+  const handleScrollToXBlock = (scrollOffset: number) => {
+    window.scrollBy({
+      top: scrollOffset + 1000,
+      behavior: 'smooth',
+    });
+  };
+
   const messageHandlers = useMessageHandlers({
     courseId,
     navigate,
     dispatch,
     setIframeOffset,
     handleDeleteXBlock,
-    handleRefetchXBlocks,
     handleDuplicateXBlock,
     handleManageXBlockAccess,
+    handleScrollToXBlock,
   });
 
   useIframeMessages(messageHandlers);

diff --git a/src/generic/clipboard/paste-component/components/PasteButton.jsx b/src/generic/clipboard/paste-component/components/PasteButton.jsx
@@ -7,7 +7,7 @@ const PasteButton = ({ onClick, text, className }) => {
   const { blockId } = useParams();
 
   const handlePasteXBlockComponent = () => {
-    onClick({ stagedContent: 'clipboard', parentLocator: blockId }, null, blockId);
+    onClick({ stagedContent: 'clipboard', parentLocator: blockId });
   };
 
   return (