Skip to content

Commit

Permalink
refactor: refactoring after review
Browse files Browse the repository at this point in the history
  • Loading branch information
PKulkoRaccoonGang committed Mar 20, 2024
1 parent 31353ab commit 1f8eafc
Show file tree
Hide file tree
Showing 15 changed files with 38 additions and 62 deletions.
10 changes: 0 additions & 10 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 3 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@
"regenerator-runtime": "0.13.7",
"universal-cookie": "^4.0.4",
"uuid": "^3.4.0",
"yup": "0.31.1",
"querystring": "0.2.1"
"yup": "0.31.1"
},
"devDependencies": {
"@edx/browserslist-config": "1.2.0",
Expand All @@ -103,15 +102,15 @@
"@testing-library/user-event": "^13.2.1",
"axios": "^0.27.2",
"axios-mock-adapter": "1.22.0",
"copy-webpack-plugin": "^11.0.0",
"eslint-import-resolver-webpack": "^0.13.8",
"glob": "7.2.3",
"husky": "7.0.4",
"jest-canvas-mock": "^2.5.2",
"jest-expect-message": "^1.1.3",
"react-test-renderer": "17.0.2",
"reactifex": "1.1.1",
"ts-loader": "^9.5.0",
"copy-webpack-plugin": "^11.0.0"
"ts-loader": "^9.5.0"
},
"peerDependencies": {
"decode-uri-component": ">=0.2.2"
Expand Down
12 changes: 6 additions & 6 deletions src/course-unit/course-xblock/CourseXBlock.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,18 @@ import { COURSE_BLOCK_NAMES } from '../../constants';
import {
getCanEdit,
getCourseId,
getXBlockIframeHtmlAndResources,
getXBlockIFrameHtmlAndResources,
} from '../data/selectors';
import {
copyToClipboard,
fetchXBlockIframeHtmlAndResourcesQuery,
} from '../../generic/data/thunks';
import { getHandlerUrl } from '../data/api';
import { fetchXBlockIFrameHtmlAndResourcesQuery } from '../data/thunk';
import { COMPONENT_TYPES } from '../constants';
import XBlockMessages from './xblock-messages/XBlockMessages';
import RenderErrorAlert from './render-error-alert';
import { XBlockContent } from './xblock-content';
import messages from './messages';
import { getHandlerUrl } from './utils';

const CourseXBlock = ({
id, title, type, unitXBlockActions, shouldScroll, userPartitionInfo,
Expand All @@ -42,15 +42,15 @@ const CourseXBlock = ({
const courseId = useSelector(getCourseId);
const canEdit = useSelector(getCanEdit);
const intl = useIntl();
const xblockIframeHtmlAndResources = useSelector(getXBlockIframeHtmlAndResources);
const xblockInstanceHtmlAndResources = find(xblockIframeHtmlAndResources, { xblockId: id });
const xblockIFrameHtmlAndResources = useSelector(getXBlockIFrameHtmlAndResources);
const xblockInstanceHtmlAndResources = find(xblockIFrameHtmlAndResources, { xblockId: id });

const visibilityMessage = userPartitionInfo.selectedGroupsLabel
? intl.formatMessage(messages.visibilityMessage, { selectedGroupsLabel: userPartitionInfo.selectedGroupsLabel })
: null;

useEffect(() => {
dispatch(fetchXBlockIframeHtmlAndResourcesQuery(id));
dispatch(fetchXBlockIFrameHtmlAndResourcesQuery(id));
}, []);

const currentItemData = {
Expand Down
4 changes: 2 additions & 2 deletions src/course-unit/course-xblock/CourseXBlock.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -323,14 +323,14 @@ describe('<CourseXBlock />', () => {
const errorAlertDescription = renderErrorAlertMessages.alertRenderErrorDescription.defaultMessage;
const errorAlertMessage = renderErrorAlertMessages.alertRenderErrorMessage.defaultMessage
.replace('{message}', renderErrorMessage);
const contentIframe = queryByTestId('content-iframe-test-id');
const contentIFrame = queryByTestId('content-iframe-test-id');

expect(getByText(errorAlertTitle)).toBeInTheDocument();
expect(getByText(errorAlertDescription)).toBeInTheDocument();
expect(getByText(errorAlertMessage)).toBeInTheDocument();
expect(getByText(name)).toBeInTheDocument();
expect(getByLabelText(messages.blockAltButtonEdit.defaultMessage)).toBeInTheDocument();
expect(getByLabelText(messages.blockActionsDropdownAlt.defaultMessage)).toBeInTheDocument();
expect(contentIframe).not.toBeInTheDocument();
expect(contentIFrame).not.toBeInTheDocument();
});
});
10 changes: 0 additions & 10 deletions src/course-unit/course-xblock/utils.js

This file was deleted.

19 changes: 5 additions & 14 deletions src/course-unit/course-xblock/xblock-content/XBlockContent.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,13 @@ import { wrapBlockHtmlForIFrame } from './iframe-wrapper';

ensureConfig(['STUDIO_BASE_URL', 'SECURE_ORIGIN_XBLOCK_BOOTSTRAP_HTML_URL'], 'studio xblock component');

/**
* React component that displays an XBlock in a sandboxed IFrame.
*
* The IFrame is resized responsively so that it fits the content height.
*
* We use an IFrame so that the XBlock code, including user-authored HTML,
* cannot access things like the user's cookies, nor can it make GET/POST
* requests as the user. However, it is allowed to call any XBlock handlers.
*/
const XBlockContent = ({
view, type, getHandlerUrl, onBlockNotification,
}) => {
const iframeRef = useRef(null);
const [html, setHtml] = useState(null);
const [iFrameHeight, setIFrameHeight] = useState(0);
const [iframeKey, setIframeKey] = useState(0);
const [iframeHeight, setIFrameHeight] = useState(0);
const [iframeKey, setIFrameKey] = useState(0);
const [isLoading, setIsLoading] = useState(true);

useEffect(() => {
Expand All @@ -40,7 +31,7 @@ const XBlockContent = ({
// Load the XBlock HTML into the IFrame:
// iframe will only re-render in React when its property changes (key here)
setHtml(newHtml);
setIframeKey(prevKey => prevKey + 1);
setIFrameKey(prevKey => prevKey + 1);
}
};

Expand Down Expand Up @@ -75,7 +66,7 @@ const XBlockContent = ({
if (onBlockNotification) {
// This is a notification from the XBlock's frontend via 'runtime.notify(event, args)'
onBlockNotification({
eventType: method.substr(7), // Remove the 'xblock:' prefix that we added in iframe-wrapper.ts
eventType: method.substr('xblock'.length), // Remove the 'xblock:' prefix that we added in iframe-wrapper.ts
...args,
});
}
Expand Down Expand Up @@ -106,7 +97,7 @@ const XBlockContent = ({
</div>
)}
<div
style={{ height: `${iFrameHeight}px` }}
style={{ height: `${iframeHeight}px` }}
className="xblock-content"
>
<iframe
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export default function wrapBlockHtmlForIFrame(html, sourceResources, studioBase
<link rel="stylesheet" href="${studioBaseUrl}/static/studio/debug_toolbar/css/toolbar.css">
<link rel="stylesheet" href="${studioBaseUrl}/static/studio/css/vendor/html5-input-polyfills/number-polyfill.css">
<link rel="stylesheet" href="${studioBaseUrl}/static/studio/css/WordCloudBlockDisplay.css">
<link rel="stylesheet" href="XBlockIframe.css">
<link rel="stylesheet" href="XBlockIFrame.css">
<!-- JS scripts that can be used by XBlocks -->
<!-- gettext & XBlock JS i18n code -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,8 @@ export function xblockIFrameConnector() {

let lastHeight = -1;
function checkFrameHeight() {
const visibleIframeContent = document.querySelector('.xblock-render');
const newHeight = visibleIframeContent.scrollHeight;
const visibleIFrameContent = document.querySelector('.xblock-render');
const newHeight = visibleIFrameContent.scrollHeight;

if (newHeight !== lastHeight) {
postMessageToParent({ method: 'update_frame_height', height: newHeight });
Expand Down
8 changes: 7 additions & 1 deletion src/course-unit/data/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,15 @@ export async function setXBlockOrderList(blockId, children) {
* @param {string} itemId - The ID of the XBlock item.
* @returns {Promise<Object>} A Promise that resolves with the XBlock iframe data.
*/
export async function getXBlockIframeData(itemId) {
export async function getXBlockIFrameData(itemId) {
const { data } = await getAuthenticatedHttpClient()
.get(getXBlockContainerPreview(itemId));

return camelCaseObject(data);
}

export const getHandlerUrl = async (blockId) => {
const baseUrl = getConfig().STUDIO_BASE_URL;

return `${baseUrl}/xblock/${blockId}/handler/handler_name`;
};
2 changes: 1 addition & 1 deletion src/course-unit/data/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export const getCourseId = (state) => state.courseDetail.courseId;
export const getSequenceId = (state) => state.courseUnit.sequenceId;
export const getCourseVerticalChildren = (state) => state.courseUnit.courseVerticalChildren;
const getLoadingStatuses = (state) => state.courseUnit.loadingStatus;
export const getXBlockIframeHtmlAndResources = (state) => state.courseUnit.xblockIframeHtmlAndResources;
export const getXBlockIFrameHtmlAndResources = (state) => state.courseUnit.xblockIFrameHtmlAndResources;
export const getIsLoading = createSelector(
[getLoadingStatuses],
loadingStatus => Object.values(loadingStatus)
Expand Down
8 changes: 4 additions & 4 deletions src/course-unit/data/slice.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const slice = createSlice({
courseSectionVertical: {},
courseVerticalChildren: {},
staticFileNotices: {},
xblockIframeHtmlAndResources: [],
xblockIFrameHtmlAndResources: [],
},
reducers: {
fetchCourseItemSuccess: (state, { payload }) => {
Expand Down Expand Up @@ -112,8 +112,8 @@ const slice = createSlice({
// This avoids the need to copy the array beforehand
state.courseVerticalChildren.children.sort((a, b) => (indexMap.get(a.id) || 0) - (indexMap.get(b.id) || 0));
},
fetchXBlockIframeResources: (state, { payload }) => {
state.xblockIframeHtmlAndResources.push(payload);
fetchXBlockIFrameResources: (state, { payload }) => {
state.xblockIFrameHtmlAndResources.push(payload);
},
},
});
Expand All @@ -137,7 +137,7 @@ export const {
duplicateXBlock,
fetchStaticFileNoticesSuccess,
reorderXBlockList,
fetchXBlockIframeResources,
fetchXBlockIFrameResources,
} = slice.actions;

export const {
Expand Down
10 changes: 5 additions & 5 deletions src/course-unit/data/thunk.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
deleteUnitItem,
duplicateUnitItem,
setXBlockOrderList,
getXBlockIframeData,
getXBlockIFrameData,
} from './api';
import {
updateLoadingCourseUnitStatus,
Expand All @@ -38,7 +38,7 @@ import {
duplicateXBlock,
fetchStaticFileNoticesSuccess,
reorderXBlockList,
fetchXBlockIframeResources,
fetchXBlockIFrameResources,
} from './slice';
import { getNotificationMessage } from './utils';

Expand Down Expand Up @@ -270,14 +270,14 @@ export function setXBlockOrderListQuery(blockId, xblockListIds, restoreCallback)
};
}

export function fetchXBlockIframeHtmlAndResourcesQuery(xblockId) {
export function fetchXBlockIFrameHtmlAndResourcesQuery(xblockId) {
return async (dispatch) => {
dispatch(updateSavingStatus({ status: RequestStatus.PENDING }));
dispatch(showProcessingNotification(NOTIFICATION_MESSAGES.adding));

try {
const xblockIframeData = await getXBlockIframeData(xblockId);
dispatch(fetchXBlockIframeResources({ xblockId, ...xblockIframeData }));
const xblockIFrameData = await getXBlockIFrameData(xblockId);
dispatch(fetchXBlockIFrameResources({ xblockId, ...xblockIFrameData }));
dispatch(updateSavingStatus({ status: RequestStatus.SUCCESSFUL }));
} catch (error) {
dispatch(updateSavingStatus({ status: RequestStatus.FAILED }));
Expand Down
2 changes: 1 addition & 1 deletion webpack.dev.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ config.plugins.push(
},
{
context: path.resolve(__dirname, 'src/course-unit/course-xblock/xblock-content/iframe-wrapper/static'),
from: 'XBlockIframe.css',
from: 'XBlockIFrame.css',
},
],
}),
Expand Down
2 changes: 1 addition & 1 deletion webpack.prod.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ config.plugins.push(
},
{
context: path.resolve(__dirname, 'src/course-unit/course-xblock/xblock-content/iframe-wrapper/static'),
from: 'XBlockIframe.css',
from: 'XBlockIFrame.css',
},
],
}),
Expand Down

0 comments on commit 1f8eafc

Please sign in to comment.