Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 22 additions & 15 deletions src/content-tags-drawer/data/apiHooks.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ export const useContentData = (contentId) => (
*/
export const useContentTaxonomyTagsUpdater = (contentId) => {
const queryClient = useQueryClient();
const unitIframe = window.frames['xblock-iframe'];

return useMutation({
/**
Expand Down Expand Up @@ -169,26 +170,32 @@ export const useContentTaxonomyTagsUpdater = (contentId) => {

Comment thread
bradenmacdonald marked this conversation as resolved.
// Sends content tags.
getContentTaxonomyTagsData(contentId).then((data) => {
const contentData = {
contentId,
...data,
const contentData = { contentId, ...data };

const message = {
type: 'authoring.events.tags.updated',
data: contentData,
};
window.top?.postMessage(
{ type: 'authoring.events.tags.updated', data: contentData },
getConfig().STUDIO_BASE_URL,
);

const targetOrigin = getConfig().STUDIO_BASE_URL;

unitIframe?.postMessage(message, targetOrigin);
window.top?.postMessage(message, targetOrigin);
});

// Sends tags count.
getContentTaxonomyTagsCount(contentId).then((data) => {
const contentData = {
contentId,
count: data,
getContentTaxonomyTagsCount(contentId).then((count) => {
const contentData = { contentId, count };

const message = {
type: 'authoring.events.tags.count.updated',
data: contentData,
};
window.top?.postMessage(
{ type: 'authoring.events.tags.count.updated', data: contentData },
getConfig().STUDIO_BASE_URL,
);

const targetOrigin = getConfig().STUDIO_BASE_URL;

unitIframe?.postMessage(message, targetOrigin);
window.top?.postMessage(message, targetOrigin);
});
}
},
Expand Down
4 changes: 3 additions & 1 deletion src/course-unit/CourseUnit.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import Breadcrumbs from './breadcrumbs/Breadcrumbs';
import HeaderNavigations from './header-navigations/HeaderNavigations';
import Sequence from './course-sequence';
import Sidebar from './sidebar';
import { useCourseUnit, useLayoutGrid } from './hooks';
import { useCourseUnit, useLayoutGrid, useScrollToLastPosition } from './hooks';
import messages from './messages';
import PublishControls from './sidebar/PublishControls';
import LocationInfo from './sidebar/LocationInfo';
Expand Down Expand Up @@ -79,6 +79,8 @@ const CourseUnit = ({ courseId }) => {
document.title = getPageHeadTitle('', unitTitle);
}, [unitTitle]);

useScrollToLastPosition();

const {
isShow: isShowProcessingNotification,
title: processingNotificationTitle,
Expand Down
60 changes: 60 additions & 0 deletions src/course-unit/CourseUnit.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import sidebarMessages from './sidebar/messages';
import { extractCourseUnitId } from './sidebar/utils';
import CourseUnit from './CourseUnit';

import tagsDrawerMessages from '../content-tags-drawer/messages';
import { getClipboardUrl } from '../generic/data/api';
import configureModalMessages from '../generic/configure-modal/messages';
import { getContentTaxonomyTagsApiUrl, getContentTaxonomyTagsCountApiUrl } from '../content-tags-drawer/data/api';
Expand Down Expand Up @@ -105,6 +106,14 @@ jest.mock('@tanstack/react-query', () => ({
isSuccess: true,
};
}
if (queryKey[0] === 'taxonomies') {
return {
data: {
results: [],
},
isSuccess: true,
};
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally instead of mocking react-query we prefer to mock the API requests using the mocks defined in content-tags-drawer/data/api.mocks.ts.

So you can just write something like this at the top of your test file:

mockContentTaxonomyTagsData.applyMock();
mockTaxonomyListData.applyMock();
mockTaxonomyTagsData.applyMock();
mockContentData.applyMock();

If you do want/need to keep it this way though, you should at least replace the 'taxonomies' string with taxonomyQueryKeys.all from here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the option with mocks from content-tags-drawer the most, but unfortunately I couldn't run the tests successfully. Probably the problem is related to this mock @tanstack/react-query

Added taxonomyQueryKeys.all instead of the taxonomies line

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah those mocks won't work if you mock out react-query entirely.

@peterkulko peterkulko Feb 26, 2025 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added taxonomyQueryKeys.all instead of the taxonomies line

Do you mind if we stop here?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's fine. No need to rewrite it, but in the future be aware of the existing mocks we have (as well as everything in testUtils.tsx as they make writing new tests way simpler.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot!

return {
data: {},
isSuccess: true,
Expand Down Expand Up @@ -261,6 +270,19 @@ describe('<CourseUnit />', () => {
});
});

it('renders the xBlocks iframe and opens the tags drawer on postMessage event', async () => {
const { getByTitle, getByText } = render(<RootWrapper />);

await waitFor(() => {
const xblocksIframe = getByTitle(xblockContainerIframeMessages.xblockIframeTitle.defaultMessage);
expect(xblocksIframe).toBeInTheDocument();
});

simulatePostMessageEvent(messageTypes.openManageTags, { contentId: blockId });

expect(getByText(tagsDrawerMessages.headerSubtitle.defaultMessage)).toBeInTheDocument();
});

it('closes the legacy edit modal when closeXBlockEditorModal message is received', async () => {
const { getByTitle, queryByTitle } = render(<RootWrapper />);

Expand Down Expand Up @@ -750,6 +772,44 @@ describe('<CourseUnit />', () => {
)).toBeInTheDocument();
});

it('handle creating Text xblock and saves scroll position in localStorage', async () => {
const { getByText, getByRole } = render(<RootWrapper />);
const xblockType = 'text';

axiosMock
.onPost(postXBlockBaseApiUrl({ type: xblockType, category: 'html', parentLocator: blockId }))
.reply(200, courseCreateXblockMock);

window.scrollTo(0, 250);
Object.defineProperty(window, 'scrollY', { value: 250, configurable: true });

await waitFor(() => {
const textButton = screen.getByRole('button', { name: /Text/i });

expect(getByText(addComponentMessages.title.defaultMessage)).toBeInTheDocument();

userEvent.click(textButton);

const addXBlockDialog = getByRole('dialog');
expect(addXBlockDialog).toBeInTheDocument();

expect(getByText(
addComponentMessages.modalContainerTitle.defaultMessage.replace('{componentTitle}', xblockType),
)).toBeInTheDocument();

const textRadio = screen.getByRole('radio', { name: /Text/i });
userEvent.click(textRadio);
expect(textRadio).toBeChecked();

const selectBtn = getByRole('button', { name: addComponentMessages.modalBtnText.defaultMessage });
expect(selectBtn).toBeInTheDocument();

userEvent.click(selectBtn);
});

expect(localStorage.getItem('createXBlockLastYPosition')).toBe('250');
});

it('correct addition of a new course unit after click on the "Add new unit" button', async () => {
const { getByRole, getAllByTestId } = render(<RootWrapper />);
let units = null;
Expand Down
3 changes: 2 additions & 1 deletion src/course-unit/add-component/AddComponent.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +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);
localStorage.setItem('createXBlockLastYPosition', window.scrollY);
navigate(`/course/${courseKey}/editor/${type}/${locator}`);
});
break;
Expand Down Expand Up @@ -92,6 +92,7 @@ const AddComponent = ({ blockId, handleCreateNewCourseXBlock }) => {
boilerplate: moduleName,
parentLocator: blockId,
}, ({ courseKey, locator }) => {
localStorage.setItem('createXBlockLastYPosition', window.scrollY);
navigate(`/course/${courseKey}/editor/html/${locator}`);
});
break;
Expand Down
1 change: 1 addition & 0 deletions src/course-unit/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,5 @@ export const messageTypes = {
completeXBlockEditing: 'completeXBlockEditing',
studioAjaxError: 'studioAjaxError',
refreshPositions: 'refreshPositions',
openManageTags: 'openManageTags',
};
1 change: 0 additions & 1 deletion src/course-unit/context/hooks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { useContext } from 'react';

import { IframeContext, IframeContextType } from './iFrameContext';

// eslint-disable-next-line import/prefer-default-export
export const useIframe = (): IframeContextType => {
const context = useContext(IframeContext);
if (!context) {
Expand Down
59 changes: 58 additions & 1 deletion src/course-unit/hooks.jsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { useCallback, useEffect, useMemo } from 'react';
import {
useCallback, useEffect, useMemo, useRef, useState,
} from 'react';
import { useDispatch, useSelector } from 'react-redux';
import { useNavigate, useSearchParams } from 'react-router-dom';
import { useToggle } from '@openedx/paragon';
Expand Down Expand Up @@ -259,3 +261,58 @@ export const useLayoutGrid = (unitCategory, isUnitLibraryType) => (
return isUnitLibraryType ? layouts.fullWidth : layouts.default;
}, [unitCategory])
);

/**
* Custom hook that restores the scroll position from `localStorage` after a page reload.
* It listens for a `plugin.resize` message event and scrolls the window to the saved position
* after a 1-second delay, provided no new resize messages are received during that time.
*
* @param {string} [storageKey='createXBlockLastYPosition'] -
* The key used to store the last scroll position in `localStorage`.
*/
export const useScrollToLastPosition = (storageKey = 'createXBlockLastYPosition') => {
const timeoutRef = useRef(null);
const [hasLastPosition, setHasLastPosition] = useState(() => !!localStorage.getItem(storageKey));

const scrollToLastPosition = useCallback(() => {
const lastYPosition = localStorage.getItem(storageKey);
if (!lastYPosition) {
setHasLastPosition(false);
return;
}

const yPosition = parseInt(lastYPosition, 10);
if (!Number.isNaN(yPosition)) {
window.scrollTo({ top: yPosition, behavior: 'smooth' });
localStorage.removeItem(storageKey);
setHasLastPosition(false);
}
}, [storageKey]);

const handleMessage = useCallback((event) => {
if (event.data?.type === messageTypes.resize) {
if (timeoutRef.current) {
clearTimeout(timeoutRef.current);
}

timeoutRef.current = setTimeout(scrollToLastPosition, 1000);
}
}, [scrollToLastPosition]);

useEffect(() => {
if (!hasLastPosition) {
return undefined;
}

window.addEventListener('message', handleMessage);

return () => {
window.removeEventListener('message', handleMessage);
if (timeoutRef.current) {
clearTimeout(timeoutRef.current);
}
};
}, [hasLastPosition, handleMessage]);

return null;
};
Loading