fix: prevent editor from loading twice before initialization (#1761)
Data from previous editor instance was being processed by current editor instance and sometimes failed due to mismatch. For example, editing text editor or any other basic editor after opening an advanced problem like drag-n-drop crashed. Now the editor is only rendered after the initialization process is complete.
This commit is contained in:
@@ -29,7 +29,7 @@ const Editor: React.FC<Props> = ({
|
||||
returnFunction = null,
|
||||
}) => {
|
||||
const dispatch = useDispatch();
|
||||
hooks.initializeApp({
|
||||
const loading = hooks.useInitializeApp({
|
||||
dispatch,
|
||||
data: {
|
||||
blockId,
|
||||
@@ -43,6 +43,11 @@ const Editor: React.FC<Props> = ({
|
||||
|
||||
const EditorComponent = supportedEditors[blockType];
|
||||
|
||||
// Do not load editor until everything is initialized.
|
||||
if (loading) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (EditorComponent === undefined && blockId) {
|
||||
return (
|
||||
<AdvancedEditor
|
||||
|
||||
@@ -11,7 +11,7 @@ const VideoSelector = ({
|
||||
studioEndpointUrl,
|
||||
}) => {
|
||||
const dispatch = useDispatch();
|
||||
hooks.initializeApp({
|
||||
const loading = hooks.useInitializeApp({
|
||||
dispatch,
|
||||
data: {
|
||||
blockId,
|
||||
@@ -21,6 +21,10 @@ const VideoSelector = ({
|
||||
studioEndpointUrl,
|
||||
},
|
||||
});
|
||||
// istanbul ignore if
|
||||
if (loading) {
|
||||
return null;
|
||||
}
|
||||
return (
|
||||
<VideoGallery />
|
||||
);
|
||||
|
||||
@@ -6,7 +6,7 @@ import * as hooks from './hooks';
|
||||
import VideoSelector from './VideoSelector';
|
||||
|
||||
jest.mock('./hooks', () => ({
|
||||
initializeApp: jest.fn(),
|
||||
useInitializeApp: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('./containers/VideoGallery', () => 'VideoGallery');
|
||||
@@ -32,7 +32,7 @@ describe('Video Selector', () => {
|
||||
describe('behavior', () => {
|
||||
it('calls initializeApp hook with dispatch, and passed data', () => {
|
||||
shallow(<VideoSelector {...props} />);
|
||||
expect(hooks.initializeApp).toHaveBeenCalledWith({
|
||||
expect(hooks.useInitializeApp).toHaveBeenCalledWith({
|
||||
dispatch: useDispatch(),
|
||||
data: initData,
|
||||
});
|
||||
|
||||
@@ -12,6 +12,7 @@ jest.mock('react', () => ({
|
||||
...jest.requireActual('react'),
|
||||
useRef: jest.fn(val => ({ current: val })),
|
||||
useEffect: jest.fn(),
|
||||
useState: jest.fn(() => [false, jest.fn()]),
|
||||
useCallback: (cb, prereqs) => ({ cb, prereqs }),
|
||||
}));
|
||||
|
||||
@@ -56,7 +57,7 @@ describe('hooks', () => {
|
||||
studioEndpointUrl: 'studioEndpointUrl',
|
||||
learningContextId: 'learningContextId',
|
||||
};
|
||||
hooks.initializeApp({ dispatch, data: fakeData });
|
||||
hooks.useInitializeApp({ dispatch, data: fakeData });
|
||||
expect(dispatch).not.toHaveBeenCalledWith(fakeData);
|
||||
const [cb, prereqs] = useEffect.mock.calls[0];
|
||||
expect(prereqs).toStrictEqual([
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useEffect } from 'react';
|
||||
import { useEffect, useState } from 'react';
|
||||
|
||||
import { sendTrackEvent } from '@edx/frontend-platform/analytics';
|
||||
import analyticsEvt from './data/constants/analyticsEvt';
|
||||
@@ -6,11 +6,15 @@ import analyticsEvt from './data/constants/analyticsEvt';
|
||||
import { actions, thunkActions } from './data/redux';
|
||||
import { RequestKeys } from './data/constants/requests';
|
||||
|
||||
// eslint-disable-next-line react-hooks/rules-of-hooks
|
||||
export const initializeApp = ({ dispatch, data }) => useEffect(
|
||||
() => dispatch(thunkActions.app.initialize(data)),
|
||||
[data?.blockId, data?.studioEndpointUrl, data?.learningContextId],
|
||||
);
|
||||
export const useInitializeApp = ({ dispatch, data }) => {
|
||||
const [loading, setLoading] = useState(true);
|
||||
useEffect(() => {
|
||||
setLoading(true);
|
||||
dispatch(thunkActions.app.initialize(data));
|
||||
setLoading(false);
|
||||
}, [data?.blockId, data?.studioEndpointUrl, data?.learningContextId]);
|
||||
return loading;
|
||||
};
|
||||
|
||||
export const navigateTo = (destination: string | URL) => {
|
||||
window.location.assign(destination);
|
||||
|
||||
Reference in New Issue
Block a user