diff --git a/package-lock.json b/package-lock.json index 2bbd4d278..8040ece19 100644 --- a/package-lock.json +++ b/package-lock.json @@ -64,8 +64,8 @@ "react-onclickoutside": "^6.13.0", "react-redux": "7.2.9", "react-responsive": "9.0.2", - "react-router": "6.23.1", - "react-router-dom": "6.23.1", + "react-router": "6.27.0", + "react-router-dom": "6.27.0", "react-select": "5.8.0", "react-textarea-autosize": "^8.5.3", "react-transition-group": "4.4.5", @@ -4275,7 +4275,9 @@ } }, "node_modules/@remix-run/router": { - "version": "1.16.1", + "version": "1.20.0", + "resolved": "https://registry.npmjs.org/@remix-run/router/-/router-1.20.0.tgz", + "integrity": "sha512-mUnk8rPJBI9loFDZ+YzPGdeniYK+FTmRD1TMCz7ev2SNIozyKKpnGgsxO34u6Z4z/t0ITuu7voi/AshfsGsgFg==", "license": "MIT", "engines": { "node": ">=14.0.0" @@ -17514,10 +17516,12 @@ } }, "node_modules/react-router": { - "version": "6.23.1", + "version": "6.27.0", + "resolved": "https://registry.npmjs.org/react-router/-/react-router-6.27.0.tgz", + "integrity": "sha512-YA+HGZXz4jaAkVoYBE98VQl+nVzI+cVI2Oj/06F5ZM+0u3TgedN9Y9kmMRo2mnkSK2nCpNQn0DVob4HCsY/WLw==", "license": "MIT", "dependencies": { - "@remix-run/router": "1.16.1" + "@remix-run/router": "1.20.0" }, "engines": { "node": ">=14.0.0" @@ -17527,11 +17531,13 @@ } }, "node_modules/react-router-dom": { - "version": "6.23.1", + "version": "6.27.0", + "resolved": "https://registry.npmjs.org/react-router-dom/-/react-router-dom-6.27.0.tgz", + "integrity": "sha512-+bvtFWMC0DgAFrfKXKG9Fc+BcXWRUO1aJIihbB79xaeq0v5UzfvnM5houGUm1Y461WVRcgAQ+Clh5rdb1eCx4g==", "license": "MIT", "dependencies": { - "@remix-run/router": "1.16.1", - "react-router": "6.23.1" + "@remix-run/router": "1.20.0", + "react-router": "6.27.0" }, "engines": { "node": ">=14.0.0" diff --git a/package.json b/package.json index 8fa50f78e..57ac9dfca 100644 --- a/package.json +++ b/package.json @@ -93,8 +93,8 @@ "react-onclickoutside": "^6.13.0", "react-redux": "7.2.9", "react-responsive": "9.0.2", - "react-router": "6.23.1", - "react-router-dom": "6.23.1", + "react-router": "6.27.0", + "react-router-dom": "6.27.0", "react-select": "5.8.0", "react-textarea-autosize": "^8.5.3", "react-transition-group": "4.4.5", diff --git a/src/course-unit/course-xblock/CourseXBlock.jsx b/src/course-unit/course-xblock/CourseXBlock.jsx index 2d8f6221e..89a13ece7 100644 --- a/src/course-unit/course-xblock/CourseXBlock.jsx +++ b/src/course-unit/course-xblock/CourseXBlock.jsx @@ -7,7 +7,7 @@ import { } from '@openedx/paragon'; import { EditOutline as EditIcon, MoreVert as MoveVertIcon } from '@openedx/paragon/icons'; import { useIntl } from '@edx/frontend-platform/i18n'; -import { useNavigate, useSearchParams } from 'react-router-dom'; +import { useSearchParams } from 'react-router-dom'; import { getCanEdit, getCourseId } from 'CourseAuthoring/course-unit/data/selectors'; import DeleteModal from '../../generic/delete-modal/DeleteModal'; @@ -19,6 +19,7 @@ import { copyToClipboard } from '../../generic/data/thunks'; import { COMPONENT_TYPES } from '../../generic/block-type-utils/constants'; import XBlockMessages from './xblock-messages/XBlockMessages'; import messages from './messages'; +import { createCorrectInternalRoute } from '../../utils'; const CourseXBlock = ({ id, title, type, unitXBlockActions, shouldScroll, userPartitionInfo, @@ -28,7 +29,6 @@ const CourseXBlock = ({ const [isDeleteModalOpen, openDeleteModal, closeDeleteModal] = useToggle(false); const [isConfigureModalOpen, openConfigureModal, closeConfigureModal] = useToggle(false); const dispatch = useDispatch(); - const navigate = useNavigate(); const canEdit = useSelector(getCanEdit); const courseId = useSelector(getCourseId); const intl = useIntl(); @@ -58,7 +58,11 @@ const CourseXBlock = ({ case COMPONENT_TYPES.html: case COMPONENT_TYPES.problem: case COMPONENT_TYPES.video: - navigate(`/course/${courseId}/editor/${type}/${id}`); + // Not using useNavigate from react router to use browser navigation + // which allows us to block back button if unsaved changes in editor are present. + window.location.assign( + createCorrectInternalRoute(`/course/${courseId}/editor/${type}/${id}`), + ); break; default: } diff --git a/src/course-unit/course-xblock/CourseXBlock.test.jsx b/src/course-unit/course-xblock/CourseXBlock.test.jsx index 0cdf05d4f..95482d098 100644 --- a/src/course-unit/course-xblock/CourseXBlock.test.jsx +++ b/src/course-unit/course-xblock/CourseXBlock.test.jsx @@ -29,7 +29,6 @@ const blockId = '567890'; const handleDeleteMock = jest.fn(); const handleDuplicateMock = jest.fn(); const handleConfigureSubmitMock = jest.fn(); -const mockedUsedNavigate = jest.fn(); const { name, block_id: id, @@ -42,11 +41,6 @@ const unitXBlockActionsMock = { handleDuplicate: handleDuplicateMock, }; -jest.mock('react-router-dom', () => ({ - ...jest.requireActual('react-router-dom'), - useNavigate: () => mockedUsedNavigate, -})); - jest.mock('react-redux', () => ({ ...jest.requireActual('react-redux'), useSelector: jest.fn(), @@ -78,6 +72,16 @@ useSelector.mockImplementation((selector) => { }); describe('', () => { + const locationTemp = window.location; + beforeAll(() => { + delete window.location; + window.location = { + assign: jest.fn(), + }; + }); + afterAll(() => { + window.location = locationTemp; + }); beforeEach(async () => { initializeMockApp({ authenticatedUser: { @@ -168,8 +172,8 @@ describe('', () => { expect(editButton).toBeInTheDocument(); userEvent.click(editButton); - expect(mockedUsedNavigate).toHaveBeenCalled(); - expect(mockedUsedNavigate).toHaveBeenCalledWith(`/course/${courseId}/editor/html/${id}`); + expect(window.location.assign).toHaveBeenCalled(); + expect(window.location.assign).toHaveBeenCalledWith(`/course/${courseId}/editor/html/${id}`); }); it('navigates to editor page on edit Video xblock', () => { @@ -182,8 +186,8 @@ describe('', () => { expect(editButton).toBeInTheDocument(); userEvent.click(editButton); - expect(mockedUsedNavigate).toHaveBeenCalled(); - expect(mockedUsedNavigate).toHaveBeenCalledWith(`/course/${courseId}/editor/video/${id}`); + expect(window.location.assign).toHaveBeenCalled(); + expect(window.location.assign).toHaveBeenCalledWith(`/course/${courseId}/editor/video/${id}`); }); it('navigates to editor page on edit Problem xblock', () => { @@ -196,8 +200,8 @@ describe('', () => { expect(editButton).toBeInTheDocument(); userEvent.click(editButton); - expect(mockedUsedNavigate).toHaveBeenCalled(); - expect(mockedUsedNavigate).toHaveBeenCalledWith(`/course/${courseId}/editor/problem/${id}`); + expect(window.location.assign).toHaveBeenCalled(); + expect(window.location.assign).toHaveBeenCalledWith(`/course/${courseId}/editor/problem/${id}`); expect(handleDeleteMock).toHaveBeenCalledWith(id); }); }); diff --git a/src/editors/containers/EditorContainer/index.test.tsx b/src/editors/containers/EditorContainer/index.test.tsx index 2d6304134..3f427bbeb 100644 --- a/src/editors/containers/EditorContainer/index.test.tsx +++ b/src/editors/containers/EditorContainer/index.test.tsx @@ -29,6 +29,12 @@ jest.spyOn(editorCmsApi, 'fetchByUnitId').mockImplementation(async () => ({ }, })); +const isDirtyMock = jest.fn(); +jest.mock('../TextEditor/hooks', () => ({ + ...jest.requireActual('../TextEditor/hooks'), + isDirty: () => isDirtyMock, +})); + const defaultPropsHtml = { blockId: 'block-v1:Org+TS100+24+type@html+block@123456html', blockType: 'html', @@ -45,15 +51,27 @@ const fieldsHtml = { }; describe('EditorContainer', () => { + let mockEvent: Event; + beforeEach(() => { initializeMocks(); + mockEvent = new Event('beforeunload'); + jest.spyOn(window, 'addEventListener'); + jest.spyOn(window, 'removeEventListener'); + jest.spyOn(mockEvent, 'preventDefault'); + Object.defineProperty(mockEvent, 'returnValue', { writable: true }); }); - test('it displays a confirmation dialog when closing the editor modal', async () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + test('it displays a confirmation dialog when closing the editor modal if data is changed', async () => { jest.spyOn(editorCmsApi, 'fetchBlockById').mockImplementationOnce(async () => ( { status: 200, data: snakeCaseObject(fieldsHtml) } )); + isDirtyMock.mockReturnValue(true); render(); // Then the editor should open @@ -68,12 +86,48 @@ describe('EditorContainer', () => { fireEvent.click(closeButton); // Now we should see the confirmation message: expect(await screen.findByText(confirmMessage)).toBeInTheDocument(); - expect(defaultPropsHtml.onClose).not.toHaveBeenCalled(); + + // Should close modal if cancelled + const cancelBtn = await screen.findByRole('button', { name: 'Cancel' }); + fireEvent.click(cancelBtn); + expect(defaultPropsHtml.onClose).not.toHaveBeenCalled(); + + // open modal again + fireEvent.click(closeButton); // And can confirm the cancelation: const confirmButton = await screen.findByRole('button', { name: 'OK' }); fireEvent.click(confirmButton); expect(defaultPropsHtml.onClose).toHaveBeenCalled(); + window.dispatchEvent(mockEvent); + // should not be blocked by beforeunload event as the page was unloaded using close/cancel option + expect(window.removeEventListener).toHaveBeenCalledWith('beforeunload', expect.any(Function)); + expect(mockEvent.preventDefault).not.toHaveBeenCalled(); + }); + + test('it does not display any confirmation dialog when closing the editor modal if data is not changed', async () => { + jest.spyOn(editorCmsApi, 'fetchBlockById').mockImplementationOnce(async () => ( + { status: 200, data: snakeCaseObject(fieldsHtml) } + )); + + isDirtyMock.mockReturnValue(false); + render(); + + // Then the editor should open + expect(await screen.findByRole('heading', { name: /Introduction to Testing/ })).toBeInTheDocument(); + + // Assert the "are you sure?" message isn't visible yet + const confirmMessage = /Are you sure you want to exit the editor/; + expect(screen.queryByText(confirmMessage)).not.toBeInTheDocument(); + + // Find and click the close button + const closeButton = await screen.findByRole('button', { name: 'Exit the editor' }); + fireEvent.click(closeButton); + // Even now we should not see the confirmation message as data is not dirty, i.e. not changed: + expect(screen.queryByText(confirmMessage)).not.toBeInTheDocument(); + + // And onClose is directly called + expect(defaultPropsHtml.onClose).toHaveBeenCalled(); }); test('it disables the save button until the fields have been loaded', async () => { @@ -94,4 +148,21 @@ describe('EditorContainer', () => { // Now the save button should be active: await waitFor(() => expect(saveButton).not.toBeDisabled()); }); + + test('beforeunload event is triggered on page unload if data is changed', async () => { + jest.spyOn(editorCmsApi, 'fetchBlockById').mockImplementationOnce(async () => ( + { status: 200, data: snakeCaseObject(fieldsHtml) } + )); + + isDirtyMock.mockReturnValue(true); + render(); + + // Then the editor should open + expect(await screen.findByRole('heading', { name: /Introduction to Testing/ })).toBeInTheDocument(); + // on beforeunload event block user + window.dispatchEvent(mockEvent); + expect(window.removeEventListener).toHaveBeenCalledWith('beforeunload', expect.any(Function)); + expect(mockEvent.preventDefault).toHaveBeenCalled(); + expect(mockEvent.returnValue).toBe(true); + }); }); diff --git a/src/editors/containers/EditorContainer/index.tsx b/src/editors/containers/EditorContainer/index.tsx index 3414680b5..eb9e08ab2 100644 --- a/src/editors/containers/EditorContainer/index.tsx +++ b/src/editors/containers/EditorContainer/index.tsx @@ -20,6 +20,7 @@ import TitleHeader from './components/TitleHeader'; import * as hooks from './hooks'; import messages from './messages'; import './index.scss'; +import usePromptIfDirty from '../../../generic/promptIfDirty/usePromptIfDirty'; interface WrapperProps { children: React.ReactNode; @@ -61,32 +62,57 @@ export const FooterWrapper: React.FC = ({ children }) => { interface Props extends EditorComponent { children: React.ReactNode; getContent: Function; + isDirty: () => boolean; validateEntry?: Function | null; } const EditorContainer: React.FC = ({ children, getContent, + isDirty, onClose = null, validateEntry = null, returnFunction = null, }) => { const intl = useIntl(); const dispatch = useDispatch(); + // Required to mark data as not dirty on save + const [saved, setSaved] = React.useState(false); const isInitialized = hooks.isInitialized(); const { isCancelConfirmOpen, openCancelConfirmModal, closeCancelConfirmModal } = hooks.cancelConfirmModalToggle(); const handleCancel = hooks.handleCancel({ onClose, returnFunction }); const disableSave = !isInitialized; const saveFailed = hooks.saveFailed(); const clearSaveFailed = hooks.clearSaveError({ dispatch }); - const onSave = hooks.handleSaveClicked({ + const handleSave = hooks.handleSaveClicked({ dispatch, getContent, validateEntry, returnFunction, }); + + const onSave = () => { + setSaved(true); + handleSave(); + }; + // Stops user from navigating away if they have unsaved changes. + usePromptIfDirty(() => { + // Do not block if cancel modal is used or data is saved. + if (isCancelConfirmOpen || saved) { + return false; + } + return isDirty(); + }); + + const confirmCancelIfDirty = () => { + if (isDirty()) { + openCancelConfirmModal(); + } else { + handleCancel(); + } + }; return ( - + {saveFailed && ( @@ -108,7 +134,9 @@ const EditorContainer: React.FC = ({ )} isOpen={isCancelConfirmOpen} - close={closeCancelConfirmModal} + close={() => { + closeCancelConfirmModal(); + }} title={intl.formatMessage(messages.cancelConfirmTitle)} > @@ -121,7 +149,7 @@ const EditorContainer: React.FC = ({ @@ -135,7 +163,7 @@ const EditorContainer: React.FC = ({ diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/__snapshots__/index.test.jsx.snap b/src/editors/containers/ProblemEditor/components/EditProblemView/__snapshots__/index.test.jsx.snap index 428feabce..987a73112 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/__snapshots__/index.test.jsx.snap +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/__snapshots__/index.test.jsx.snap @@ -3,6 +3,7 @@ exports[`EditorProblemView component renders raw editor 1`] = ` { }; }; +/** Checks if any tinymce editor in window is dirty */ +export const checkIfEditorsDirty = () => { + const EditorsArray = window.tinymce.editors; + return Object.entries(EditorsArray).some(([id, editor]) => { + if (Number.isNaN(parseInt(id, 10))) { + if (!editor.isNotDirty) { + return true; + } + } + return false; + }); +}; + export const fetchEditorContent = ({ format }) => { const editorObject = { hints: [] }; const EditorsArray = window.tinymce.editors; diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/hooks.test.js b/src/editors/containers/ProblemEditor/components/EditProblemView/hooks.test.js index 11f38473b..6fa704fc3 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/hooks.test.js +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/hooks.test.js @@ -362,3 +362,43 @@ describe('EditProblemView hooks parseState', () => { }); }); }); + +describe('checkIfEditorsDirty', () => { + let windowSpy; + beforeEach(() => { + windowSpy = jest.spyOn(window, 'window', 'get'); + }); + afterEach(() => { + windowSpy.mockRestore(); + }); + describe('state hook', () => { + test('should return false if none of editors are dirty', () => { + windowSpy.mockImplementation(() => ({ + tinymce: { + editors: { + some_id: { isNotDirty: true }, + some_id2: { isNotDirty: true }, + some_id3: { isNotDirty: true }, + some_id4: { isNotDirty: true }, + some_id5: { isNotDirty: true }, + }, + }, + })); + expect(hooks.checkIfEditorsDirty()).toEqual(false); + }); + test('should return true if any editor is dirty', () => { + windowSpy.mockImplementation(() => ({ + tinymce: { + editors: { + some_id: { isNotDirty: true }, + some_id2: { isNotDirty: true }, + some_id3: { isNotDirty: false }, + some_id4: { isNotDirty: true }, + some_id5: { isNotDirty: false }, + }, + }, + })); + expect(hooks.checkIfEditorsDirty()).toEqual(true); + }); + }); +}); diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/index.jsx b/src/editors/containers/ProblemEditor/components/EditProblemView/index.jsx index 64d922906..fbeb086b2 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/index.jsx +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/index.jsx @@ -17,7 +17,9 @@ import { selectors } from '../../../../data/redux'; import RawEditor from '../../../../sharedComponents/RawEditor'; import { ProblemTypeKeys } from '../../../../data/constants/problem'; -import { parseState, saveWarningModalToggle, getContent } from './hooks'; +import { + checkIfEditorsDirty, parseState, saveWarningModalToggle, getContent, +} from './hooks'; import './index.scss'; import messages from './messages'; @@ -32,6 +34,7 @@ const EditProblemView = ({ lmsEndpointUrl, returnUrl, analytics, + isDirty, // injected intl, }) => { @@ -40,6 +43,14 @@ const EditProblemView = ({ const isAdvancedProblemType = problemType === ProblemTypeKeys.ADVANCED; const { isSaveWarningModalOpen, openSaveWarningModal, closeSaveWarningModal } = saveWarningModalToggle(); + const checkIfDirty = () => { + if (isAdvancedProblemType && editorRef && editorRef?.current) { + /* istanbul ignore next */ + return editorRef.current.observer?.lastChange !== 0; + } + return isDirty || checkIfEditorsDirty(); + }; + return ( getContent({ @@ -49,6 +60,7 @@ const EditProblemView = ({ editorRef, lmsEndpointUrl, })} + isDirty={checkIfDirty} returnFunction={returnFunction} > ({ returnUrl: selectors.app.returnUrl(state), problemType: selectors.problem.problemType(state), problemState: selectors.problem.completeState(state), + isDirty: selectors.problem.isDirty(state), }); export const EditProblemViewInternal = EditProblemView; // For testing only diff --git a/src/editors/containers/TextEditor/__snapshots__/index.test.jsx.snap b/src/editors/containers/TextEditor/__snapshots__/index.test.jsx.snap index 32b7e2030..b79edd040 100644 --- a/src/editors/containers/TextEditor/__snapshots__/index.test.jsx.snap +++ b/src/editors/containers/TextEditor/__snapshots__/index.test.jsx.snap @@ -14,6 +14,18 @@ exports[`TextEditor snapshots block failed to load, Toast is shown 1`] = ` }, } } + isDirty={ + { + "isDirty": { + "editorRef": { + "current": { + "value": "something", + }, + }, + "showRawEditor": false, + }, + } + } onClose={[MockFunction props.onClose]} returnFunction={null} > @@ -67,6 +79,18 @@ exports[`TextEditor snapshots loaded, raw editor 1`] = ` }, } } + isDirty={ + { + "isDirty": { + "editorRef": { + "current": { + "value": "something", + }, + }, + "showRawEditor": true, + }, + } + } onClose={[MockFunction props.onClose]} returnFunction={null} > @@ -114,6 +138,18 @@ exports[`TextEditor snapshots not yet loaded, Spinner appears 1`] = ` }, } } + isDirty={ + { + "isDirty": { + "editorRef": { + "current": { + "value": "something", + }, + }, + "showRawEditor": false, + }, + } + } onClose={[MockFunction props.onClose]} returnFunction={null} > @@ -153,6 +189,18 @@ exports[`TextEditor snapshots renders as expected with default behavior 1`] = ` }, } } + isDirty={ + { + "isDirty": { + "editorRef": { + "current": { + "value": "something", + }, + }, + "showRawEditor": false, + }, + } + } onClose={[MockFunction props.onClose]} returnFunction={null} > @@ -206,6 +254,18 @@ exports[`TextEditor snapshots renders static images with relative paths 1`] = ` }, } } + isDirty={ + { + "isDirty": { + "editorRef": { + "current": { + "value": "something", + }, + }, + "showRawEditor": false, + }, + } + } onClose={[MockFunction props.onClose]} returnFunction={null} > diff --git a/src/editors/containers/TextEditor/hooks.js b/src/editors/containers/TextEditor/hooks.js index 3d725f114..6271776f0 100644 --- a/src/editors/containers/TextEditor/hooks.js +++ b/src/editors/containers/TextEditor/hooks.js @@ -9,3 +9,14 @@ export const getContent = ({ editorRef, showRawEditor }) => () => { : editorRef.current?.getContent()); return setAssetToStaticUrl({ editorValue: content }); }; + +export const isDirty = ({ editorRef, showRawEditor }) => () => { + /* istanbul ignore next */ + if (!editorRef?.current) { + return false; + } + const dirty = (showRawEditor && editorRef && editorRef.current + ? editorRef.current.observer?.lastChange !== 0 + : !editorRef.current.isNotDirty); + return dirty; +}; diff --git a/src/editors/containers/TextEditor/hooks.test.jsx b/src/editors/containers/TextEditor/hooks.test.jsx index bac63ab33..c12b2e271 100644 --- a/src/editors/containers/TextEditor/hooks.test.jsx +++ b/src/editors/containers/TextEditor/hooks.test.jsx @@ -61,5 +61,26 @@ describe('TextEditor hooks', () => { expect(getContent).toEqual(rawContent); }); }); + + describe('isDirty', () => { + test('checks isNotDirty flag when showRawEditor is false', () => { + const editorRef = { + current: { + isNotDirty: false, + }, + }; + const isDirty = module.isDirty({ editorRef, showRawEditor: false })(); + expect(isDirty).toEqual(true); + }); + test('checks observer.lastChange flag when showRawEditor is true', () => { + const editorRef = { + current: { + observer: { lastChange: 123 }, + }, + }; + const isDirty = module.isDirty({ editorRef, showRawEditor: true })(); + expect(isDirty).toEqual(true); + }); + }); }); }); diff --git a/src/editors/containers/TextEditor/index.jsx b/src/editors/containers/TextEditor/index.jsx index b3368152b..0546e65b0 100644 --- a/src/editors/containers/TextEditor/index.jsx +++ b/src/editors/containers/TextEditor/index.jsx @@ -80,6 +80,7 @@ const TextEditor = ({ return ( diff --git a/src/editors/containers/TextEditor/index.test.jsx b/src/editors/containers/TextEditor/index.test.jsx index 69752df78..ea3bffc94 100644 --- a/src/editors/containers/TextEditor/index.test.jsx +++ b/src/editors/containers/TextEditor/index.test.jsx @@ -22,6 +22,7 @@ jest.mock('../EditorContainer', () => 'EditorContainer'); jest.mock('./hooks', () => ({ getContent: jest.fn(args => ({ getContent: args })), + isDirty: jest.fn(args => ({ isDirty: args })), nullMethod: jest.fn().mockName('hooks.nullMethod'), })); diff --git a/src/editors/containers/VideoEditor/__snapshots__/index.test.tsx.snap b/src/editors/containers/VideoEditor/__snapshots__/index.test.tsx.snap index 0c21226d4..4f5e00fd4 100644 --- a/src/editors/containers/VideoEditor/__snapshots__/index.test.tsx.snap +++ b/src/editors/containers/VideoEditor/__snapshots__/index.test.tsx.snap @@ -5,6 +5,7 @@ exports[`VideoEditor snapshots renders as expected with default behavior 1`] = ` value="hooks.errorsHook.error" > diff --git a/src/editors/containers/VideoEditor/index.tsx b/src/editors/containers/VideoEditor/index.tsx index 1cc717935..f2ad7c670 100644 --- a/src/editors/containers/VideoEditor/index.tsx +++ b/src/editors/containers/VideoEditor/index.tsx @@ -31,6 +31,7 @@ const VideoEditor: React.FC = ({ true} onClose={onClose} returnFunction={returnFunction} validateEntry={validateEntry} diff --git a/src/editors/data/redux/problem/reducers.js b/src/editors/data/redux/problem/reducers.js index 034f1bbb3..3a0a76281 100644 --- a/src/editors/data/redux/problem/reducers.js +++ b/src/editors/data/redux/problem/reducers.js @@ -16,6 +16,7 @@ const initialState = { generalFeedback: '', additionalAttributes: {}, defaultSettings: {}, + isDirty: false, settings: { randomization: null, scoring: { @@ -52,6 +53,7 @@ const problem = createSlice({ updateQuestion: (state, { payload }) => ({ ...state, question: payload, + isDirty: true, }), updateAnswer: (state, { payload }) => { const { id, hasSingleAnswer, ...answer } = payload; @@ -77,6 +79,7 @@ const problem = createSlice({ ...state, correctAnswerCount, answers, + isDirty: true, }; }, deleteAnswer: (state, { payload }) => { @@ -86,6 +89,7 @@ const problem = createSlice({ return { ...state, correctAnswerCount: state.problemType === ProblemTypeKeys.NUMERIC ? 1 : 0, + isDirty: true, answers: [{ id: 'A', title: '', @@ -140,6 +144,7 @@ const problem = createSlice({ answers, correctAnswerCount: correct ? state.correctAnswerCount - 1 : state.correctAnswerCount, groupFeedbackList, + isDirty: true, }; }, addAnswer: (state) => { @@ -167,6 +172,7 @@ const problem = createSlice({ return { ...state, correctAnswerCount, + isDirty: true, answers, }; }, @@ -185,6 +191,7 @@ const problem = createSlice({ ...state, correctAnswerCount, answers: [newOption], + isDirty: true, }; }, @@ -194,6 +201,7 @@ const problem = createSlice({ ...state.settings, ...payload, }, + isDirty: true, }), load: (state, { payload: { settings: { scoring, showAnswer, ...settings }, ...payload } }) => ({ ...state, diff --git a/src/editors/data/redux/problem/reducers.test.js b/src/editors/data/redux/problem/reducers.test.js index b503206de..83421f4c7 100644 --- a/src/editors/data/redux/problem/reducers.test.js +++ b/src/editors/data/redux/problem/reducers.test.js @@ -19,6 +19,7 @@ describe('problem reducer', () => { it(`load ${target} from payload`, () => { expect(reducer(testingState, actions[action](testValue))).toEqual({ ...testingState, + isDirty: true, [target]: testValue, }); }); @@ -62,6 +63,7 @@ describe('problem reducer', () => { expect(reducer(testingState, actions.addAnswer(answer))).toEqual({ ...testingState, answers: [answer], + isDirty: true, }); }); }); @@ -79,6 +81,7 @@ describe('problem reducer', () => { const payload = { hints: ['soMehInt'] }; expect(reducer(testingState, actions.updateSettings(payload))).toEqual({ ...testingState, + isDirty: true, settings: { ...testingState.settings, ...payload, @@ -99,6 +102,7 @@ describe('problem reducer', () => { expect(reducer({ ...testingState, problemType: 'choiceresponse' }, actions.addAnswer())).toEqual({ ...testingState, problemType: 'choiceresponse', + isDirty: true, answers: [answer], }); }); @@ -111,6 +115,7 @@ describe('problem reducer', () => { expect(reducer(numericTestState, actions.addAnswer())).toEqual({ ...numericTestState, correctAnswerCount: 1, + isDirty: true, answers: [{ ...answer, correct: true, @@ -131,6 +136,7 @@ describe('problem reducer', () => { expect(reducer({ ...testingState, problemType: ProblemTypeKeys.NUMERIC }, actions.addAnswerRange())).toEqual({ ...testingState, correctAnswerCount: 1, + isDirty: true, problemType: ProblemTypeKeys.NUMERIC, answers: [answerRange], }); @@ -151,6 +157,7 @@ describe('problem reducer', () => { )).toEqual({ ...testingState, correctAnswerCount: 1, + isDirty: true, answers: [{ id: 'A', correct: true }], }); }); @@ -183,6 +190,7 @@ describe('problem reducer', () => { actions.deleteAnswer(payload), )).toEqual({ ...testingState, + isDirty: true, correctAnswerCount: 0, answers: [{ id: 'A', @@ -220,6 +228,7 @@ describe('problem reducer', () => { )).toEqual({ ...testingState, correctAnswerCount: 1, + isDirty: true, answers: [{ id: 'A', correct: true, @@ -259,6 +268,7 @@ describe('problem reducer', () => { )).toEqual({ ...testingState, problemType: ProblemTypeKeys.SINGLESELECT, + isDirty: true, correctAnswerCount: 1, answers: [{ id: 'A', @@ -300,6 +310,7 @@ describe('problem reducer', () => { )).toEqual({ ...testingState, correctAnswerCount: 1, + isDirty: true, answers: [{ id: 'A', correct: true, @@ -380,6 +391,7 @@ describe('problem reducer', () => { )).toEqual({ ...testingState, correctAnswerCount: 1, + isDirty: true, answers: [{ id: 'A', correct: true, @@ -429,6 +441,7 @@ describe('problem reducer', () => { ...testingState, problemType: ProblemTypeKeys.NUMERIC, correctAnswerCount: 1, + isDirty: true, answers: [{ id: 'A', title: '', diff --git a/src/editors/data/redux/problem/selectors.js b/src/editors/data/redux/problem/selectors.js index 1ba3c3ea0..fcd36b37a 100644 --- a/src/editors/data/redux/problem/selectors.js +++ b/src/editors/data/redux/problem/selectors.js @@ -17,6 +17,7 @@ export const simpleSelectors = { question: mkSimpleSelector(problemData => problemData.question), defaultSettings: mkSimpleSelector(problemData => problemData.defaultSettings), completeState: mkSimpleSelector(problemData => problemData), + isDirty: mkSimpleSelector(problemData => problemData.isDirty), }; export default { diff --git a/src/generic/promptIfDirty/PromtIfDirty.test.jsx b/src/generic/promptIfDirty/usePromptIfDirty.test.jsx similarity index 52% rename from src/generic/promptIfDirty/PromtIfDirty.test.jsx rename to src/generic/promptIfDirty/usePromptIfDirty.test.jsx index b429a7e13..a82597ad4 100644 --- a/src/generic/promptIfDirty/PromtIfDirty.test.jsx +++ b/src/generic/promptIfDirty/usePromptIfDirty.test.jsx @@ -1,21 +1,15 @@ -import React from 'react'; -import { render, unmountComponentAtNode } from 'react-dom'; -import { act } from 'react-dom/test-utils'; -import PromptIfDirty from './PromptIfDirty'; +import { renderHook } from '@testing-library/react-hooks'; +import usePromptIfDirty from './usePromptIfDirty'; -describe('PromptIfDirty', () => { - let container = null; +describe('usePromptIfDirty', () => { let mockEvent = null; beforeEach(() => { - container = document.createElement('div'); - document.body.appendChild(container); mockEvent = new Event('beforeunload'); jest.spyOn(window, 'addEventListener'); jest.spyOn(window, 'removeEventListener'); jest.spyOn(mockEvent, 'preventDefault'); Object.defineProperty(mockEvent, 'returnValue', { writable: true }); - mockEvent.returnValue = ''; }); afterEach(() => { @@ -23,49 +17,32 @@ describe('PromptIfDirty', () => { window.removeEventListener.mockRestore(); mockEvent.preventDefault.mockRestore(); mockEvent = null; - unmountComponentAtNode(container); - container.remove(); - container = null; }); it('should add event listener on mount', () => { - act(() => { - render(, container); - }); + renderHook(() => usePromptIfDirty(() => true)); expect(window.addEventListener).toHaveBeenCalledWith('beforeunload', expect.any(Function)); }); it('should remove event listener on unmount', () => { - act(() => { - render(, container); - }); - act(() => { - unmountComponentAtNode(container); - }); + const { unmount } = renderHook(() => usePromptIfDirty(() => true)); + unmount(); expect(window.removeEventListener).toHaveBeenCalledWith('beforeunload', expect.any(Function)); }); it('should call preventDefault and set returnValue when dirty is true', () => { - act(() => { - render(, container); - }); - act(() => { - window.dispatchEvent(mockEvent); - }); + renderHook(() => usePromptIfDirty(() => true)); + window.dispatchEvent(mockEvent); expect(mockEvent.preventDefault).toHaveBeenCalled(); - expect(mockEvent.returnValue).toBe(''); + expect(mockEvent.returnValue).toBe(true); }); it('should not call preventDefault when dirty is false', () => { - act(() => { - render(, container); - }); - act(() => { - window.dispatchEvent(mockEvent); - }); + renderHook(() => usePromptIfDirty(() => false)); + window.dispatchEvent(mockEvent); expect(mockEvent.preventDefault).not.toHaveBeenCalled(); }); diff --git a/src/generic/promptIfDirty/PromptIfDirty.jsx b/src/generic/promptIfDirty/usePromptIfDirty.ts similarity index 57% rename from src/generic/promptIfDirty/PromptIfDirty.jsx rename to src/generic/promptIfDirty/usePromptIfDirty.ts index a686ea2e8..c14be2763 100644 --- a/src/generic/promptIfDirty/PromptIfDirty.jsx +++ b/src/generic/promptIfDirty/usePromptIfDirty.ts @@ -1,12 +1,13 @@ import { useEffect } from 'react'; -import PropTypes from 'prop-types'; -const PromptIfDirty = ({ dirty }) => { +const usePromptIfDirty = (checkIfDirty : () => boolean) => { useEffect(() => { // eslint-disable-next-line consistent-return const handleBeforeUnload = (event) => { - if (dirty) { + if (checkIfDirty()) { event.preventDefault(); + // Included for legacy support, e.g. Chrome/Edge < 119 + event.returnValue = true; // eslint-disable-line no-param-reassign } }; window.addEventListener('beforeunload', handleBeforeUnload); @@ -14,11 +15,9 @@ const PromptIfDirty = ({ dirty }) => { return () => { window.removeEventListener('beforeunload', handleBeforeUnload); }; - }, [dirty]); + }, [checkIfDirty]); return null; }; -PromptIfDirty.propTypes = { - dirty: PropTypes.bool.isRequired, -}; -export default PromptIfDirty; + +export default usePromptIfDirty; diff --git a/src/textbooks/textbook-form/TextbookForm.jsx b/src/textbooks/textbook-form/TextbookForm.jsx index f065b2c11..dfc472ce4 100644 --- a/src/textbooks/textbook-form/TextbookForm.jsx +++ b/src/textbooks/textbook-form/TextbookForm.jsx @@ -18,7 +18,7 @@ import { } from '@openedx/paragon'; import FormikControl from '../../generic/FormikControl'; -import PromptIfDirty from '../../generic/promptIfDirty/PromptIfDirty'; +import PromptIfDirty from '../../generic/prompt-if-dirty/PromptIfDirty'; import ModalDropzone from '../../generic/modal-dropzone/ModalDropzone'; import { useModel } from '../../generic/model-store'; import { UPLOAD_FILE_MAX_SIZE } from '../../constants';