From 4168e542b82d19a3caee182371c77356f2b0ddf0 Mon Sep 17 00:00:00 2001
From: Peter Kulko <peter.kulko@raccoongang.com>
Date: Wed, 4 Dec 2024 11:38:29 +0200
Subject: [PATCH 1/3] chore: iframe rendering optimization
refactor: corrected scroll to target xblock
refactor: fixed scroll behavior to xblock element
refactor: after rebase
refactor: fixed tests
refactor: updated tests
---
.gitignore | 1 +
src/course-unit/CourseUnit.test.jsx | 16 ++++--
.../add-component/AddComponent.jsx | 1 +
src/course-unit/constants.js | 8 ++-
src/course-unit/data/thunk.js | 32 +++++++----
src/course-unit/header-title/HeaderTitle.jsx | 8 ---
.../header-title/HeaderTitle.test.jsx | 56 ++++++++-----------
src/course-unit/hooks.jsx | 13 +++--
src/course-unit/move-modal/hooks.tsx | 4 +-
src/course-unit/sidebar/PublishControls.jsx | 14 +++--
.../xblock-container-iframe/hooks/types.ts | 2 +-
.../hooks/useIframeContent.tsx | 19 +------
.../hooks/useMessageHandlers.tsx | 7 ++-
.../xblock-container-iframe/index.tsx | 24 ++++----
.../components/PasteButton.jsx | 2 +-
.../configure-modal/ConfigureModal.jsx | 2 +-
src/generic/configure-modal/UnitTab.jsx | 5 +-
17 files changed, 108 insertions(+), 106 deletions(-)
diff --git a/.gitignore b/.gitignore
index 925d1768a4..810bfdb938 100755
--- 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/CourseUnit.test.jsx b/src/course-unit/CourseUnit.test.jsx
index 6d64615de0..1fae368da7 100644
--- a/src/course-unit/CourseUnit.test.jsx
+++ b/src/course-unit/CourseUnit.test.jsx
@@ -243,7 +243,7 @@ describe('<CourseUnit />', () => {
);
simulatePostMessageEvent(messageTypes.deleteXBlock, {
- id: courseVerticalChildrenMock.children[0].block_id,
+ usageId: courseVerticalChildrenMock.children[0].block_id,
});
expect(getByText(/Delete this component?/i)).toBeInTheDocument();
@@ -257,10 +257,10 @@ describe('<CourseUnit />', () => {
const deleteButton = getAllByRole('button', { name: /Delete/i })
.find(({ classList }) => classList.contains('btn-primary'));
- userEvent.click(cancelButton);
+ expect(cancelButton).toBeInTheDocument();
simulatePostMessageEvent(messageTypes.deleteXBlock, {
- id: courseVerticalChildrenMock.children[0].block_id,
+ usageId: courseVerticalChildrenMock.children[0].block_id,
});
expect(getByRole('dialog')).toBeInTheDocument();
@@ -296,8 +296,12 @@ describe('<CourseUnit />', () => {
axiosMock
.onDelete(getXBlockBaseApiUrl(courseVerticalChildrenMock.children[0].block_id))
- .replyOnce(200, { dummy: 'value' });
- await executeThunk(deleteUnitItemQuery(courseId, blockId), store.dispatch);
+ .reply(200, { dummy: 'value' });
+ await executeThunk(deleteUnitItemQuery(
+ courseId,
+ courseVerticalChildrenMock.children[0].block_id,
+ simulatePostMessageEvent,
+ ), store.dispatch);
const updatedCourseVerticalChildren = courseVerticalChildrenMock.children.filter(
child => child.block_id !== courseVerticalChildrenMock.children[0].block_id,
@@ -1617,6 +1621,8 @@ describe('<CourseUnit />', () => {
callbackFn: requestData.callbackFn,
}), store.dispatch);
+ simulatePostMessageEvent(messageTypes.rollbackMovedXBlock, { locator: requestData.sourceLocator });
+
const dismissButton = queryByRole('button', {
name: /dismiss/i, hidden: true,
});
diff --git a/src/course-unit/add-component/AddComponent.jsx b/src/course-unit/add-component/AddComponent.jsx
index 70962a7ac6..ac1a815dd4 100644
--- a/src/course-unit/add-component/AddComponent.jsx
+++ b/src/course-unit/add-component/AddComponent.jsx
@@ -61,6 +61,7 @@ const AddComponent = ({ blockId, handleCreateNewCourseXBlock }) => {
case COMPONENT_TYPES.problem:
case COMPONENT_TYPES.video:
handleCreateNewCourseXBlock({ type, parentLocator: blockId }, ({ courseKey, locator }) => {
+ localStorage.setItem('modalEditLastYPosition', window.scrollY);
navigate(`/course/${courseKey}/editor/${type}/${locator}`);
});
break;
diff --git a/src/course-unit/constants.js b/src/course-unit/constants.js
index 37c3609005..487dba6589 100644
--- a/src/course-unit/constants.js
+++ b/src/course-unit/constants.js
@@ -52,14 +52,20 @@ export const messageTypes = {
videoFullScreen: 'plugin.videoFullScreen',
refreshXBlock: 'refreshXBlock',
showMoveXBlockModal: 'showMoveXBlockModal',
+ completeXBlockMoving: 'completeXBlockMoving',
+ rollbackMovedXBlock: 'rollbackMovedXBlock',
showMultipleComponentPicker: 'showMultipleComponentPicker',
addSelectedComponentsToBank: 'addSelectedComponentsToBank',
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',
};
diff --git a/src/course-unit/data/thunk.js b/src/course-unit/data/thunk.js
index 8f560e75a5..464111ed27 100644
--- a/src/course-unit/data/thunk.js
+++ b/src/course-unit/data/thunk.js
@@ -9,6 +9,7 @@ import { RequestStatus } from '../../data/constants';
import { NOTIFICATION_MESSAGES } from '../../constants';
import { updateModel, updateModels } from '../../generic/model-store';
import { updateClipboardData } from '../../generic/data/slice';
+import { messageTypes } from '../constants';
import {
getCourseUnitData,
editUnitDisplayName,
@@ -126,6 +127,7 @@ export function editCourseUnitVisibilityAndData(
isVisible,
groupAccess,
isDiscussionEnabled,
+ callback,
blockId = itemId,
) {
return async (dispatch) => {
@@ -143,6 +145,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);
@@ -158,11 +163,8 @@ export function editCourseUnitVisibilityAndData(
};
}
-export function createNewCourseXBlock(body, callback, blockId) {
+export function createNewCourseXBlock(body, callback, blockId, sendMessageToIframe) {
return async (dispatch) => {
- dispatch(updateLoadingCourseXblockStatus({ status: RequestStatus.IN_PROGRESS }));
- dispatch(updateSavingStatus({ status: RequestStatus.PENDING }));
-
if (body.stagedContent) {
dispatch(showProcessingNotification(NOTIFICATION_MESSAGES.pasting));
} else {
@@ -188,10 +190,10 @@ 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 }));
if (callback) {
callback(result);
+ } else {
+ sendMessageToIframe(messageTypes.addXBlock, { data: result });
}
const currentBlockId = body.category === 'vertical' ? formattedResult.locator : blockId;
const courseUnit = await getCourseUnitData(currentBlockId);
@@ -220,13 +222,14 @@ export function fetchCourseVerticalChildrenData(itemId) {
};
}
-export function deleteUnitItemQuery(itemId, xblockId) {
+export function deleteUnitItemQuery(itemId, xblockId, sendMessageToIframe) {
return async (dispatch) => {
dispatch(updateSavingStatus({ status: RequestStatus.PENDING }));
dispatch(showProcessingNotification(NOTIFICATION_MESSAGES.deleting));
try {
await deleteUnitItem(xblockId);
+ sendMessageToIframe(messageTypes.completeXBlockDeleting, null);
const { userClipboard } = await getCourseSectionVerticalData(itemId);
dispatch(updateClipboardData(userClipboard));
const courseUnit = await getCourseUnitData(itemId);
@@ -240,13 +243,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());
@@ -300,9 +304,13 @@ export function patchUnitItemQuery({
dispatch(updateMovedXBlockParams(xBlockParams));
dispatch(updateCourseOutlineInfo({}));
dispatch(updateCourseOutlineInfoLoadingStatus({ status: RequestStatus.IN_PROGRESS }));
- const courseUnit = await getCourseUnitData(currentParentLocator);
- dispatch(fetchCourseItemSuccess(courseUnit));
- callbackFn();
+ try {
+ const courseUnit = await getCourseUnitData(currentParentLocator);
+ dispatch(fetchCourseItemSuccess(courseUnit));
+ } catch (error) {
+ handleResponseErrors(error, dispatch, updateSavingStatus);
+ }
+ callbackFn(sourceLocator);
} catch (error) {
handleResponseErrors(error, dispatch, updateSavingStatus);
} finally {
diff --git a/src/course-unit/header-title/HeaderTitle.jsx b/src/course-unit/header-title/HeaderTitle.jsx
index bb3cd3a72c..336d986fab 100644
--- 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/header-title/HeaderTitle.test.jsx b/src/course-unit/header-title/HeaderTitle.test.jsx
index 4383fcf6ca..314b0c25a8 100644
--- a/src/course-unit/header-title/HeaderTitle.test.jsx
+++ b/src/course-unit/header-title/HeaderTitle.test.jsx
@@ -60,11 +60,9 @@ describe('<HeaderTitle />', () => {
it('render HeaderTitle component correctly', () => {
const { getByText, getByRole } = renderComponent();
- waitFor(() => {
- expect(getByText(unitTitle)).toBeInTheDocument();
- expect(getByRole('button', { name: messages.altButtonEdit.defaultMessage })).toBeInTheDocument();
- expect(getByRole('button', { name: messages.altButtonSettings.defaultMessage })).toBeInTheDocument();
- });
+ expect(getByText(unitTitle)).toBeInTheDocument();
+ expect(getByRole('button', { name: messages.altButtonEdit.defaultMessage })).toBeInTheDocument();
+ expect(getByRole('button', { name: messages.altButtonSettings.defaultMessage })).toBeInTheDocument();
});
it('render HeaderTitle with open edit form', () => {
@@ -72,41 +70,35 @@ describe('<HeaderTitle />', () => {
isTitleEditFormOpen: true,
});
- waitFor(() => {
- expect(getByRole('textbox', { name: messages.ariaLabelButtonEdit.defaultMessage })).toBeInTheDocument();
- expect(getByRole('textbox', { name: messages.ariaLabelButtonEdit.defaultMessage })).toHaveValue(unitTitle);
- expect(getByRole('button', { name: messages.altButtonEdit.defaultMessage })).toBeInTheDocument();
- expect(getByRole('button', { name: messages.altButtonSettings.defaultMessage })).toBeInTheDocument();
- });
+ expect(getByRole('textbox', { name: messages.ariaLabelButtonEdit.defaultMessage })).toBeInTheDocument();
+ expect(getByRole('textbox', { name: messages.ariaLabelButtonEdit.defaultMessage })).toHaveValue(unitTitle);
+ expect(getByRole('button', { name: messages.altButtonEdit.defaultMessage })).toBeInTheDocument();
+ expect(getByRole('button', { name: messages.altButtonSettings.defaultMessage })).toBeInTheDocument();
});
it('calls toggle edit title form by clicking on Edit button', () => {
const { getByRole } = renderComponent();
- waitFor(() => {
- const editTitleButton = getByRole('button', { name: messages.altButtonEdit.defaultMessage });
- userEvent.click(editTitleButton);
- expect(handleTitleEdit).toHaveBeenCalledTimes(1);
- });
+ const editTitleButton = getByRole('button', { name: messages.altButtonEdit.defaultMessage });
+ userEvent.click(editTitleButton);
+ expect(handleTitleEdit).toHaveBeenCalledTimes(1);
});
- it('calls saving title by clicking outside or press Enter key', async () => {
+ it('calls saving title by clicking outside or press Enter key', () => {
const { getByRole } = renderComponent({
isTitleEditFormOpen: true,
});
- waitFor(() => {
- const titleField = getByRole('textbox', { name: messages.ariaLabelButtonEdit.defaultMessage });
- userEvent.type(titleField, ' 1');
- expect(titleField).toHaveValue(`${unitTitle} 1`);
- userEvent.click(document.body);
- expect(handleTitleEditSubmit).toHaveBeenCalledTimes(1);
-
- userEvent.click(titleField);
- userEvent.type(titleField, ' 2[Enter]');
- expect(titleField).toHaveValue(`${unitTitle} 1 2`);
- expect(handleTitleEditSubmit).toHaveBeenCalledTimes(2);
- });
+ const titleField = getByRole('textbox', { name: messages.ariaLabelButtonEdit.defaultMessage });
+ userEvent.type(titleField, ' 1');
+ expect(titleField).toHaveValue(`${unitTitle} 1`);
+ userEvent.click(document.body);
+ expect(handleTitleEditSubmit).toHaveBeenCalledTimes(1);
+
+ userEvent.click(titleField);
+ userEvent.type(titleField, ' 2[Enter]');
+ expect(titleField).toHaveValue(`${unitTitle} 1 2`);
+ expect(handleTitleEditSubmit).toHaveBeenCalledTimes(2);
});
it('displays a visibility message with the selected groups for the unit', async () => {
@@ -125,7 +117,7 @@ describe('<HeaderTitle />', () => {
const visibilityMessage = messages.definedVisibilityMessage.defaultMessage
.replace('{selectedGroupsLabel}', 'Visibility group 1');
- waitFor(() => {
+ await waitFor(() => {
expect(getByText(visibilityMessage)).toBeInTheDocument();
});
});
@@ -140,8 +132,8 @@ describe('<HeaderTitle />', () => {
await executeThunk(fetchCourseUnitQuery(blockId), store.dispatch);
const { getByText } = renderComponent();
- waitFor(() => {
- expect(getByText(messages.someVisibilityMessage.defaultMessage)).toBeInTheDocument();
+ await waitFor(() => {
+ expect(getByText(messages.commonVisibilityMessage.defaultMessage)).toBeInTheDocument();
});
});
});
diff --git a/src/course-unit/hooks.jsx b/src/course-unit/hooks.jsx
index 11731cc2ad..d28aaa600f 100644
--- 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();
@@ -113,15 +114,19 @@ export const useCourseUnit = ({ courseId, blockId }) => {
};
const handleCreateNewCourseXBlock = (body, callback) => (
- dispatch(createNewCourseXBlock(body, callback, blockId))
+ dispatch(createNewCourseXBlock(body, callback, blockId, sendMessageToIframe))
);
const unitXBlockActions = {
handleDelete: (XBlockId) => {
- dispatch(deleteUnitItemQuery(blockId, XBlockId));
+ dispatch(deleteUnitItemQuery(blockId, XBlockId, sendMessageToIframe));
},
handleDuplicate: (XBlockId) => {
- dispatch(duplicateUnitItemQuery(blockId, XBlockId));
+ dispatch(duplicateUnitItemQuery(
+ blockId,
+ XBlockId,
+ (courseKey, locator) => sendMessageToIframe(messageTypes.completeXBlockDuplicating, { courseKey, locator }),
+ ));
},
};
@@ -136,7 +141,7 @@ export const useCourseUnit = ({ courseId, blockId }) => {
currentParentLocator,
isMoving: false,
callbackFn: () => {
- sendMessageToIframe(messageTypes.refreshXBlock, null);
+ sendMessageToIframe(messageTypes.rollbackMovedXBlock, { locator: sourceLocator });
window.scrollTo({ top: 0, behavior: 'smooth' });
},
}));
diff --git a/src/course-unit/move-modal/hooks.tsx b/src/course-unit/move-modal/hooks.tsx
index 69ad13470c..d21014e3bb 100644
--- a/src/course-unit/move-modal/hooks.tsx
+++ b/src/course-unit/move-modal/hooks.tsx
@@ -184,8 +184,8 @@ export const useMoveModal = ({
title: state.sourceXBlockInfo.current.displayName,
currentParentLocator: blockId,
isMoving: true,
- callbackFn: () => {
- sendMessageToIframe(messageTypes.refreshXBlock, null);
+ callbackFn: (sourceLocator: string) => {
+ sendMessageToIframe(messageTypes.completeXBlockMoving, { locator: sourceLocator });
closeModal();
window.scrollTo({ top: 0, behavior: 'smooth' });
},
diff --git a/src/course-unit/sidebar/PublishControls.jsx b/src/course-unit/sidebar/PublishControls.jsx
index 0ef08baf28..d5076aa838 100644
--- 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
index 3974656c49..c759f403f9 100644
--- 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
index abbc98b212..17507a8241 100644
--- a/src/course-unit/xblock-container-iframe/hooks/useIframeContent.tsx
+++ b/src/course-unit/xblock-container-iframe/hooks/useIframeContent.tsx
@@ -1,6 +1,4 @@
-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.
@@ -8,26 +6,15 @@ import { messageTypes } from '../../constants';
* @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
index 974f7bf0c6..8633e63e96 100644
--- a/src/course-unit/xblock-container-iframe/hooks/useMessageHandlers.tsx
+++ b/src/course-unit/xblock-container-iframe/hooks/useMessageHandlers.tsx
@@ -1,4 +1,5 @@
import { useMemo } from 'react';
+import { debounce } from 'lodash';
import { copyToClipboard } from '../../../generic/data/thunks';
import { messageTypes } from '../../constants';
@@ -16,8 +17,8 @@ export const useMessageHandlers = ({
dispatch,
setIframeOffset,
handleDeleteXBlock,
- handleRefetchXBlocks,
handleDuplicateXBlock,
+ handleScrollToXBlock,
handleManageXBlockAccess,
}: UseMessageHandlersTypes): MessageHandlersTypes => useMemo(() => ({
[messageTypes.copyXBlock]: ({ usageId }) => dispatch(copyToClipboard(usageId)),
@@ -25,14 +26,14 @@ export const useMessageHandlers = ({
[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]: debounce(({ scrollOffset }) => handleScrollToXBlock(scrollOffset), 3000),
[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
index 9b47a32d07..d0a8528378 100644
--- 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 supportedEditors from '../../editors/supportedEditors';
-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 (supportedEditors[blockType]) {
navigate(`/course/${courseId}/editor/${blockType}/${usageId}`);
}
- refreshIframeContent();
},
- [unitXBlockActions, courseId, navigate, refreshIframeContent],
+ [unitXBlockActions, courseId, navigate],
);
const handleDeleteXBlock = (usageId: string) => {
@@ -76,15 +75,10 @@ const XBlockContainerIframe: FC<XBlockContainerIframeProps> = ({
}
};
- const handleRefetchXBlocks = useCallback(() => {
- setTimeout(() => dispatch(fetchCourseUnitQuery(blockId)), 1000);
- }, [dispatch, blockId]);
-
const onDeleteSubmit = () => {
if (deleteXBlockId) {
unitXBlockActions.handleDelete(deleteXBlockId);
closeDeleteModal();
- refreshIframeContent();
}
};
@@ -92,19 +86,25 @@ const XBlockContainerIframe: FC<XBlockContainerIframeProps> = ({
if (configureXBlockId) {
handleConfigureSubmit(configureXBlockId, ...args, closeConfigureModal);
setAccessManagedXBlockData({});
- refreshIframeContent();
}
};
+ const handleScrollToXBlock = (scrollOffset: number) => {
+ window.scrollBy({
+ top: scrollOffset,
+ 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
index a13dc28c6b..173e57ce33 100644
--- 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 (
diff --git a/src/generic/configure-modal/ConfigureModal.jsx b/src/generic/configure-modal/ConfigureModal.jsx
index 04c82200df..4aa03435f1 100644
--- a/src/generic/configure-modal/ConfigureModal.jsx
+++ b/src/generic/configure-modal/ConfigureModal.jsx
@@ -365,7 +365,7 @@ ConfigureModal.propTypes = {
supportsOnboarding: PropTypes.bool,
showReviewRules: PropTypes.bool,
onlineProctoringRules: PropTypes.string,
- discussionEnabled: PropTypes.bool.isRequired,
+ discussionEnabled: PropTypes.bool,
}).isRequired,
isXBlockComponent: PropTypes.bool,
isSelfPaced: PropTypes.bool.isRequired,
diff --git a/src/generic/configure-modal/UnitTab.jsx b/src/generic/configure-modal/UnitTab.jsx
index 01e18be075..28d7ab45ac 100644
--- a/src/generic/configure-modal/UnitTab.jsx
+++ b/src/generic/configure-modal/UnitTab.jsx
@@ -152,13 +152,14 @@ UnitTab.propTypes = {
isXBlockComponent: PropTypes.bool,
values: PropTypes.shape({
isVisibleToStaffOnly: PropTypes.bool.isRequired,
- discussionEnabled: PropTypes.bool.isRequired,
+ discussionEnabled: PropTypes.bool,
selectedPartitionIndex: PropTypes.oneOfType([
PropTypes.string,
PropTypes.number,
]).isRequired,
selectedGroups: PropTypes.oneOfType([
- PropTypes.string,
+ PropTypes.arrayOf(PropTypes.string),
+ PropTypes.array,
]),
}).isRequired,
setFieldValue: PropTypes.func.isRequired,
From 8078c1993fecd898831ea8c638b5b3a9a3dad6cd Mon Sep 17 00:00:00 2001
From: Peter Kulko <peter.kulko@raccoongang.com>
Date: Thu, 16 Jan 2025 16:48:10 +0200
Subject: [PATCH 2/3] refactor: corrected some tests
---
.../hooks/tests/hooks.test.tsx | 37 ++++++++++++++++++-
1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/src/course-unit/xblock-container-iframe/hooks/tests/hooks.test.tsx b/src/course-unit/xblock-container-iframe/hooks/tests/hooks.test.tsx
index 8883efb04d..f4dd037308 100644
--- a/src/course-unit/xblock-container-iframe/hooks/tests/hooks.test.tsx
+++ b/src/course-unit/xblock-container-iframe/hooks/tests/hooks.test.tsx
@@ -4,7 +4,9 @@ import { useKeyedState } from '@edx/react-unit-test-utils';
import { logError } from '@edx/frontend-platform/logging';
import { stateKeys, messageTypes } from '../../../constants';
-import { useLoadBearingHook, useIFrameBehavior } from '..';
+import { useLoadBearingHook, useIFrameBehavior, useMessageHandlers } from '..';
+
+jest.useFakeTimers();
jest.mock('@edx/react-unit-test-utils', () => ({
useKeyedState: jest.fn(),
@@ -171,3 +173,36 @@ describe('useLoadBearingHook', () => {
expect(setValue.mock.calls);
});
});
+
+describe('useMessageHandlers', () => {
+ it('calls handleScrollToXBlock after debounce delay', () => {
+ const mockHandleScrollToXBlock = jest.fn();
+ const courseId = 'course-v1:Test+101+2025';
+ const navigate = jest.fn();
+ const dispatch = jest.fn();
+ const setIframeOffset = jest.fn();
+ const handleDeleteXBlock = jest.fn();
+ const handleDuplicateXBlock = jest.fn();
+ const handleManageXBlockAccess = jest.fn();
+
+ const { result } = renderHook(() => useMessageHandlers({
+ courseId,
+ navigate,
+ dispatch,
+ setIframeOffset,
+ handleDeleteXBlock,
+ handleDuplicateXBlock,
+ handleScrollToXBlock: mockHandleScrollToXBlock,
+ handleManageXBlockAccess,
+ }));
+
+ act(() => {
+ result.current[messageTypes.scrollToXBlock]({ scrollOffset: 200 });
+ });
+
+ jest.advanceTimersByTime(3000);
+
+ expect(mockHandleScrollToXBlock).toHaveBeenCalledTimes(1);
+ expect(mockHandleScrollToXBlock).toHaveBeenCalledWith(200);
+ });
+});
From b0faf3f99fada7e8fa985a8baa0a503b8ef49a1d Mon Sep 17 00:00:00 2001
From: PKulkoRaccoonGang <peter.kulko@raccoongang.com>
Date: Wed, 22 Jan 2025 11:50:14 -0800
Subject: [PATCH 3/3] refactor: after review
---
src/course-unit/data/thunk.js | 2 +-
src/course-unit/hooks.jsx | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/course-unit/data/thunk.js b/src/course-unit/data/thunk.js
index 464111ed27..ba09a70159 100644
--- a/src/course-unit/data/thunk.js
+++ b/src/course-unit/data/thunk.js
@@ -229,7 +229,7 @@ export function deleteUnitItemQuery(itemId, xblockId, sendMessageToIframe) {
try {
await deleteUnitItem(xblockId);
- sendMessageToIframe(messageTypes.completeXBlockDeleting, null);
+ sendMessageToIframe(messageTypes.completeXBlockDeleting, { locator: xblockId });
const { userClipboard } = await getCourseSectionVerticalData(itemId);
dispatch(updateClipboardData(userClipboard));
const courseUnit = await getCourseUnitData(itemId);
diff --git a/src/course-unit/hooks.jsx b/src/course-unit/hooks.jsx
index d28aaa600f..51c2ff2048 100644
--- a/src/course-unit/hooks.jsx
+++ b/src/course-unit/hooks.jsx
@@ -84,7 +84,7 @@ export const useCourseUnit = ({ courseId, blockId }) => {
isVisible,
groupAccess,
isDiscussionEnabled,
- () => sendMessageToIframe(messageTypes.completeManageXBlockAccess, null),
+ () => sendMessageToIframe(messageTypes.completeManageXBlockAccess, { locator: id }),
blockId,
));
closeModalFn();